feat: auto-generate design review documents from spec annotations#34
Merged
darko-mijic merged 12 commits intomainfrom Mar 13, 2026
Merged
feat: auto-generate design review documents from spec annotations#34darko-mijic merged 12 commits intomainfrom
darko-mijic merged 12 commits intomainfrom
Conversation
…ase A) Register 4 sequence tags (sequence-orchestrator, sequence-step, sequence-module, sequence-error) in the tag registry, add rule-level tags to BusinessRuleSchema, wire both sync/async extractors to propagate sequenceOrchestrator and rule tags, and add Input/Output marker extraction to parseBusinessRuleAnnotations. Includes annotated SetupCommand spec and hand-crafted reference design review document.
…command (Phase B) Add SequenceIndex pre-computed view to MasterDataset for design review diagram generation. Includes: SequenceStepSchema and SequenceIndexEntry schemas, sequence-utils.ts transform utilities that parse rule-level sequence tags and annotations, single-pass accumulation in transform-dataset.ts, errorScenarioNames on BusinessRuleSchema, and a process API 'sequence' subcommand. Also adds roadmap spec for DesignReviewGeneration (Phase 46).
…ams (Phase C) Creates the DesignReviewCodec that transforms MasterDataset.sequenceIndex into RenderableDocuments containing auto-generated Mermaid sequence diagrams, component diagrams with type hexagons, type definition tables, and design question templates. Wires the generator into the pipeline with docs:design-review script.
…e (Phase D) Adds 44 Gherkin-only tests covering the full DesignReviewGeneration pipeline: SequenceIndex building, step sorting, participant deduplication, data flow types, sequence diagram participants/alt blocks, component diagram grouping/hexagons, and design question auto-metrics. Transitions spec to active with all 8 deliverables marked complete.
…ion, type-safe tests - Add per-pattern try-catch in DesignReviewGenerator to prevent one bad annotation from killing all design review generation (H1) - Add sanitizeMermaidLabel() for diagram labels/notes/alt blocks to prevent Mermaid syntax corruption from special characters (M1) - Use fallback labels when Input/Output annotations are missing instead of producing empty arrow labels in sequence diagrams (M2) - Add sequenceOrchestrator to TestPatternOptions and remove unsafe Record<string, unknown> cast in test helper (M3) - Surface validation warning when pattern has sequence-orchestrator but no sequence-step tags on any rules (M4)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds sequence-oriented metadata, extraction, validation schemas, a SequenceIndex builder and dataset integration, a DesignReview codec and generator that emits markdown, a CLI Changes
Sequence Diagram(s)sequenceDiagram
participant Extractor as Gherkin Extractor
participant Transform as Transform Dataset
participant Index as Sequence Index Builder
participant Codec as DesignReview Codec
participant Generator as Design Review Generator
participant Renderer as Markdown Renderer
participant FS as File System
Extractor->>Transform: emit extracted patterns (with sequence metadata)
Transform->>Index: buildSequenceIndexEntry(orchestrator, rules)
Index-->>Transform: SequenceIndexEntry
Transform->>Generator: include sequenceIndex in MasterDataset
Generator->>Codec: decode(patternName, dataset)
Codec-->>Generator: RenderableDocument
Generator->>Renderer: renderToMarkdown(document)
Renderer->>FS: write design-reviews/{pattern}.md
FS-->>Generator: write confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderable/codecs/helpers.ts (1)
814-857:⚠️ Potential issue | 🟡 Minor
Input/Outputare stripped here, but they still leak into rendered rule text.This only updates
parseBusinessRuleAnnotations().renderRuleDescription()still rebuilds the stripped description independently and only removes**Invariant:**,**Rationale:**, and**Verified by:**, so rules with sequence annotations will still render literal**Input:**/**Output:**lines in ADR/business-rule output.Please either reuse
annotations.remainingContentthere or extend the later strip list to cover the new markers too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderable/codecs/helpers.ts` around lines 814 - 857, The parseBusinessRuleAnnotations() change strips **Input:**/**Output:** into annotations.remainingContent but renderRuleDescription() still rebuilds and only removes **Invariant:**, **Rationale:** and **Verified by:**, causing Input/Output lines to leak; fix by updating renderRuleDescription() to either reuse annotations.remainingContent (call parseBusinessRuleAnnotations() and use annotations.remainingContent instead of reconstructing the description) or add removal of /\*\*Input:\*\*\s*[\s\S]*?(?=\*\*[A-Z]|\*\*$|$)/i and /\*\*Output:\*\*\s*[\s\S]*?(?=\*\*[A-Z]|\*\*$|$)/i to the strip list so **Input:**/**Output:** are also stripped from the rendered rule text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@delivery-process/design-reviews/setup-command.md`:
- Around line 117-122: The component diagram groups distinct sequence steps
because groupStepsByInputType() in src/renderable/codecs/design-review.ts merges
contiguous steps by input type; update that function so it preserves
sequence-step isolation by creating a separate phase for every sequence-step
(use the step's sequence-step identifier or index instead of inputType when
partitioning), ensuring generate-config.ts, augment-package-json.ts,
scaffold-dirs.ts, and generate-example.ts become individual phases in the
component view; alternatively, if you prefer the current behavior, update the
component diagram header text to explicitly state that phases are grouped by
shared input type rather than by sequence-step.
In `@delivery-process/specs/setup-command.feature`:
- Line 10: Change the orchestrator tag string
"@libar-docs-sequence-orchestrator:init-cli" to use the deliverable basename
"init" (i.e., "@libar-docs-sequence-orchestrator:init") so the sequence resolver
(which matches by deliverable basename like "init" -> init.ts) selects the
actual entry point; update any occurrences of the old tag to the new tag in the
feature file.
In `@src/cli/process-api.ts`:
- Around line 1003-1015: The no-arg branch in handleSequence currently returns
raw { patterns, count } and bypasses global output modifiers, so flags like
--count and --names-only are ignored; change the no-argument path in
handleSequence to route the result through the same modifier handling used
elsewhere (consult ctx.modifiers or the existing modifier-processing utility) so
modifiers are applied, or explicitly detect and reject unsupported modifiers
with a clear error; apply the same fix to the other occurrence referenced around
lines 1518-1519 to ensure consistent behavior.
In `@src/extractor/gherkin-extractor.ts`:
- Around line 341-342: Sync and async extraction produce different pattern
payloads because the raw-pattern construction is duplicated; pull the shared
raw-pattern building logic into a helper (e.g., buildRawPatternFromMetadata or
constructRawPattern) and use it from both extractPatternsFromGherkinAsync() and
the sync extractor so both paths set the same fields (ensure the helper assigns
effortActual, adrTheme, adrLayer, the claude* fields, sequenceOrchestrator, etc.
using the existing helpers like assignIfDefined). Replace the duplicated
construction in both branches with a single call to this helper, returning the
constructed rawPattern object so subsequent transformation code remains
unchanged.
In `@src/generators/built-in/codec-generators.ts`:
- Line 31: The file imports createDesignReviewGenerator (DesignReviewGenerator)
but the file header lacks a `@libar-docs-uses` relationship tag; update the
TypeScript file header comment to include a `@libar-docs-uses`:
DesignReviewGenerator (or createDesignReviewGenerator) entry so API-driven
discovery can see the dependency, and add the same annotation for the related
section referenced around the other generator usages (lines shown in review,
e.g., the block at 183-188) so both the top-of-file metadata and the additional
generator-related header blocks include `@libar-docs-uses` for
DesignReviewGenerator.
In `@src/renderable/codecs/design-review.ts`:
- Around line 375-383: The loop currently only emits module→orchestrator edges
when step.output contains the '--' type separator; change it to emit an edge
whenever step.output exists and reserve the '--' check only for creating a typed
node. Concretely: inside the for (const step of entry.steps) loop, replace the
initial if (step.output?.includes('--') !== true) continue with if
(!step.output) continue, then for each mod (using sanitizeNodeId(mod) as before)
push an edge for every output (e.g. lines.push(` ${modId}
-->|"${step.output}"| ${orchId}`)), and additionally, if
step.output.includes('--') === true call extractTypeName(step.output) and
push/use the typed label where the diagram expects a type node; leave
sanitizeNodeId, extractTypeName, orchId and lines.push usage intact.
In `@src/validation-schemas/master-dataset.ts`:
- Around line 213-251: The schemas allow empty strings and empty arrays; update
SequenceStepSchema, SequenceIndexEntrySchema and SequenceIndexSchema to enforce
trimmed non-empty strings and non-empty arrays: change string fields like
ruleName, modules' items, errorScenarios, orchestrator, participants,
errorPaths, dataFlowTypes, input/output/invariant (if required) to use trimmed
non-empty checks (e.g., z.string().trim().min(1)) and change arrays like
modules, steps, participants, errorScenarios, errorPaths, dataFlowTypes to
require at least one element (e.g., z.array(...).min(1).readonly()) so
invalid/empty sequence metadata is rejected at validation time. Ensure
SequenceStepSchema, SequenceIndexEntrySchema and SequenceIndexSchema are updated
accordingly.
In `@tests/features/generation/design-review.feature`:
- Around line 87-91: The feature repeats the step text "the rendered markdown
contains {string}" twice which causes duplicate step-pattern issues; replace the
two Then/And lines with a single step using a docstring/DataTable (e.g. Then
'the rendered markdown contains:' followed by a newline-separated block of
expected strings) and update the step-definition that matches the pattern "the
rendered markdown contains" to accept the docstring, split it into lines,
trim/filter empties and assert each line is contained in the stored markdown
(use the existing state accessor like requireState and s.markdown in the step
handler).
In `@tests/steps/generation/design-review.steps.ts`:
- Around line 1-16: Add an `@libar-docs-uses` metadata tag to the existing module
JSDoc at the top of the "Design Review Generation Step Definitions" file so the
process API can discover this step-definition module; include the relevant
relationship identifiers such as SequenceIndex, DesignReviewCodec, orchestrator,
and any other internal components this module uses (e.g., participant
deduplication, stepNumber sorting, data flow extraction) in a comma-separated
`@libar-docs-uses` entry and place it inside the existing block comment (near the
header text) following the project's `@libar-docs` annotation conventions.
In `@tests/support/helpers/design-review-state.ts`:
- Around line 1-8: This file has `@libar-docs` but lacks relationship metadata;
add an `@libar-docs-uses` tag to the module header listing the modules this helper
depends on (for example SequenceIndex and DesignReviewCodec or other fixture
builders referenced by the helpers) so the helper becomes discoverable in API
queries; update the top-of-file comment block (the existing doc comment) to
include a line like "@libar-docs-uses <ModuleName1>, <ModuleName2>" referencing
the actual symbols this test helper consumes.
---
Outside diff comments:
In `@src/renderable/codecs/helpers.ts`:
- Around line 814-857: The parseBusinessRuleAnnotations() change strips
**Input:**/**Output:** into annotations.remainingContent but
renderRuleDescription() still rebuilds and only removes **Invariant:**,
**Rationale:** and **Verified by:**, causing Input/Output lines to leak; fix by
updating renderRuleDescription() to either reuse annotations.remainingContent
(call parseBusinessRuleAnnotations() and use annotations.remainingContent
instead of reconstructing the description) or add removal of
/\*\*Input:\*\*\s*[\s\S]*?(?=\*\*[A-Z]|\*\*$|$)/i and
/\*\*Output:\*\*\s*[\s\S]*?(?=\*\*[A-Z]|\*\*$|$)/i to the strip list so
**Input:**/**Output:** are also stripped from the rendered rule text.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b5ca0e0-bb8f-4231-8ff0-3d51d54a5910
⛔ Files ignored due to path filters (2)
delivery-process.config.tsis excluded by none and included by nonepackage.jsonis excluded by none and included by none
📒 Files selected for processing (19)
delivery-process/design-reviews/setup-command.mddelivery-process/specs/design-review-generation.featuredelivery-process/specs/setup-command.featuresrc/cli/process-api.tssrc/extractor/gherkin-extractor.tssrc/generators/built-in/codec-generators.tssrc/generators/built-in/design-review-generator.tssrc/generators/pipeline/sequence-utils.tssrc/generators/pipeline/transform-dataset.tssrc/renderable/codecs/design-review.tssrc/renderable/codecs/helpers.tssrc/scanner/gherkin-ast-parser.tssrc/taxonomy/registry-builder.tssrc/validation-schemas/extracted-pattern.tssrc/validation-schemas/master-dataset.tstests/features/generation/design-review.featuretests/fixtures/pattern-factories.tstests/steps/generation/design-review.steps.tstests/support/helpers/design-review-state.ts
darko-mijic
changed the title
darko-mijic
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/generators/built-in/design-review-generator.ts`:
- Around line 62-64: The empty catch block in the annotated-pattern processing
loop in design-review-generator.ts swallows decode/render errors, violating the
GeneratorOutput contract; update that catch to either log the error and re-throw
it so the orchestrator can handle failures, or accumulate per-pattern errors
into an array and after the loop throw an AggregateError (or a single Error)
summarizing failed patterns; target the catch that currently swallows exceptions
around the annotated pattern decode/render logic and ensure the generator throws
on failure in accordance with GeneratorOutput and orchestrator expectations.
In `@tests/steps/generation/design-review.steps.ts`:
- Around line 346-354: There are duplicate step definitions using the same
pattern ('the rendered markdown contains {string}') implemented as Then(...) and
And(...); remove the redundant definition and keep a single step handler (either
the Then or And invocation) so the step pattern is defined only once, making
sure the kept handler still calls requireState(state) and asserts
expect(s.markdown).toContain(expected).
In `@tests/support/helpers/design-review-state.ts`:
- Around line 26-47: The DesignReviewState interface uses mixed nullability
(entry: SequenceIndexEntry | undefined vs dataset: MasterDataset | null and doc:
RenderableDocument | null); pick one convention and make the types consistent
(e.g., change dataset and doc to MasterDataset | undefined and
RenderableDocument | undefined, or change entry to SequenceIndexEntry | null),
then update any code/tests that read or assign
DesignReviewState.{entry,dataset,doc} to use the chosen sentinel (undefined or
null) consistently to avoid runtime checks mismatches.
- Around line 115-123: createPlainRule currently omits the errorScenarioNames
field which createSequenceRule sets to an empty array; update createPlainRule to
include errorScenarioNames: [] so rules without errors have the same shape as
those from createSequenceRule, ensuring consumers expecting errorScenarioNames
won't break (locate and modify the createPlainRule function to add the
errorScenarioNames property).
- Around line 136-161: The call to buildEntry() in generateDesignReview is
unnecessary because state.entry is never used and createTestMasterDataset
already builds the sequenceIndex via the transform pipeline; remove the
conditional block that checks state.entry and calls buildEntry(state). Leave the
rest intact (pattern creation with createTestPattern, dataset via
createTestMasterDataset, and codec decode/render steps). If a sequence entry is
later required, obtain it from the dataset using resolveSequenceEntry(dataset,
...) rather than prebuilding with buildEntry.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b1f28061-2e4f-42f0-9234-a00570b072d1
📒 Files selected for processing (9)
delivery-process/design-reviews/setup-command.mdsrc/api/pattern-helpers.tssrc/cli/process-api.tssrc/generators/built-in/design-review-generator.tssrc/generators/pipeline/sequence-utils.tssrc/renderable/codecs/design-review.tstests/features/generation/design-review.featuretests/steps/generation/design-review.steps.tstests/support/helpers/design-review-state.ts
darko-mijic
changed the title
…lent swallow Add console.warn with pattern name and error message when a design review fails to generate, replacing the empty catch block. Add explanatory comment for the Mermaid `--` → em-dash sanitization. Regenerate setup-command.md to align committed output with generator truth.
renderTable now pads cells and separators to column width, producing output that prettier won't reformat. Eliminates spurious diffs when regenerating docs. Updates test expectations to match padded format.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/renderable/codecs/design-review.ts (1)
381-388:⚠️ Potential issue | 🟠 MajorDon't drop component edges for non-typed outputs.
Line 383 still suppresses every module → orchestrator edge unless the annotation uses the
Type -- fieldsform. Plain-text outputs are valid too, so the component diagram becomes incomplete for those steps.🩹 Suggested fix
- // Edges: module outputs → orchestrator (only for proper types with -- field separator) + // Edges: module outputs → orchestrator for (const step of entry.steps) { - if (step.output?.includes('--') !== true) continue; - const outputType = extractTypeName(step.output); + if (!step.output) continue; + const outputType = extractTypeName(step.output) || 'result'; for (const mod of step.modules) { const modId = sanitizeNodeId(mod); lines.push(` ${modId} -->|${quoteMermaidText(outputType)}| ${orchId}`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderable/codecs/design-review.ts` around lines 381 - 388, The loop over entry.steps currently drops all module→orchestrator edges when step.output doesn't include '--'; instead only skip when there is no output at all and treat plain-text outputs as valid labels. Change the logic in the entry.steps loop to: if step.output is falsy then continue; if step.output.includes('--') use extractTypeName(step.output) as outputType else use step.output directly (or a sanitized/quoted variant) before calling sanitizeNodeId(mod), quoteMermaidText(outputType) and pushing the line with orchId via lines.push; keep references to extractTypeName, sanitizeNodeId, quoteMermaidText, orchId and the lines.push call to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/renderable/codecs/design-review.ts`:
- Around line 381-388: The loop over entry.steps currently drops all
module→orchestrator edges when step.output doesn't include '--'; instead only
skip when there is no output at all and treat plain-text outputs as valid
labels. Change the logic in the entry.steps loop to: if step.output is falsy
then continue; if step.output.includes('--') use extractTypeName(step.output) as
outputType else use step.output directly (or a sanitized/quoted variant) before
calling sanitizeNodeId(mod), quoteMermaidText(outputType) and pushing the line
with orchId via lines.push; keep references to extractTypeName, sanitizeNodeId,
quoteMermaidText, orchId and the lines.push call to locate and update the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb0c7bcc-0a2d-40e0-85d7-29ef2647fabd
📒 Files selected for processing (2)
src/generators/built-in/design-review-generator.tssrc/renderable/codecs/design-review.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/steps/behavior/cli/process-api-reference.steps.ts (1)
101-110:⚠️ Potential issue | 🟠 MajorHeader assertions are now too permissive and may miss regressions.
These checks only assert
| ${cX}appears somewhere in content. That can match non-header lines, so header breakages can slip through undetected.Proposed fix (scope assertions to the section header row)
+function escapeRegExp(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +function extractTableHeader(content: string, afterHeading: string): string { + const lines = content.split('\n'); + let inSection = false; + for (const line of lines) { + if (line.startsWith('## ') && line.includes(afterHeading)) { + inSection = true; + continue; + } + if (inSection && line.startsWith('## ')) break; + if (inSection && line.startsWith('| ')) return line; + } + return ''; +}- expect(content).toContain(`| ${c1}`); - expect(content).toContain(`| ${c2}`); - expect(content).toContain(`| ${c3}`); - expect(content).toContain(`| ${c4}`); + const header = extractTableHeader(content, 'Global Options'); + expect(header).toMatch(new RegExp(`\\|\\s*${escapeRegExp(c1)}\\s*\\|`)); + expect(header).toMatch(new RegExp(`\\|\\s*${escapeRegExp(c2)}\\s*\\|`)); + expect(header).toMatch(new RegExp(`\\|\\s*${escapeRegExp(c3)}\\s*\\|`)); + expect(header).toMatch(new RegExp(`\\|\\s*${escapeRegExp(c4)}\\s*\\|`));Also applies to: 139-145, 177-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/steps/behavior/cli/process-api-reference.steps.ts` around lines 101 - 110, The header assertions in the step "the output contains a table with columns {string}, {string}, {string}, {string}" are too permissive because they search the whole content; change the step to locate the table header row (use getContent() to split into lines, find a line that is a header by detecting a following separator line that matches /^\|\s*[-:]+\s*(\|\s*[-:]+\s*)+$/ or similar) and then assert the header line contains the exact column names (check that the header line includes `| ${c1} |`, `| ${c2} |`, `| ${c3} |`, `| ${c4} |` or equivalent padding-tolerant patterns) so the checks only apply to the actual table header; update the same logic for the other similar steps referenced (the other step definitions at the same pattern).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderable/render.ts`:
- Around line 367-373: The code pads short rows but doesn't truncate overlong
rows, causing table rows to exceed the header/separator count; update the
normalization of block.rows inside the escapedRows computation (and any later
renderer that currently iterates raw rows at render time) to first truncate each
row to block.columns.length (e.g., slice or set length) and then pad if shorter,
then map escapeTableCell over that normalized row so both escapedRows and the
rendering loop (referencing escapedRows and escapeTableCell) use the same
fixed-width rows matching block.columns.
In `@tests/steps/behavior/render-output.steps.ts`:
- Line 177: Replace the fragile exact-string lookup using
state!.markdown.indexOf('| A | B |') with a whitespace-tolerant regex
search: run /\|\s*A\s*\|\s*B\s*\|/.exec(state!.markdown) (or use string.search
with that regex) and derive tableMarker from the match index (e.g.
tableMarkerMatch ? tableMarkerMatch.index : -1) so the test tolerates
non-semantic spacing; update any subsequent uses of tableMarker accordingly.
---
Outside diff comments:
In `@tests/steps/behavior/cli/process-api-reference.steps.ts`:
- Around line 101-110: The header assertions in the step "the output contains a
table with columns {string}, {string}, {string}, {string}" are too permissive
because they search the whole content; change the step to locate the table
header row (use getContent() to split into lines, find a line that is a header
by detecting a following separator line that matches
/^\|\s*[-:]+\s*(\|\s*[-:]+\s*)+$/ or similar) and then assert the header line
contains the exact column names (check that the header line includes `| ${c1}
|`, `| ${c2} |`, `| ${c3} |`, `| ${c4} |` or equivalent padding-tolerant
patterns) so the checks only apply to the actual table header; update the same
logic for the other similar steps referenced (the other step definitions at the
same pattern).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 93a4a96a-dd00-4bb1-8756-856b6047e1a1
📒 Files selected for processing (4)
src/renderable/render.tstests/features/behavior/render-blocks.featuretests/steps/behavior/cli/process-api-reference.steps.tstests/steps/behavior/render-output.steps.ts
…anup Component-diagram nodes are now phase-scoped (phase_N_ prefix) to prevent Mermaid ID collisions when the same module appears in non-contiguous phases. Invalid sequence annotations (duplicate step numbers, missing modules) are excluded from sequenceIndex and reported as malformedPatterns instead of producing misleading diagrams. The generator now detects orphaned design-reviews/*.md files and reports them via filesToDelete.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/renderable/codecs/design-review.ts (3)
384-395:⚠️ Potential issue | 🟠 MajorRender module outputs even when they are prose, not typed models.
The
step.output?.includes('--')guard drops real side effects from the component diagram. Insetup-command, that removes the return edges for config generation, package.json augmentation, scaffolding, and example generation, so Phase 3 looks write-only from the orchestrator's perspective.🩹 Suggested fix
- if (step.output?.includes('--') !== true) continue; - const outputType = extractTypeName(step.output); + if (!step.output) continue; + const outputLabel = step.output.includes('--') + ? extractTypeName(step.output) + : step.output; for (const mod of step.modules) { const modId = getPhaseModuleNodeId(i, mod); - lines.push(` ${modId} -->|${quoteMermaidText(outputType)}| ${orchId}`); + lines.push(` ${modId} -->|${quoteMermaidText(outputLabel)}| ${orchId}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderable/codecs/design-review.ts` around lines 384 - 395, The current guard step.output?.includes('--') prevents emitting edges for prose outputs; remove that filter so module→orchestrator edges are always emitted. In the loop over phase.steps (symbols: phases, step, getPhaseModuleNodeId, orchId, lines.push), compute the label conditionally: if step.output includes '--' then use extractTypeName(step.output) else use the raw step.output (trimmed) and pass it through quoteMermaidText for safe rendering, then push the same `${modId} -->|...| ${orchId}` line so prose side-effect outputs are shown.
613-628:⚠️ Potential issue | 🟠 MajorCollect structured
Input:types here too.This helper feeds the hexagon nodes, the key type table, and the DQ-2/summary type counts, but it only scans
step.output. A request type declared only in**Input:**disappears from all three sections even though the component diagram still shows it on the dispatch edge.🩹 Suggested fix
function collectTypeDefs(steps: readonly SequenceStep[]): TypeDef[] { const seen = new Set<string>(); const defs: TypeDef[] = []; for (const step of steps) { - if (!step.output) continue; - const dashIndex = step.output.indexOf('--'); - if (dashIndex < 0) continue; // Skip description-style outputs - - const name = step.output.slice(0, dashIndex).trim(); - if (seen.has(name)) continue; - seen.add(name); - - const rawFields = step.output.slice(dashIndex + 2).trim(); - defs.push({ name, fields: rawFields.length > 0 ? rawFields : undefined }); + for (const annotation of [step.input, step.output]) { + if (!annotation) continue; + const dashIndex = annotation.indexOf('--'); + if (dashIndex < 0) continue; // Skip description-style annotations + + const name = annotation.slice(0, dashIndex).trim(); + if (seen.has(name)) continue; + seen.add(name); + + const rawFields = annotation.slice(dashIndex + 2).trim(); + defs.push({ name, fields: rawFields.length > 0 ? rawFields : undefined }); + } } return defs; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderable/codecs/design-review.ts` around lines 613 - 628, collectTypeDefs currently only scans SequenceStep.output and thus misses types declared in an Input: section; update the function collectTypeDefs to also examine SequenceStep.input (or the raw step text where Input: appears), parse the same "name -- fields" pattern from inputs as well as outputs, deduplicate using the same seen set, and push TypeDef entries just like for outputs (use the same logic for dashIndex, name, rawFields and fields). Ensure you reference the same TypeDef shape and seen/set logic so input-declared types appear in the hexagon nodes, key type table, and DQ-2/summary counts.
316-319:⚠️ Potential issue | 🟡 MinorDescribe the actual grouping rule here.
The caption says the component view is grouped by
sequence-step, but the implementation callsgroupStepsByInputType(). That mismatch is already visible indelivery-process/design-reviews/setup-command.md, where one phase merges several distinct steps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderable/codecs/design-review.ts` around lines 316 - 319, The caption claims grouping is by `sequence-step` but the code actually groups via `groupStepsByInputType()`; update the wording or the implementation so they match: either change the paragraph text to accurately describe the grouping rule produced by groupStepsByInputType (explain that steps are grouped by input type/shape and how merged phases occur), or replace/rename the grouping call to use the actual sequence-step grouping logic (or call the function that groups by `sequence-step`) so the caption and the implemented grouping are consistent; reference `groupStepsByInputType()` and the paragraph text near the component caption to locate the discrepancy.tests/support/helpers/design-review-state.ts (2)
1-8: 🛠️ Refactor suggestion | 🟠 MajorAdd
@libar-docs-usesrelationship metadata to this helper module.The module opts into
@libar-docsbut omits usage relationships, which reduces discoverability in API/tag queries.Proposed header update
/** * Shared state and helpers for design review generation tests. * * Provides test state management and fixture builders for testing * the SequenceIndex builder and DesignReviewCodec pipeline. * * `@libar-docs` + * `@libar-docs-uses` pattern-helpers, sequence-utils, transform-dataset, design-review-codec, render, tag-registry, dataset-factories */As per coding guidelines,
**/*.{ts,tsx}should “Use@libar-docsannotations on TypeScript files and add relationship tags (@libar-docs-depends-on,@libar-docs-uses)” and**/*.{ts,tsx,feature}requires@libar-docs-useson TypeScript only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/support/helpers/design-review-state.ts` around lines 1 - 8, The file header has an `@libar-docs` annotation but lacks relationship metadata; update the top-of-file comment in the design-review-state module to include an `@libar-docs-uses` tag listing the things this helper uses (e.g., SequenceIndex, DesignReviewCodec, and any fixture builders or helpers exported/consumed by this module) so API/tag queries can surface its usage relationships; locate the module header comment (the block starting with /** and the existing `@libar-docs`) and add a concise `@libar-docs-uses` line naming those symbols.
146-149: 🧹 Nitpick | 🔵 TrivialRemove the unused pre-build of
state.entryingenerateDesignReview.
buildEntry(state)is called, butstate.entryis not used by this function.createTestMasterDatasetalready builds the sequence index path used by the codec.Proposed simplification
export function generateDesignReview(state: DesignReviewState): void { - // First build the entry if not already done - if (state.entry === undefined && state.rules.length > 0) { - buildEntry(state); - } - // Create a pattern with sequence orchestrator and rules const pattern = createTestPattern({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/support/helpers/design-review-state.ts` around lines 146 - 149, In generateDesignReview remove the unused pre-build call: delete the conditional block that calls buildEntry(state) when state.entry === undefined, since state.entry is never used there and createTestMasterDataset already constructs the required sequence index path; leave createTestMasterDataset and any other buildEntry callers intact and run tests to confirm no behavior change.tests/features/generation/design-review.feature (1)
97-98:⚠️ Potential issue | 🟠 MajorConsolidate duplicate
the rendered markdown contains {string}assertions into one docstring-based step.This repeats the same step pattern in a scenario, which is brittle with
vitest-cucumber. Use one multiline step and assert each line in the step definition.Proposed feature-file change
- Then the rendered markdown contains "alt Config not found" - And the rendered markdown contains "alt Invalid preset" + Then the rendered markdown contains: + """ + alt Config not found + alt Invalid preset + """ @@ - Then the rendered markdown contains "module|"alpha.ts" - And the rendered markdown contains "Config "Draft" | Preview % % comment" + Then the rendered markdown contains: + """ + module|"alpha.ts" + Config "Draft" | Preview % % comment + """As per coding guidelines,
**/*.{feature,steps.ts}should “Consolidate multipleAndsteps with identical text into a single step with DataTable or docstring; multiple identical patterns fail in vitest-cucumber.”Also applies to: 155-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/features/generation/design-review.feature` around lines 97 - 98, The feature repeats the step pattern "the rendered markdown contains {string}" multiple times; consolidate them into a single docstring-based step (e.g. "Then the rendered markdown contains:" followed by a multiline string) and update the matching step definition for the "the rendered markdown contains {string}" pattern to accept a docstring (or DataTable) and iterate over each line asserting its presence in the rendered markdown; find the existing step implementation for the "the rendered markdown contains {string}" pattern and change it to loop over docstring lines and assert each line instead of relying on duplicate step calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/generators/pipeline/sequence-utils.ts`:
- Around line 49-53: The current sequence-step parsing in sequence-utils.ts uses
parseInt on tag.split(':')[1] which accepts malformed values like "1foo" and
allows 0/negative numbers; replace that logic in the
tag.startsWith('sequence-step:') branch by extracting the raw token (e.g., const
token = tag.split(':')[1] ?? ''), validate it against /^[1-9]\d*$/ and only then
convert to a number (Number(token)) and assign to step; if the token fails the
regex, do not set step and instead surface an issue via the existing issues
collection (include the offending tag and mention SequenceStepSchema.stepNumber
must be a positive integer).
In `@src/renderable/codecs/design-review.ts`:
- Around line 271-275: The note block currently uses step.ruleName (in the loop
over entry.steps where lines.push is called and sanitizeMermaidRawText is
applied) but must render the rule contract stored in step.invariant; update the
lines.push call that builds the Mermaid note (inside the for (const step of
entry.steps) loop) to include step.invariant (sanitized with
sanitizeMermaidRawText) instead of or appended to step.ruleName so the Note over
${orchId}: shows the invariant text preserved by
parseSequenceAnnotations/SequenceStepSchema rather than repeating the rule
title.
In `@tests/steps/generation/design-review-generator.steps.ts`:
- Around line 1-7: The module header uses `@libar-docs` but is missing
relationship metadata, so add an appropriate `@libar-docs-uses` entry to the
file-level JSDoc (and `@libar-docs-depends-on` if relevant) to declare which
generator/test or other modules this step-definition consumes; update the top
comment block in design-review-generator.steps.ts to include lines like
`@libar-docs-uses` <identifier(s)> referencing the generator or test module names
used by this step so the process/API can discover the linkage via metadata.
---
Duplicate comments:
In `@src/renderable/codecs/design-review.ts`:
- Around line 384-395: The current guard step.output?.includes('--') prevents
emitting edges for prose outputs; remove that filter so module→orchestrator
edges are always emitted. In the loop over phase.steps (symbols: phases, step,
getPhaseModuleNodeId, orchId, lines.push), compute the label conditionally: if
step.output includes '--' then use extractTypeName(step.output) else use the raw
step.output (trimmed) and pass it through quoteMermaidText for safe rendering,
then push the same `${modId} -->|...| ${orchId}` line so prose side-effect
outputs are shown.
- Around line 613-628: collectTypeDefs currently only scans SequenceStep.output
and thus misses types declared in an Input: section; update the function
collectTypeDefs to also examine SequenceStep.input (or the raw step text where
Input: appears), parse the same "name -- fields" pattern from inputs as well as
outputs, deduplicate using the same seen set, and push TypeDef entries just like
for outputs (use the same logic for dashIndex, name, rawFields and fields).
Ensure you reference the same TypeDef shape and seen/set logic so input-declared
types appear in the hexagon nodes, key type table, and DQ-2/summary counts.
- Around line 316-319: The caption claims grouping is by `sequence-step` but the
code actually groups via `groupStepsByInputType()`; update the wording or the
implementation so they match: either change the paragraph text to accurately
describe the grouping rule produced by groupStepsByInputType (explain that steps
are grouped by input type/shape and how merged phases occur), or replace/rename
the grouping call to use the actual sequence-step grouping logic (or call the
function that groups by `sequence-step`) so the caption and the implemented
grouping are consistent; reference `groupStepsByInputType()` and the paragraph
text near the component caption to locate the discrepancy.
In `@tests/features/generation/design-review.feature`:
- Around line 97-98: The feature repeats the step pattern "the rendered markdown
contains {string}" multiple times; consolidate them into a single
docstring-based step (e.g. "Then the rendered markdown contains:" followed by a
multiline string) and update the matching step definition for the "the rendered
markdown contains {string}" pattern to accept a docstring (or DataTable) and
iterate over each line asserting its presence in the rendered markdown; find the
existing step implementation for the "the rendered markdown contains {string}"
pattern and change it to loop over docstring lines and assert each line instead
of relying on duplicate step calls.
In `@tests/support/helpers/design-review-state.ts`:
- Around line 1-8: The file header has an `@libar-docs` annotation but lacks
relationship metadata; update the top-of-file comment in the design-review-state
module to include an `@libar-docs-uses` tag listing the things this helper uses
(e.g., SequenceIndex, DesignReviewCodec, and any fixture builders or helpers
exported/consumed by this module) so API/tag queries can surface its usage
relationships; locate the module header comment (the block starting with /** and
the existing `@libar-docs`) and add a concise `@libar-docs-uses` line naming those
symbols.
- Around line 146-149: In generateDesignReview remove the unused pre-build call:
delete the conditional block that calls buildEntry(state) when state.entry ===
undefined, since state.entry is never used there and createTestMasterDataset
already constructs the required sequence index path; leave
createTestMasterDataset and any other buildEntry callers intact and run tests to
confirm no behavior change.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f49197e7-7b0d-4eab-b803-5ba3c44f784d
📒 Files selected for processing (10)
delivery-process/design-reviews/setup-command.mdsrc/generators/built-in/design-review-generator.tssrc/generators/pipeline/sequence-utils.tssrc/generators/pipeline/transform-dataset.tssrc/renderable/codecs/design-review.tstests/features/generation/design-review-generator.featuretests/features/generation/design-review.featuretests/steps/generation/design-review-generator.steps.tstests/steps/generation/design-review.steps.tstests/support/helpers/design-review-state.ts
…port, annotations - Sequence schema: add .trim().min(1) constraints to string fields, .min(1) to required arrays (SequenceStepSchema, SequenceIndexEntrySchema) - handleSequence: pass OutputModifiers, apply --count/--names-only in list mode - Note blocks: render step.invariant ?? step.ruleName for richer diagrams - Consolidate duplicate Then/And steps into single docstring pattern - Add errorScenarioNames: [] to createPlainRule for shape consistency - Add @libar-docs-uses annotations to 4 files for API discoverability
This was referenced Mar 14, 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
1 participant
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
Design reviews for new features require Mermaid sequence and component diagrams that visualize how modules interact at runtime. Creating these by hand is slow (~30 min per pattern), error-prone, and the diagrams drift every time the spec changes.
This PR adds a generation pipeline that reads structured annotations from Gherkin spec files and produces design review documents with:
Output lands in
delivery-process/design-reviews/{pattern-name}.md, auto-discovered from annotations with no config flags needed.How It Works
Annotation Convention
Authors annotate their Gherkin specs with four tags:
@libar-docs-sequence-orchestrator@libar-docs-sequence-step@libar-docs-sequence-module@libar-docs-sequence-errorAdditionally,
**Input:**and**Output:**markers in Rule descriptions define data flow types for diagram edges.Pipeline Architecture
The pipeline fits into the existing four-stage architecture:
SequenceIndexpre-computed view onMasterDataset(single-pass, alongside existingarchIndex)SequenceIndexentries into Mermaid diagrams, type tables, and design questionsExample: Generated Design Review
The
SetupCommandspec is the first pattern annotated with sequence tags. Runningpnpm docs:design-reviewproducesdelivery-process/design-reviews/setup-command.mdcontaining:Sequence Diagram — Runtime Interaction Flow
sequenceDiagram participant User participant init_cli as "init-cli.ts" participant detect_context as "detect-context.ts" participant prompts as "prompts.ts" participant generate_config as "generate-config.ts" participant augment_package_json as "augment-package-json.ts" participant scaffold_dirs as "scaffold-dirs.ts" participant generate_example as "generate-example.ts" participant validate_setup as "validate-setup.ts" User->>init_cli: invoke Note over init_cli: Rule 1 — Init detects existing project context before making changes init_cli->>+detect_context: targetDir: string detect_context-->>-init_cli: ProjectContext alt Fails gracefully when no package.json exists init_cli-->>User: error init_cli->>init_cli: exit(1) end Note over init_cli: Rule 2 — Interactive prompts configure preset and source paths init_cli->>+prompts: ProjectContext prompts-->>-init_cli: InitConfig alt Non-interactive mode refuses to overwrite existing config init_cli-->>User: error init_cli->>init_cli: exit(1) end Note over init_cli: Rule 3 — Generated config file uses defineConfig with correct imports init_cli->>+generate_config: InitConfig generate_config-->>-init_cli: delivery-process.config.ts written Note over init_cli: Rule 4 — Npm scripts are injected using bin command names init_cli->>+augment_package_json: InitConfig augment_package_json-->>-init_cli: package.json updated Note over init_cli: Rule 5 — Directory structure and example enable immediate first run init_cli->>+scaffold_dirs: InitConfig scaffold_dirs-->>-init_cli: directories created init_cli->>+generate_example: InitConfig generate_example-->>-init_cli: example annotated .ts file Note over init_cli: Rule 6 — Init validates the complete setup by running the pipeline init_cli->>+validate_setup: targetDir: string validate_setup-->>-init_cli: SetupResult alt Failed validation prints diagnostic information init_cli-->>User: error init_cli->>init_cli: exit(1) endComponent Diagram — Types and Data Flow
graph LR subgraph phase_1["Phase 1: targetDir: string"] detect_context["detect-context.ts"] end subgraph phase_2["Phase 2: ProjectContext"] prompts["prompts.ts"] end subgraph phase_3["Phase 3: InitConfig"] generate_config["generate-config.ts"] augment_package_json["augment-package-json.ts"] scaffold_dirs["scaffold-dirs.ts"] generate_example["generate-example.ts"] end subgraph phase_4["Phase 4: targetDir: string"] validate_setup["validate-setup.ts"] end subgraph orchestrator["Orchestrator"] init_cli["init-cli.ts"] end subgraph types["Key Types"] ProjectContext{{"ProjectContext\n-----------\npackageJsonPath\npackageJson\ntsconfigExists\ntsconfigModuleResolution\nexistingConfigPath\nisMonorepo\nhasEsmType"}} InitConfig{{"InitConfig\n-----------\ntargetDir\npreset\nsources\nforce\ncontext"}} SetupResult{{"SetupResult\n-----------\nsuccess\npatternCount\ndiagnostics"}} end detect_context -->|"ProjectContext"| init_cli prompts -->|"InitConfig"| init_cli validate_setup -->|"SetupResult"| init_cli init_cli -->|"targetDir: string"| detect_context init_cli -->|"ProjectContext"| prompts init_cli -->|"InitConfig"| generate_config init_cli -->|"InitConfig"| augment_package_json init_cli -->|"InitConfig"| scaffold_dirs init_cli -->|"InitConfig"| generate_example init_cli -->|"targetDir: string"| validate_setupType Definition Table + Design Questions
Key Type Definitions:
ProjectContextInitConfigSetupResultDesign Questions (auto-computed):
What Changed
Tag Registration + Extraction
sequence-orchestrator,sequence-step,sequence-module,sequence-error) in the tag registrytagsanderrorScenarioNamestoBusinessRuleSchema;sequenceOrchestratortoExtractedPatternSchema**Input:**/**Output:**marker extraction toparseBusinessRuleAnnotationsMasterDataset Integration
SequenceStepSchema,SequenceIndexEntrySchema, andSequenceIndexSchemainmaster-dataset.tssequence-utils.tswith tag parsing, annotation extraction, and index buildingsequenceIndexin the single-pass transform loop alongsidearchIndexsequencesubcommand to Process API CLICodec + Generator
DesignReviewCodecwith factory pattern matching existing codec architectureDesignReviewGeneratorImplwith per-pattern error isolation (one bad annotation can't break all reviews)codec-generators.tsHardening
sanitizeMermaidLabel()prevents diagram corruption from special characters in labelsInput/Outputannotations are missingsequence-steptagsRecord<string, unknown>cast)Architecture Decisions
ReferenceDocCodecsequence-utils.tsSequenceIndexkeyed by pattern nameMasterDatasetis the sole read model (ADR-006)Files Changed (22 files, +2582/−19)
Schemas:
master-dataset.ts,extracted-pattern.tsPipeline:
registry-builder.ts,gherkin-ast-parser.ts,gherkin-extractor.ts,sequence-utils.ts(new),transform-dataset.tsCodec + Generator:
design-review.ts(new),helpers.ts,design-review-generator.ts(new),codec-generators.tsCLI + Config:
process-api.ts,delivery-process.config.ts,package.jsonSpecs:
setup-command.feature(annotated),design-review.feature(new — 44 BDD scenarios)Tests:
design-review.steps.ts(new),design-review-state.ts(new),pattern-factories.tsGenerated:
design-reviews/setup-command.md(new)Test Plan
pnpm typecheck— cleanpnpm lint— 0 errorspnpm test— all passing (126 test files, 8165 assertions)pnpm docs:design-review— generatessetup-command.mdfrom annotationspnpm process:query -- sequence SetupCommand— returns structured sequence datapnpm process:query -- sequence— lists all annotated patternsSummary by CodeRabbit