docs: normalize feature callouts and sync PROGRESS#129
Merged
Conversation
- Table layout for wireframes and Implementation status (29 feature files) - Add scripts/normalize-feature-docs.py + drift guard for deprecated format - Sync PROGRESS.md (React 19, E06 infra, F02/F04 backend, retrofit status) - Document layer-status conventions in epics/README and docs-style.md - Regenerate system/container diagrams for React 19 Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR standardizes epic feature documentation formatting across 50+ markdown files by introducing a template, documenting formatting rules, building a normalization script, and integrating it into CI checks. Additionally, it updates frontend stack references from React 18 to React 19 in diagrams and progress tracking. ChangesEpic Feature Documentation Format Standards and Enforcement
Frontend Stack Version Update (React 18 → 19)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
- Fix epics/README link to docs-style (lychee was failing) - Add docs/epics/_template-feature-us.md with valid example paths - normalize-feature-docs.py --check for CI via check-doc-drift.sh - Restore docs/README.md; document template in navigation table - Update CONTRIBUTING and agent-checklist for normalize workflow Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (13)
docs/epics/README.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEscape pipes in inline code inside the table cell.
Line 16 uses
|characters inside backticks within a table row, which breaks column parsing in many Markdown renderers and linters (MD056). Escape the pipes (\|) or move the command out of the table.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/README.md` at line 16, The table row contains inline code with unescaped pipe characters which breaks Markdown table parsing; update the inline code in the row (the grep command string `grep -rE "\| Application \| ⚠️|\| Infrastructure \| ⚠️|\| API \| ⚠️" docs/epics/`) to escape each pipe as `\|` (or alternatively move the command out of the table into a fenced code block), so replace the inline snippet in the README table row with an escaped-pipe version to satisfy MD056.docs/epics/E02-identity-access/features/F02-user-management.md-99-100 (1)
99-100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit
Deferred (PR#Nfollow-up)lines for pending work.These sections mark work as pending/not started, but the callouts do not include a dedicated deferred line. Please add one per deferred item to keep status semantics explicit.
As per coding guidelines, "Add a
Deferred (PR#Nfollow-up)line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".Also applies to: 139-140, 176-177
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E02-identity-access/features/F02-user-management.md` around lines 99 - 100, The doc currently lists pending items (session sign-in after accept, platform-wide email check, and related notes) without explicit deferral markers; update the feature callouts to include a "Deferred (PR `#N` follow-up)" line for each deferred/not-started item (e.g., where you mention Invitation.Accept(), ValidationException handling, AcceptInvitationHandler calling user.VerifyEmail(), and the session sign-in/API auth concern) so the implementation status explicitly records these as deferred and not marked ✅.docs/epics/E01-platform-foundation/features/F01-tenant-registration.md-154-154 (1)
154-154:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize deferred follow-up format to include PR number.
Use
Deferred (PR#Nfollow-up)instead ofDeferred (PR follow-up)to match the documented convention.As per coding guidelines, "Add a
Deferred (PR#Nfollow-up)line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E01-platform-foundation/features/F01-tenant-registration.md` at line 154, Replace the ambiguous "Deferred (PR follow-up)" callout text with the standardized format including the PR number (e.g., "Deferred (PR `#123` follow-up)") wherever the exact string "Deferred (PR follow-up)" appears in the implementation status callout; update the specific line containing "**Deferred (PR follow-up): external paging integration for platform alerts (critical log is the MVP signal).**" to "Deferred (PR `#N` follow-up): ..." with the correct PR number and ensure the callout is not marked as completed (remove any ✅ status) so the deferred work follows the documented convention.docs/epics/E01-platform-foundation/features/F04-subscription-plans.md-105-105 (1)
105-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the required deferred label pattern.
Please rewrite this to
Deferred (PR#Nfollow-up): ...so it matches the docs standard.As per coding guidelines, "Add a
Deferred (PR#Nfollow-up)line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E01-platform-foundation/features/F04-subscription-plans.md` at line 105, Replace the callout line that currently reads "Deferred (follow-up PR): atomic check-and-consume for monthly execution starts (race can briefly exceed cap under concurrency); fail-closed when usage counter/Redis unavailable (today logs warning and treats usage as 0)." with the standardized pattern "Deferred (PR `#N` follow-up): atomic check-and-consume for monthly execution starts (race can briefly exceed cap under concurrency); fail-closed when usage counter/Redis unavailable (today logs warning and treats usage as 0)." so it matches the docs standard and include a placeholder PR number (PR `#N`) to be updated when the follow-up PR is created.docs/epics/E02-identity-access/features/F03-role-management.md-89-90 (1)
89-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCall out deferred items explicitly in each affected status block.
Where work is marked pending in
Gaps vs spec, add a standaloneDeferred (PR#Nfollow-up): ...line to keep the implementation status format consistent and auditable.As per coding guidelines, "Add a
Deferred (PR#Nfollow-up)line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".Also applies to: 127-128, 163-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E02-identity-access/features/F03-role-management.md` around lines 89 - 90, Update the implementation status callouts in the role-management feature doc so any "pending" item in the "Gaps vs spec" blocks is accompanied by a standalone "Deferred (PR `#N` follow-up): <short description>" line; specifically, locate the "Gaps vs spec" callouts that currently say "403 permission check requires JWT identity from API layer — pending" (and the other two occurrences) and add a Deferred line after each pending note that references a follow-up PR placeholder (PR `#N`) and a brief action (e.g., "implement API-layer JWT identity check and 403 enforcement"). Ensure the new Deferred lines are separate sentences/lines in the same status callout so the implementation status format remains consistent and auditable.docs/epics/E01-platform-foundation/features/F01-tenant-registration.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate
## Wireframesheading.There are two identical section headings in the same file, which triggers duplicate-heading lint and makes the doc structure ambiguous.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E01-platform-foundation/features/F01-tenant-registration.md` at line 16, The file contains a duplicated section heading "## Wireframes"; remove the redundant "## Wireframes" occurrence so only a single "## Wireframes" heading remains (locate the two identical headings in F01-tenant-registration.md and delete the extra one), then verify the document structure still flows correctly and re-run the markdown linter to ensure the duplicate-heading warning is resolved.docs/epics/E01-platform-foundation/features/F02-organization-management.md-143-143 (1)
143-143:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSplit deferred content into its own
Deferredline.
Deferred (PR#127follow-up)is currently embedded inGaps vs spec; move it to a standalone deferred line in the callout for consistency with the format standard.As per coding guidelines, "Add a
Deferred (PR#Nfollow-up)line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E01-platform-foundation/features/F02-organization-management.md` at line 143, The callout line containing "Gaps vs spec: Deferred (PR `#127` follow-up): ..." should be split so that "Deferred (PR `#127` follow-up): marketing-page redirect + forced sign-out after schedule (Frontend/session); ..." is its own standalone Deferred line; locate the callout containing the "Gaps vs spec:" text and remove the embedded "Deferred (PR `#127` follow-up)" portion from that line, then add a separate line starting with "Deferred (PR `#127` follow-up):" containing the deferred items (including the reference to docs/WORKAROUNDS.md#org-hard-delete-modulith-cancellers), and ensure the implementation status callout follows the guideline of adding a Deferred line rather than silently marking incomplete work as ✅.docs/epics/E03-data-modeling/features/F01-model-definition.md-168-168 (1)
168-168:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the required deferred follow-up format.
Please rewrite this as
Deferred (PR#Nfollow-up): ...so the handoff is trackable and consistent with the docs standard.As per coding guidelines, "Add a
Deferred (PR#Nfollow-up)line in the implementation status callout when deferring work".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E03-data-modeling/features/F01-model-definition.md` at line 168, Replace the current "Deferred:" line with the required format "Deferred (PR `#N` follow-up): ..." so the handoff is trackable; specifically update the implementation status callout in the feature doc (the line currently reading "Deferred: DataModeling relation fields on other models flagged broken when target model deleted. WorkflowBuilder `record.*` trigger broken flags shipped via `ModelDeletedHandler` (Kafka).") to use the exact phrasing "Deferred (PR `#N` follow-up): ..." and include the PR number placeholder, preserving the rest of the explanation and references to WorkflowBuilder and ModelDeletedHandler.docs/epics/E04-workflow-builder/features/F01-workflow-definition.md-79-90 (1)
79-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatus tables should not show ✅ where API work is still pending.
These sections list unresolved backend/API work in “Gaps vs spec” but still mark API as complete. Please make the API status non-complete until those gaps are closed.
As per coding guidelines, "never silently mark incomplete work as ✅".
Also applies to: 114-125, 149-160
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E04-workflow-builder/features/F01-workflow-definition.md` around lines 79 - 90, Update the implementation status tables so the API row is not marked as complete: change the API cell from "✅" to "⏳" (or another non-complete symbol) in the "Implementation status" table at the current block and the other occurrences referenced (around the sections covering lines 114-125 and 149-160) to reflect the "Gaps vs spec" notes about status-tab filter and last-execution-date/backend polish; ensure the table text remains aligned and the "Gaps vs spec" note clearly corresponds to the API being in-progress.docs/epics/E02-identity-access/features/F04-permissions.md-91-103 (1)
91-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign API status with listed pending work.
Both callouts mark API as ✅ while the same blocks say policy middleware/authorization work is still pending. Please switch those API statuses to a non-complete state and keep the gap note, so progress isn’t overstated.
As per coding guidelines, "never silently mark incomplete work as ✅".
Also applies to: 128-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E02-identity-access/features/F04-permissions.md` around lines 91 - 103, Update the "Implementation status" table under the "Implementation status" heading so the API row is not marked ✅ (use a non-complete marker like ⏳ or ⚠️) and keep the "Gaps vs spec:" note about policy-based authorization middleware and the `[RequirePermission]` attribute; also locate and update the second identical status table later in the document (the other "Implementation status" block) to match this change so the API progress isn’t overstated while preserving the existing gap/decision text.docs/epics/E02-identity-access/features/F05-password-security.md-53-65 (1)
53-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatus tables currently over-report completion for API work.
These sections list API/backend gaps but still mark API as ✅. Please mark API as incomplete until those items are shipped, then flip to ✅.
As per coding guidelines, "never silently mark incomplete work as ✅".
Also applies to: 92-104, 131-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E02-identity-access/features/F05-password-security.md` around lines 53 - 65, The Implementation status table under the "**Implementation status**" heading currently marks the API as ✅ despite listed API/backend gaps; update the API row (the cell showing "API | ✅") to an incomplete state (e.g., ⏳ or ❌) and do the same for the other two occurrences referenced (the sections around lines 92-104 and 131-143) so the docs don't silently report incomplete work as done; keep the Domain/Application/Infrastructure/Frontend statuses unchanged and ensure a brief note next to the API row can indicate "gaps: rate limiting, auto sign-in after reset" if you want immediate clarity.docs/epics/E03-data-modeling/features/F04-data-records.md-245-245 (1)
245-245:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConvert this deferral note to the mandated follow-up line format.
Please add a dedicated
Deferred (PR#Nfollow-up): ...line in the implementation-status callout instead of inline “deferred” wording.As per coding guidelines, "Add a
Deferred (PR#Nfollow-up)line in the implementation status callout when deferring work".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E03-data-modeling/features/F04-data-records.md` at line 245, Update the implementation-status callout so the inline text "async export for >5,000 records deferred" is removed and replaced by a dedicated follow-up line reading exactly "Deferred (PR `#N` follow-up): async export for >5,000 records — pending Wolverine background job + in-app notification infrastructure"; edit the file's implementation-status block in docs/epics/E03-data-modeling/features/F04-data-records.md and locate the sentence containing "async export for >5,000 records deferred" to convert it into the new Deferred (PR `#N` follow-up) line, leaving the rest of the CSV export streaming note intact and using a placeholder PR number to be filled when creating the follow-up PR.docs/epics/E04-workflow-builder/features/F02-visual-canvas.md-155-161 (1)
155-161:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize layer-status symbols to the required scheme.
These callouts use
N/A/⏳, but the doc rule requires layer statuses expressed with✅or⚠️. Please convert to the approved symbols and keep the explanatory note for client-only scope.As per coding guidelines, "Include an
Implementation statuscallout ... with layer status (✅ or⚠️ )".Also applies to: 189-195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/epics/E04-workflow-builder/features/F02-visual-canvas.md` around lines 155 - 161, The "Implementation status" callout uses the wrong symbols—replace the table values so layer statuses use the approved symbols (use ✅ for implemented/covered and ⚠️ for client-only or pending) by editing the markdown table under the "Implementation status" header (replace "N/A" with ✅/⚠️ as appropriate and replace the ⏳ with ⚠️ for Frontend), preserve the explanatory note about client-only scope immediately below the table, and apply the same replacements to the other occurrence of the same "Implementation status" table later in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/epics/E04-workflow-builder/features/F03-step-types.md`:
- Around line 45-57: The Implementation status callouts that list incomplete
work (e.g., the one noting "Frontend ⏳" and gaps like "form picker UI, assignee
expression evaluation, and timeout enforcement pending Frontend + E06") must
include an explicit deferred-tracking line; add a line reading "Deferred (PR `#N`
follow-up)" under each Implementation status table entry that is not complete so
the pending work is tracked, and repeat this change for the other Implementation
status callouts referenced in the comment (the similar tables at the other
sections). Ensure you place the exact text "Deferred (PR `#N` follow-up)" beneath
the callout so it follows the project guideline.
In `@docs/epics/E04-workflow-builder/features/F04-triggers.md`:
- Around line 43-55: The "Implementation status" callouts in the F04-triggers
doc mark some layers as deferred/⏳ but omit the required deferred tracking line;
update each "Implementation status" callout block (the ones showing
Domain/Application/Infrastructure/API/Frontend statuses) to include a line
"Deferred (PR `#N` follow-up)" for any layer not completed (⏳), ensuring every
deferred status block contains that exact tracking line; apply this change to
the three other callouts referenced (the blocks around lines 83-94, 122-133,
162-173) so no incomplete work is silently marked as ✅.
In `@docs/epics/E04-workflow-builder/features/F05-branching.md`:
- Around line 42-54: The implementation status callout currently marks
incomplete work without an explicit deferred-followup note; update the
Implementation status table(s) in this document (the main "Implementation
status" callout and the other similar callouts referenced later) so that any
layer not finished (e.g., Frontend ⏳ and any other non-✅ entries) includes a
line reading "Deferred (PR `#N` follow-up)"; ensure you add that deferred line for
each deferred layer rather than leaving it silently incomplete and keep the
existing status symbols and text intact.
In `@docs/epics/E04-workflow-builder/features/F06-parallel-execution.md`:
- Around line 44-55: Update the "Implementation status" callouts to explicitly
annotate any pending/incomplete work with the mandated deferred note: add a line
"Deferred (PR `#N` follow-up)" inside each callout that currently lists gaps
(e.g., the callout around "canvas container node rendering and step nesting UI
pending Frontend" and the other callouts referenced at 81-92 and 115-126), and
ensure no incomplete item is marked with a definitive ✅; replace or augment any
ambiguous status entries so the callout contains the exact deferred annotation.
In `@docs/epics/E04-workflow-builder/features/F07-import-export.md`:
- Around line 42-54: The "Implementation status" callout blocks that list
Layer|Status (the tables showing Domain/Application/Infrastructure/API/Frontend
and the related callouts) are missing the required deferred follow-up tag; for
each callout that currently shows unfinished work (e.g., the Frontend ⏳ or other
deferred notes in the same callout), append a line "Deferred (PR `#N` follow-up)"
immediately under the callout text so each deferred implementation status
includes that exact follow-up marker; apply the same change to the other two
matching callouts in this document (the blocks around the other "Implementation
status" callouts).
In `@docs/epics/E05-form-builder/features/F01-form-definition.md`:
- Around line 42-54: Update each "Implementation status" callout in the document
to append a deferred follow-up note for any layer that is not fully implemented:
for every status table where a layer shows pending work (e.g., the Frontend
marked ⏳), add a new line "Deferred (PR `#N` follow-up)" immediately after the
table; ensure you update the three other callouts referenced (lines 78-90,
114-126, 150-162) as well so no incomplete work remains marked solely as ✅. Use
the same phrasing "Deferred (PR `#N` follow-up)" and place it directly under the
Implementation status table for the sections that have pending work.
In `@docs/epics/E05-form-builder/features/F02-form-fields.md`:
- Around line 47-59: Update the "Implementation status" callout blocks so any
non-complete layer shows a deferred follow-up line; specifically, for the four
status blocks in this doc (the block showing
Domain/Application/Infrastructure/API/Frontend and the other three mentioned at
lines ~86-97, ~121-133, ~157-169), append a line "Deferred (PR `#N` follow-up)"
for each layer that is not final/complete (e.g., where Frontend is ⏳ or gaps are
listed). Ensure you keep the existing status symbols and add the deferred note
consistently across all four blocks and replace N with the actual PR number when
known.
In `@docs/epics/E05-form-builder/features/F03-workflow-integration.md`:
- Around line 44-56: The implementation status tables are missing explicit
deferred tracking entries; update each "Frontend | ⏳" (and any other non-✅ rows)
in the "Implementation status" callouts to append a line like "Deferred (PR `#N`
follow-up)" so deferred work is tied to a follow-up PR, and ensure the same
change is applied to the other callouts referenced (around the second and third
tables); keep the descriptive notes about GetFormPickerQuery and
IsReferencedByWorkflowAsync untouched, only add the Deferred (PR `#N` follow-up)
lines to the deferred status entries.
In `@docs/epics/E05-form-builder/features/F04-form-submission.md`:
- Around line 45-56: The "Implementation status" callout(s) currently list
incomplete layers (e.g., Application, API, Frontend) but omit the required
follow-up marker; update each "Implementation status" callout by adding a new
line reading "Deferred (PR `#N` follow-up)" for every deferred/incomplete layer
(replace `#N` with the appropriate PR number), and ensure you apply this same
change to all other "Implementation status" callouts in the document (the other
occurrences that show pending/⏳/⚠️ statuses) so every deferred item is
explicitly tracked.
In `@docs/epics/E06-workflow-engine/features/F01-execution-management.md`:
- Around line 46-57: Update every "Implementation status" callout that lists
incomplete layers to include a deferred follow-up line; specifically add a line
like "Deferred (PR `#N` follow-up): describe remaining work" under each block that
currently shows ⏳ (e.g., the callout showing API and Frontend as ⏳ in the
existing Implementation status and the similar callouts around the other
sections referenced). Ensure the message names the PR placeholder "PR `#N`" and a
brief description of what's deferred (e.g., "trigger HTTP endpoint,
schedule/webhook/event trigger handlers, stale-PENDING recovery job") so no
incomplete work is silently marked, and apply the same change to the other
Implementation status blocks noted in the comment.
In `@docs/epics/E06-workflow-engine/features/F02-step-handlers.md`:
- Around line 100-114: Update the "Implementation status" callout block to
explicitly list the deferred follow-up by adding a line like "Deferred (PR `#N`
follow-up)" under the status/gaps content (near the other bullet points about
gaps vs spec), so the callout that begins with "**Implementation status**" and
the subsequent "**Gaps vs spec:**" section includes a clear Deferred entry
indicating which PR will follow up on the unfinished work.
In `@docs/epics/E06-workflow-engine/features/F03-error-handling.md`:
- Around line 44-55: In the "Implementation status" callout and the two other
deferred callouts that omit follow-ups, add explicit follow-up lines using the
exact format "Deferred (PR `#N` follow-up): <brief next-step>" so each deferred
item (e.g., the Application-layer notification dispatch handler, wiring
ExecutionFailed event to channels, and pending email/in-app/webhook dispatch +
rate-limiting work) documents the PR number and next action; locate the three
callouts by the "Implementation status" table and the two other deferred
callouts in this feature doc and append a "Deferred (PR `#N` follow-up): ..." line
to each deferred entry.
In `@docs/epics/E06-workflow-engine/features/F04-execution-history.md`:
- Around line 45-57: Update each "Implementation status" callout that lists
deferred work to include a "Deferred (PR `#N` follow-up)" line; specifically add
this to the callout that mentions Filter UI, date/trigger query params, and
running-first sort pending API + Frontend (the block that references
GetExecutionsByWorkflowHandler and GetAllExecutionsHandler using
IExecutionRepository.GetPagedByWorkflowAsync/GetPagedAsync), and likewise add
the same deferred line to the other two implementation-status callouts
referenced in the comment (around the blocks at lines 83-94 and 118-129) so no
incomplete work is marked as ✅ without a follow-up PR reference.
In `@docs/epics/E06-workflow-engine/features/F05-manual-retry.md`:
- Around line 47-59: Update each "Implementation status" callout that contains
any incomplete work to include a deferred follow-up note: add the exact line
"Deferred (PR `#N` follow-up)" beneath the status table for the callout shown (the
callout starting with "**Implementation status**" and its status table), and do
the same for the two other matching callouts later in the document (the other
"**Implementation status**" sections referenced). Ensure the deferred line
appears whenever a non-✅ item or pending work is present (e.g., Frontend ⏳,
archived-definition warning), keeping the table and surrounding wording
unchanged except for inserting the Deferred (PR `#N` follow-up) line.
In `@scripts/normalize-feature-docs.py`:
- Around line 230-243: The script collects deprecated_issues by calling
check_deprecated_format(path) before process_file(path), causing exit_code to be
set even when process_file normalizes the file; update the loop that iterates
over ROOT.glob(FEATURE_GLOB) to run process_file(path, check_only=args.check)
first and only call check_deprecated_format(path) afterward (or re-run
check_deprecated_format only when args.check is true) so deprecated_issues
reflects the post-normalization state; reference the loop, deprecated_issues,
check_deprecated_format, and process_file to locate and change the call order or
conditional logic.
---
Minor comments:
In `@docs/epics/E01-platform-foundation/features/F01-tenant-registration.md`:
- Line 154: Replace the ambiguous "Deferred (PR follow-up)" callout text with
the standardized format including the PR number (e.g., "Deferred (PR `#123`
follow-up)") wherever the exact string "Deferred (PR follow-up)" appears in the
implementation status callout; update the specific line containing "**Deferred
(PR follow-up): external paging integration for platform alerts (critical log is
the MVP signal).**" to "Deferred (PR `#N` follow-up): ..." with the correct PR
number and ensure the callout is not marked as completed (remove any ✅ status)
so the deferred work follows the documented convention.
- Line 16: The file contains a duplicated section heading "## Wireframes";
remove the redundant "## Wireframes" occurrence so only a single "## Wireframes"
heading remains (locate the two identical headings in F01-tenant-registration.md
and delete the extra one), then verify the document structure still flows
correctly and re-run the markdown linter to ensure the duplicate-heading warning
is resolved.
In `@docs/epics/E01-platform-foundation/features/F02-organization-management.md`:
- Line 143: The callout line containing "Gaps vs spec: Deferred (PR `#127`
follow-up): ..." should be split so that "Deferred (PR `#127` follow-up):
marketing-page redirect + forced sign-out after schedule (Frontend/session);
..." is its own standalone Deferred line; locate the callout containing the
"Gaps vs spec:" text and remove the embedded "Deferred (PR `#127` follow-up)"
portion from that line, then add a separate line starting with "Deferred (PR
`#127` follow-up):" containing the deferred items (including the reference to
docs/WORKAROUNDS.md#org-hard-delete-modulith-cancellers), and ensure the
implementation status callout follows the guideline of adding a Deferred line
rather than silently marking incomplete work as ✅.
In `@docs/epics/E01-platform-foundation/features/F04-subscription-plans.md`:
- Line 105: Replace the callout line that currently reads "Deferred (follow-up
PR): atomic check-and-consume for monthly execution starts (race can briefly
exceed cap under concurrency); fail-closed when usage counter/Redis unavailable
(today logs warning and treats usage as 0)." with the standardized pattern
"Deferred (PR `#N` follow-up): atomic check-and-consume for monthly execution
starts (race can briefly exceed cap under concurrency); fail-closed when usage
counter/Redis unavailable (today logs warning and treats usage as 0)." so it
matches the docs standard and include a placeholder PR number (PR `#N`) to be
updated when the follow-up PR is created.
In `@docs/epics/E02-identity-access/features/F02-user-management.md`:
- Around line 99-100: The doc currently lists pending items (session sign-in
after accept, platform-wide email check, and related notes) without explicit
deferral markers; update the feature callouts to include a "Deferred (PR `#N`
follow-up)" line for each deferred/not-started item (e.g., where you mention
Invitation.Accept(), ValidationException handling, AcceptInvitationHandler
calling user.VerifyEmail(), and the session sign-in/API auth concern) so the
implementation status explicitly records these as deferred and not marked ✅.
In `@docs/epics/E02-identity-access/features/F03-role-management.md`:
- Around line 89-90: Update the implementation status callouts in the
role-management feature doc so any "pending" item in the "Gaps vs spec" blocks
is accompanied by a standalone "Deferred (PR `#N` follow-up): <short description>"
line; specifically, locate the "Gaps vs spec" callouts that currently say "403
permission check requires JWT identity from API layer — pending" (and the other
two occurrences) and add a Deferred line after each pending note that references
a follow-up PR placeholder (PR `#N`) and a brief action (e.g., "implement
API-layer JWT identity check and 403 enforcement"). Ensure the new Deferred
lines are separate sentences/lines in the same status callout so the
implementation status format remains consistent and auditable.
In `@docs/epics/E02-identity-access/features/F04-permissions.md`:
- Around line 91-103: Update the "Implementation status" table under the
"Implementation status" heading so the API row is not marked ✅ (use a
non-complete marker like ⏳ or ⚠️) and keep the "Gaps vs spec:" note about
policy-based authorization middleware and the `[RequirePermission]` attribute;
also locate and update the second identical status table later in the document
(the other "Implementation status" block) to match this change so the API
progress isn’t overstated while preserving the existing gap/decision text.
In `@docs/epics/E02-identity-access/features/F05-password-security.md`:
- Around line 53-65: The Implementation status table under the "**Implementation
status**" heading currently marks the API as ✅ despite listed API/backend gaps;
update the API row (the cell showing "API | ✅") to an incomplete state (e.g., ⏳
or ❌) and do the same for the other two occurrences referenced (the sections
around lines 92-104 and 131-143) so the docs don't silently report incomplete
work as done; keep the Domain/Application/Infrastructure/Frontend statuses
unchanged and ensure a brief note next to the API row can indicate "gaps: rate
limiting, auto sign-in after reset" if you want immediate clarity.
In `@docs/epics/E03-data-modeling/features/F01-model-definition.md`:
- Line 168: Replace the current "Deferred:" line with the required format
"Deferred (PR `#N` follow-up): ..." so the handoff is trackable; specifically
update the implementation status callout in the feature doc (the line currently
reading "Deferred: DataModeling relation fields on other models flagged broken
when target model deleted. WorkflowBuilder `record.*` trigger broken flags
shipped via `ModelDeletedHandler` (Kafka).") to use the exact phrasing "Deferred
(PR `#N` follow-up): ..." and include the PR number placeholder, preserving the
rest of the explanation and references to WorkflowBuilder and
ModelDeletedHandler.
In `@docs/epics/E03-data-modeling/features/F04-data-records.md`:
- Line 245: Update the implementation-status callout so the inline text "async
export for >5,000 records deferred" is removed and replaced by a dedicated
follow-up line reading exactly "Deferred (PR `#N` follow-up): async export for
>5,000 records — pending Wolverine background job + in-app notification
infrastructure"; edit the file's implementation-status block in
docs/epics/E03-data-modeling/features/F04-data-records.md and locate the
sentence containing "async export for >5,000 records deferred" to convert it
into the new Deferred (PR `#N` follow-up) line, leaving the rest of the CSV export
streaming note intact and using a placeholder PR number to be filled when
creating the follow-up PR.
In `@docs/epics/E04-workflow-builder/features/F01-workflow-definition.md`:
- Around line 79-90: Update the implementation status tables so the API row is
not marked as complete: change the API cell from "✅" to "⏳" (or another
non-complete symbol) in the "Implementation status" table at the current block
and the other occurrences referenced (around the sections covering lines 114-125
and 149-160) to reflect the "Gaps vs spec" notes about status-tab filter and
last-execution-date/backend polish; ensure the table text remains aligned and
the "Gaps vs spec" note clearly corresponds to the API being in-progress.
In `@docs/epics/E04-workflow-builder/features/F02-visual-canvas.md`:
- Around line 155-161: The "Implementation status" callout uses the wrong
symbols—replace the table values so layer statuses use the approved symbols (use
✅ for implemented/covered and ⚠️ for client-only or pending) by editing the
markdown table under the "Implementation status" header (replace "N/A" with ✅/⚠️
as appropriate and replace the ⏳ with ⚠️ for Frontend), preserve the explanatory
note about client-only scope immediately below the table, and apply the same
replacements to the other occurrence of the same "Implementation status" table
later in the file.
In `@docs/epics/README.md`:
- Line 16: The table row contains inline code with unescaped pipe characters
which breaks Markdown table parsing; update the inline code in the row (the grep
command string `grep -rE "\| Application \| ⚠️|\| Infrastructure \| ⚠️|\| API \|
⚠️" docs/epics/`) to escape each pipe as `\|` (or alternatively move the command
out of the table into a fenced code block), so replace the inline snippet in the
README table row with an escaped-pipe version to satisfy MD056.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5888b654-3bd0-43d5-8471-4706ef62d619
⛔ Files ignored due to path filters (2)
docs/diagrams/container.svgis excluded by!**/*.svgdocs/diagrams/system-context.svgis excluded by!**/*.svg
📒 Files selected for processing (42)
CONTRIBUTING.mddocs/PROGRESS.mddocs/README.mddocs/diagrams/container.excalidrawdocs/diagrams/generate-diagrams.mjsdocs/diagrams/system-context.excalidrawdocs/epics/E01-platform-foundation/README.mddocs/epics/E01-platform-foundation/features/F01-tenant-registration.mddocs/epics/E01-platform-foundation/features/F02-organization-management.mddocs/epics/E01-platform-foundation/features/F03-tenant-isolation.mddocs/epics/E01-platform-foundation/features/F04-subscription-plans.mddocs/epics/E02-identity-access/features/F01-authentication.mddocs/epics/E02-identity-access/features/F02-user-management.mddocs/epics/E02-identity-access/features/F03-role-management.mddocs/epics/E02-identity-access/features/F04-permissions.mddocs/epics/E02-identity-access/features/F05-password-security.mddocs/epics/E03-data-modeling/features/F01-model-definition.mddocs/epics/E03-data-modeling/features/F02-field-types.mddocs/epics/E03-data-modeling/features/F03-data-classes.mddocs/epics/E03-data-modeling/features/F04-data-records.mddocs/epics/E04-workflow-builder/features/F01-workflow-definition.mddocs/epics/E04-workflow-builder/features/F02-visual-canvas.mddocs/epics/E04-workflow-builder/features/F03-step-types.mddocs/epics/E04-workflow-builder/features/F04-triggers.mddocs/epics/E04-workflow-builder/features/F05-branching.mddocs/epics/E04-workflow-builder/features/F06-parallel-execution.mddocs/epics/E04-workflow-builder/features/F07-import-export.mddocs/epics/E05-form-builder/features/F01-form-definition.mddocs/epics/E05-form-builder/features/F02-form-fields.mddocs/epics/E05-form-builder/features/F03-workflow-integration.mddocs/epics/E05-form-builder/features/F04-form-submission.mddocs/epics/E06-workflow-engine/features/F01-execution-management.mddocs/epics/E06-workflow-engine/features/F02-step-handlers.mddocs/epics/E06-workflow-engine/features/F03-error-handling.mddocs/epics/E06-workflow-engine/features/F04-execution-history.mddocs/epics/E06-workflow-engine/features/F05-manual-retry.mddocs/epics/README.mddocs/epics/_template-feature-us.mddocs/playbooks/agent-checklist.mddocs/playbooks/docs-style.mdscripts/check-doc-drift.shscripts/normalize-feature-docs.py
Comment on lines
+45
to
57
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ✅ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** form picker UI, assignee expression evaluation, and timeout enforcement pending Frontend + E06. | ||
| > **Decisions:** step config (formId, assignee, timeout) stored as JSONB dict in `steps` column. `StepType` enum includes `Start` and `End` values. | ||
|
|
There was a problem hiding this comment.
Add required deferred follow-up lines to each Implementation status callout.
These callouts document pending work but do not include the required Deferred (PR #N follow-up) line. Please add it for each deferred gap so incomplete items are explicitly tracked per guideline.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 86-97, 124-135, 164-175, 204-215
🧰 Tools
🪛 LanguageTool
[grammar] ~45-~45: Ensure spelling is correct
Context: ...onse) — not in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E04-workflow-builder/features/F03-step-types.md` around lines 45 -
57, The Implementation status callouts that list incomplete work (e.g., the one
noting "Frontend ⏳" and gaps like "form picker UI, assignee expression
evaluation, and timeout enforcement pending Frontend + E06") must include an
explicit deferred-tracking line; add a line reading "Deferred (PR `#N` follow-up)"
under each Implementation status table entry that is not complete so the pending
work is tracked, and repeat this change for the other Implementation status
callouts referenced in the comment (the similar tables at the other sections).
Ensure you place the exact text "Deferred (PR `#N` follow-up)" beneath the callout
so it follows the project guideline.
Comment on lines
+43
to
55
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ✅ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** input variable prompt dialog and `POST /workflows/{id}/executions` endpoint pending API + E06. | ||
| > **Decisions:** trigger config (input variable definitions) stored as JSONB in `triggers` column; domain guards against duplicate trigger type per workflow (AddTrigger returns Conflict on second call for same type). `TriggerConfig` is a value object (no `id`, owned by `WorkflowDefinition`). | ||
|
|
There was a problem hiding this comment.
Implementation status sections are missing the mandatory deferred tracking line.
Each callout includes pending gaps but omits Deferred (PR #N follow-up). Add that line in every deferred status block.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 83-94, 122-133, 162-173
🧰 Tools
🪛 LanguageTool
[grammar] ~43-~43: Ensure spelling is correct
Context: ...ic; covered in E07. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E04-workflow-builder/features/F04-triggers.md` around lines 43 -
55, The "Implementation status" callouts in the F04-triggers doc mark some
layers as deferred/⏳ but omit the required deferred tracking line; update each
"Implementation status" callout block (the ones showing
Domain/Application/Infrastructure/API/Frontend statuses) to include a line
"Deferred (PR `#N` follow-up)" for any layer not completed (⏳), ensuring every
deferred status block contains that exact tracking line; apply this change to
the three other callouts referenced (the blocks around lines 83-94, 122-133,
162-173) so no incomplete work is silently marked as ✅.
Comment on lines
+42
to
54
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ✅ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** canvas branch label rendering pending Frontend; branch evaluation at execution time pending E06. | ||
| > **Decisions:** cycle detection implemented in domain (DFS reachability check in AddTransition). `Transition` is a value object (only `fromStepId`, `toStepId`, `condition` — no identity or ordering fields). | ||
|
|
There was a problem hiding this comment.
Please add explicit deferred follow-up lines in all status callouts.
Pending items are noted, but the required Deferred (PR #N follow-up) line is missing in each Implementation status callout.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 81-92, 116-127
🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: Ensure spelling is correct
Context: ...are blocked in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E04-workflow-builder/features/F05-branching.md` around lines 42 -
54, The implementation status callout currently marks incomplete work without an
explicit deferred-followup note; update the Implementation status table(s) in
this document (the main "Implementation status" callout and the other similar
callouts referenced later) so that any layer not finished (e.g., Frontend ⏳ and
any other non-✅ entries) includes a line reading "Deferred (PR `#N` follow-up)";
ensure you add that deferred line for each deferred layer rather than leaving it
silently incomplete and keep the existing status symbols and text intact.
Comment on lines
+44
to
55
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ✅ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** canvas container node rendering and step nesting UI pending Frontend; parallel group represented via step config in existing JSONB storage. `ParallelGroup` and `JoinType` are Phase 2 — shown as planned (dashed) in diagram. | ||
|
|
There was a problem hiding this comment.
Status callouts need the mandated deferred annotation.
For each pending implementation gap, add Deferred (PR #N follow-up) in the callout; it is currently missing in all three sections.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 81-92, 115-126
🧰 Tools
🪛 LanguageTool
[grammar] ~44-~44: Ensure spelling is correct
Context: ...time) — not in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E04-workflow-builder/features/F06-parallel-execution.md` around
lines 44 - 55, Update the "Implementation status" callouts to explicitly
annotate any pending/incomplete work with the mandated deferred note: add a line
"Deferred (PR `#N` follow-up)" inside each callout that currently lists gaps
(e.g., the callout around "canvas container node rendering and step nesting UI
pending Frontend" and the other callouts referenced at 81-92 and 115-126), and
ensure no incomplete item is marked with a definitive ✅; replace or augment any
ambiguous status entries so the callout contains the exact deferred annotation.
Comment on lines
+42
to
54
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ✅ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** broken-reference `"broken": true` flag pending E03/E05 integration; export notice and broken-reference warning UI pending Frontend. | ||
| > **Decisions:** credential scrubbing in `ExportWorkflowHandler` — keys matching token/api_key/apikey/secret/password/authorization/auth_token/hmac_secret/client_secret/private_key/bearer/access_token/refresh_token replaced with `[REDACTED]` (OrdinalIgnoreCase). | ||
|
|
There was a problem hiding this comment.
Add Deferred (PR #N follow-up) to each deferred implementation status block.
All three callouts indicate unfinished work but omit the required deferred follow-up line.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 83-94, 117-128
🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: Ensure spelling is correct
Context: ...BPMN) — not in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E04-workflow-builder/features/F07-import-export.md` around lines
42 - 54, The "Implementation status" callout blocks that list Layer|Status (the
tables showing Domain/Application/Infrastructure/API/Frontend and the related
callouts) are missing the required deferred follow-up tag; for each callout that
currently shows unfinished work (e.g., the Frontend ⏳ or other deferred notes in
the same callout), append a line "Deferred (PR `#N` follow-up)" immediately under
the callout text so each deferred implementation status includes that exact
follow-up marker; apply the same change to the other two matching callouts in
this document (the blocks around the other "Implementation status" callouts).
Comment on lines
+100
to
+114
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ⚠️ | | ||
| > | API | ⏳ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** | ||
| > - AC "concurrent delivery: second detects Running and exits" — spec wording updated in decision below; concurrent-duplicate protection is implemented via `UseXminAsConcurrencyToken()` on `execution_steps` rows. The second concurrent writer receives a `DbUpdateConcurrencyException` (translated to `ConcurrencyException`), logs, and exits gracefully. The "Running guard" approach was deliberately rejected — see Decisions. | ||
| > - Engine-level 5-minute step timeout not yet implemented (E06 F03 scope). | ||
| > - `IScriptExecutor` and `INotificationSender` are stubs returning empty/success; real JS sandbox engine and email/webhook dispatch are deferred. | ||
| > Decisions: `ExecutionStep.IsTerminal` covers Completed/Failed/Cancelled. `Skipped` is a pre-start terminal state set by the Condition handler for rejected branches. `StepType` is an enum (Start, End, Form, HttpRequest, Condition, Script, Notification). `WorkflowSnapshot` is a local read model populated from `WorkflowPublished` domain events — WorkflowEngine never queries WorkflowBuilder's DB. Concurrent-duplicate protection uses PostgreSQL `xmin` system column (`UseXminAsConcurrencyToken()` on the owned `execution_steps` table) — no migration required. All step handlers and `StepCompletedHandler`/`StepFailedHandler` catch `ConcurrencyException` from `IUnitOfWork.SaveChangesAsync` and exit gracefully, letting the winning instance complete the step. The Running guard (`if step.Status == Running → skip`) was rejected because `ExecuteNextStepHandler` always starts a step (→ Running) before dispatching; a Running guard would block all normal first-delivery executions. | ||
| > **Decisions:** `ExecutionStep.IsTerminal` covers Completed/Failed/Cancelled. `Skipped` is a pre-start terminal state set by the Condition handler for rejected branches. `StepType` is an enum (Start, End, Form, HttpRequest, Condition, Script, Notification). `WorkflowSnapshot` is a local read model populated from `WorkflowPublished` domain events — WorkflowEngine never queries WorkflowBuilder's DB. Concurrent-duplicate protection uses PostgreSQL `xmin` system column (`UseXminAsConcurrencyToken()` on the owned `execution_steps` table) — no migration required. All step handlers and `StepCompletedHandler`/`StepFailedHandler` catch `ConcurrencyException` from `IUnitOfWork.SaveChangesAsync` and exit gracefully, letting the winning instance complete the step. The Running guard (`if step.Status == Running → skip`) was rejected because `ExecuteNextStepHandler` always starts a step (→ Running) before dispatching; a Running guard would block all normal first-delivery executions. |
There was a problem hiding this comment.
Add the required deferred follow-up line to this status callout.
This block lists deferred work but does not include Deferred (PR #N follow-up). Please add it explicitly under the status/gaps content.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
🧰 Tools
🪛 LanguageTool
[grammar] ~100-~100: Ensure spelling is correct
Context: ...users — not in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E06-workflow-engine/features/F02-step-handlers.md` around lines
100 - 114, Update the "Implementation status" callout block to explicitly list
the deferred follow-up by adding a line like "Deferred (PR `#N` follow-up)" under
the status/gaps content (near the other bullet points about gaps vs spec), so
the callout that begins with "**Implementation status**" and the subsequent
"**Gaps vs spec:**" section includes a clear Deferred entry indicating which PR
will follow up on the unfinished work.
Comment on lines
+44
to
55
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ⚠️ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ⏳ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** no Application-layer notification dispatch handler; `ExecutionFailed` domain event raised but notification channels not wired; email/in-app/webhook dispatch and rate-limiting pending Application layer + a future cross-cutting notification service (outside WorkflowEngine Infrastructure, which is complete). | ||
|
|
There was a problem hiding this comment.
Track deferred scope explicitly with Deferred (PR #N follow-up) lines.
All three callouts defer implementation but omit the required deferred-follow-up line. Please add a Deferred (PR #N follow-up): ... entry in each one.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 81-92, 118-129
🧰 Tools
🪛 LanguageTool
[grammar] ~44-~44: Ensure spelling is correct
Context: ...tions — not in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E06-workflow-engine/features/F03-error-handling.md` around lines
44 - 55, In the "Implementation status" callout and the two other deferred
callouts that omit follow-ups, add explicit follow-up lines using the exact
format "Deferred (PR `#N` follow-up): <brief next-step>" so each deferred item
(e.g., the Application-layer notification dispatch handler, wiring
ExecutionFailed event to channels, and pending email/in-app/webhook dispatch +
rate-limiting work) documents the PR number and next action; locate the three
callouts by the "Implementation status" table and the two other deferred
callouts in this feature doc and append a "Deferred (PR `#N` follow-up): ..." line
to each deferred entry.
Comment on lines
+45
to
57
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ✅ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** `GET /api/executions` and per-workflow paged list ✅. Filter UI, date/trigger query params, and running-first sort pending API + Frontend. | ||
| > **Decisions:** `GetExecutionsByWorkflowHandler` and `GetAllExecutionsHandler` use `IExecutionRepository.GetPagedByWorkflowAsync`/`GetPagedAsync` — server-side pagination with `pageSize` clamped to 100. Status filter forwarded to repository. Date range filter and trigger type filter deferred to API layer query parameters. | ||
|
|
There was a problem hiding this comment.
Add deferred follow-up entries to each Implementation status section.
These callouts include pending work but don’t add the mandated Deferred (PR #N follow-up) line. Please add it for each deferred block.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 83-94, 118-129
🧰 Tools
🪛 LanguageTool
[grammar] ~45-~45: Ensure spelling is correct
Context: ...ends) — not in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E06-workflow-engine/features/F04-execution-history.md` around
lines 45 - 57, Update each "Implementation status" callout that lists deferred
work to include a "Deferred (PR `#N` follow-up)" line; specifically add this to
the callout that mentions Filter UI, date/trigger query params, and
running-first sort pending API + Frontend (the block that references
GetExecutionsByWorkflowHandler and GetAllExecutionsHandler using
IExecutionRepository.GetPagedByWorkflowAsync/GetPagedAsync), and likewise add
the same deferred line to the other two implementation-status callouts
referenced in the comment (around the blocks at lines 83-94 and 118-129) so no
incomplete work is marked as ✅ without a follow-up PR reference.
Comment on lines
+47
to
59
| > **Implementation status** | ||
| > | ||
| > | Layer | Status | | ||
| > |-------|--------| | ||
| > | Domain | ✅ | | ||
| > | Application | ✅ | | ||
| > | Infrastructure | ✅ | | ||
| > | API | ✅ | | ||
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** `POST /api/executions/{id}/retry` and retry-with-context ✅. Retry UI and archived-definition warning pending Frontend. | ||
| > **Decisions:** `CreateRetry()` produces a new `WorkflowExecution` with `RetryOfExecutionId` set; context is copied from original at time of retry. | ||
|
|
There was a problem hiding this comment.
Include Deferred (PR #N follow-up) for deferred items in all three status callouts.
Each section marks work as pending but omits the required deferred follow-up line. Please add explicit deferred tracking entries to these callouts.
As per coding guidelines, "Add a Deferred (PR #N follow-up) line in the implementation status callout when deferring work; never silently mark incomplete work as ✅".
Also applies to: 84-95, 121-133
🧰 Tools
🪛 LanguageTool
[grammar] ~47-~47: Ensure spelling is correct
Context: ...tion) — not in MVP. > Implementation status > > | Layer | Status | > |-------|--------|...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E06-workflow-engine/features/F05-manual-retry.md` around lines 47
- 59, Update each "Implementation status" callout that contains any incomplete
work to include a deferred follow-up note: add the exact line "Deferred (PR `#N`
follow-up)" beneath the status table for the callout shown (the callout starting
with "**Implementation status**" and its status table), and do the same for the
two other matching callouts later in the document (the other "**Implementation
status**" sections referenced). Ensure the deferred line appears whenever a
non-✅ item or pending work is present (e.g., Frontend ⏳, archived-definition
warning), keeping the table and surrounding wording unchanged except for
inserting the Deferred (PR `#N` follow-up) line.
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/epics/E03-data-modeling/features/F01-model-definition.md`:
- Line 168: Replace the literal placeholder "PR `#N`" in the deferred note with a
concrete tracking reference: either the actual follow-up PR number (e.g., "PR
`#1234`") if that PR exists or a tracking issue ID (e.g., "ISSUE-567") if the work
will be created later; update the sentence mentioning ModelDeletedHandler and
WorkflowBuilder `record.*` trigger so it reads e.g. "Deferred (follow-up: PR
`#1234`)" or "Deferred (tracking: ISSUE-567)" to make the follow-up unambiguous
and searchable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 49881429-0c61-4a41-910e-fa9de25238ca
📒 Files selected for processing (6)
docs/epics/E01-platform-foundation/features/F01-tenant-registration.mddocs/epics/E01-platform-foundation/features/F02-organization-management.mddocs/epics/E01-platform-foundation/features/F04-subscription-plans.mddocs/epics/E03-data-modeling/features/F01-model-definition.mddocs/epics/README.mdscripts/normalize-feature-docs.py
✅ Files skipped from review due to trivial changes (3)
- docs/epics/E01-platform-foundation/features/F04-subscription-plans.md
- docs/epics/E01-platform-foundation/features/F02-organization-management.md
- docs/epics/E01-platform-foundation/features/F01-tenant-registration.md
| > | Frontend | ⏳ | | ||
| > | ||
| > **Gaps vs spec:** workflow reference check pending E04; form Relation Picker refs blocked/flagged via FormBuilder `ModelDeletedEvent` consumer (US-033 partial); 30-day purge background job pending. | ||
| > **Deferred (PR #N follow-up):** DataModeling relation fields on other models flagged broken when target model deleted. WorkflowBuilder `record.*` trigger broken flags shipped via `ModelDeletedHandler` (Kafka). |
There was a problem hiding this comment.
Replace placeholder "PR #N" with actual future PR number or tracking mechanism.
The deferred work notation uses a literal "PR #N" placeholder instead of a concrete reference. This makes it unclear how to track when this work will be completed. As per coding guidelines, when deferring work, you should reference a specific follow-up PR number or create a tracking issue.
📝 Suggested approaches
Option 1: If the follow-up PR doesn't exist yet, create a tracking issue:
-> **Deferred (PR `#N` follow-up):** DataModeling relation fields...
+> **Deferred (Issue `#NNN` follow-up):** DataModeling relation fields...Option 2: If you plan to address this in a known upcoming PR, reference it:
-> **Deferred (PR `#N` follow-up):** DataModeling relation fields...
+> **Deferred (PR `#130` follow-up):** DataModeling relation fields...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| > **Deferred (PR #N follow-up):** DataModeling relation fields on other models flagged broken when target model deleted. WorkflowBuilder `record.*` trigger broken flags shipped via `ModelDeletedHandler` (Kafka). | |
| > **Deferred (Issue `#NNN` follow-up):** DataModeling relation fields on other models flagged broken when target model deleted. WorkflowBuilder `record.*` trigger broken flags shipped via `ModelDeletedHandler` (Kafka). |
Suggested change
| > **Deferred (PR #N follow-up):** DataModeling relation fields on other models flagged broken when target model deleted. WorkflowBuilder `record.*` trigger broken flags shipped via `ModelDeletedHandler` (Kafka). | |
| > **Deferred (PR `#130` follow-up):** DataModeling relation fields on other models flagged broken when target model deleted. WorkflowBuilder `record.*` trigger broken flags shipped via `ModelDeletedHandler` (Kafka). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/epics/E03-data-modeling/features/F01-model-definition.md` at line 168,
Replace the literal placeholder "PR `#N`" in the deferred note with a concrete
tracking reference: either the actual follow-up PR number (e.g., "PR `#1234`") if
that PR exists or a tracking issue ID (e.g., "ISSUE-567") if the work will be
created later; update the sentence mentioning ModelDeletedHandler and
WorkflowBuilder `record.*` trigger so it reads e.g. "Deferred (follow-up: PR
`#1234`)" or "Deferred (tracking: ISSUE-567)" to make the follow-up unambiguous
and searchable.
7 tasks
This was referenced May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit-driven documentation sync after recent F02/F04 and E05/E06 retrofit work. All 29 feature files now use scannable Wireframes and Implementation status tables instead of cramped inline blockquotes.
PROGRESS.md, E01 README, and system diagrams are aligned with the codebase (React 19, E06 DB/migrations, F02/F04 backend complete). Addsscripts/normalize-feature-docs.pywith--checkmode (wired intocheck-doc-drift.sh), a copy-paste US template, and documented layer-status conventions so agents do not misread epic-levelAPI ✅vs per-US gaps.Follow-up review fixes in this PR iteration:
docs/epics/README.mdcommand table row.## Wireframessection in E01 F01 and normalized deferred label formats toDeferred (PR #N follow-up)where applicable.Gaps vs specinto a standalone deferred line in E01 F02.normalize-feature-docs.pyso deprecated-format detection reflects post-normalization state during write mode.Linked spec
Cross-cutting — docs governance (
docs/epics/README.md,docs/playbooks/docs-style.md,docs/PROGRESS.md,docs/epics/**/features/*.md,docs/epics/_template-feature-us.md).Requirements & rules followed
./scripts/check-doc-drift.shgreen (includesnormalize-feature-docs.py --check)Summary by CodeRabbit
Documentation
Infrastructure