chore: pre-rebranding cleanup — refactor, test coverage, generated docs#36
Merged
darko-mijic merged 8 commits intomainfrom Mar 14, 2026
Merged
chore: pre-rebranding cleanup — refactor, test coverage, generated docs#36darko-mijic merged 8 commits intomainfrom
darko-mijic merged 8 commits intomainfrom
Conversation
Structural cleanup before the Architect rebrand to reduce noise in the rename diff and improve module boundaries. Package cleanup: - Remove unused devDeps (pixelmatch, quickpickle) - Unblock @types/node with caret version process-api.ts refactor: - Split parseArgs() (320 lines) into 3 focused handlers + routing loop - Replace unsafe dynamic method dispatch with typed API_DISPATCH map Architecture: - Decouple generators→lint: new src/git/ module for branch diff - Split transform-dataset.ts (777→480 lines) into context-inference, transform-types, and relationship-resolver sub-modules - Document intentional config→renderable cross-layer dependency - Add error convention comments to 8 CLI entry points Test coverage for previously untested modules: - src/taxonomy/: normalized-status, deliverable-status, tag-registry-builder - src/validation-schemas/: codec-utils, tag-registry-schemas, workflow-config - src/cache/: file-cache - 7 new feature files, 320 new tests (8257→8577 total) CI: add coverage reporting (v8 provider, Node 22 only, artifact upload)
- Fix parseNameStatus to handle R100/C087 rename/copy status variants (was matching exact 'R'/'C', silently dropping renamed files) - Fix rename path extraction to use last pathParts element instead of joining paths then splitting on '->' (which git doesn't use) - Exclude deleted files from getChangedFilesList — orchestrator only needs files that still exist for PR-scoped generation - Add extendedBy sort in buildReverseLookups for deterministic output - Remove unnecessary `const p = pattern` alias in transform-dataset - Remove no-op css/plugins config from vitest.config.ts
…tching, test pattern IDs - Extract shared parseGitNameStatus() into src/git/name-status.ts for NUL-delimited (-z) git diff output — handles renames (R100), copies (C087), and filenames with spaces correctly - Switch both branch-diff.ts and detect-changes.ts to use the shared parser with -z flag instead of whitespace splitting - Fix context-inference.ts matchPattern() to use segment-boundary checks (hasPathPrefix) so src/validation/** no longer matches src/validation2/ - Rename test pattern IDs to *Testing suffix to avoid colliding with production pattern names in the living-docs dataset - Add git-branch-diff.feature with tests for modify/add/delete classification, rename/copy statuses, and filenames with spaces - Add @vitest/coverage-v8 dev dependency for test:coverage support - Fix cli-runner to strip NODE_V8_COVERAGE from child env - Regenerate product-area docs
Add 3 new reference docs (Configuration Guide, Validation Tools Guide, Gherkin Authoring Guide) via preamble-driven ReferenceDocConfig entries. Extend Annotation Reference and Process Guard Reference preambles to close remaining quality gaps. Add deprecation notices to all manual docs except METHODOLOGY.md (kept as editorial). Regenerate all docs-live/.
Add diagramKeyComponentsOnly option (default: true) to ArchitectureCodec that filters diagram nodes to only patterns with an explicit archRole. Reduces diagram from 162 nodes to 59 — removing barrel exports, type modules, ADRs, and test features that add noise without architectural significance. The component inventory table retains all 162 entries.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughReorganizes pipeline types and helpers, adds Git change-detection and context-inference utilities, introduces relationship-resolution helpers, refactors CLI argument parsing to a state-machine with typed dispatch, adds architecture diagram filtering, updates CI to upload coverage only on Node 22, and expands BDD tests and test helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Layer (process-api.ts)
participant Parser as ParseState / Arg Handlers
participant Dispatch as API_DISPATCH
participant API as API Modules (generators, git)
participant Result as Result Handler
CLI->>Parser: init ParseState, consume args
Parser->>Parser: handle flags/positionals
Parser->>CLI: return parsed method & args
CLI->>Dispatch: validate method name
Dispatch->>API: call typed handler (e.g., getChangedFilesList, generator)
API->>Result: return Result<T,E>
Result->>CLI: propagate ok / err
CLI->>CLI: unified error handler -> process.exit on failure
sequenceDiagram
participant Raw as RawDataset
participant Transform as transformToMasterDataset
participant ContextInf as context-inference.ts
participant RelRes as relationship-resolver.ts
participant Types as transform-types.ts
Raw->>Transform: provide patterns, tagRegistry, rules
Transform->>ContextInf: inferContext for each file
ContextInf->>Transform: returns context or undefined
Transform->>RelRes: buildReverseLookups & detectDanglingReferences
RelRes->>Transform: reverse lookups + dangling refs
Transform->>Types: assemble RuntimeMasterDataset + ValidationSummary
Transform->>Caller: return TransformResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Move execGitSafe() and sanitizeBranchName() from branch-diff.ts and detect-changes.ts into shared src/git/helpers.ts. Complete barrel export with parseGitNameStatus and ParsedGitNameStatus.
Picks up new GitHelpers pattern, deterministic diagram sorting, and formatting improvements from codec updates.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/features/behavior/architecture-diagrams/layered-diagram.feature (1)
106-125:⚠️ Potential issue | 🟡 MinorUpdate the Rule prose to match the new “key components” metric.
Line 125 now asserts
2 key components, but Lines 108-109 still describe the Overview as an annotated-source-file count. Since this text is published as documentation, the prose should match the executable expectation.✏️ Suggested wording
- **Invariant:** The generated layered diagram document must include an Overview section with annotated source file count. - **Rationale:** Without summary counts, readers cannot assess diagram completeness or detect missing annotated sources. + **Invariant:** The generated layered diagram document must include an Overview section with key component count. + **Rationale:** Without summary counts, readers cannot assess diagram scope or detect missing key components.As per coding guidelines,
**/*.feature: Feature files are both executable specs and documentation source; feature description content appears in generated docs, so prioritize clarity and structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/features/behavior/architecture-diagrams/layered-diagram.feature` around lines 106 - 125, The feature's Rule prose still describes the Overview as an "annotated source file count" while the Scenario "Summary section for layered view" asserts "2 key components"; update the Rule text (the header and the two sentences that read "The generated layered diagram document must include an Overview section with annotated source file count" and "The generated document starts with an overview section specific to layered architecture visualization.") to instead describe an Overview that reports the annotated key components count (e.g., "Overview section with annotated key components count" and similar wording) so the documentation prose matches the executable expectation.src/cli/lint-steps.ts (1)
3-9: 🧹 Nitpick | 🔵 TrivialMissing
@libar-docsannotation.Per coding guidelines, TypeScript files should include
@libar-docsannotations for discoverability via API queries. Consider adding the annotation to the JSDoc block.📝 Suggested addition
/** + * `@libar-docs` + * `@libar-docs-cli` + * `@libar-docs-pattern` LintStepsCLI + * `@libar-docs-status` completed + * * CLI for vitest-cucumber step/feature compatibility linting. * * Detects common vitest-cucumber traps statically — before tests run. * Catches mismatches between .feature files and .steps.ts files that * cause cryptic runtime failures. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/lint-steps.ts` around lines 3 - 9, Add the missing `@libar-docs` JSDoc tag to the top-of-file JSDoc block in src/cli/lint-steps.ts (the existing comment describing the CLI for vitest-cucumber step/feature compatibility linting); update that header comment to include "@libar-docs" so the file becomes discoverable by API queries while leaving the existing description intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/GHERKIN-PATTERNS.md`:
- Line 3: Update the "**Deprecated:**" note and surrounding copy in the
GHERKIN-PATTERNS.md header to clearly mark the rest of the page as
historical/stale: replace or augment "Preserved for reference only" with an
explicit warning (e.g., "Historical: examples may be outdated — do not copy; see
the auto-generated Gherkin Authoring Guide"), and either remove or annotate
outdated examples (for instance any deliverable tables using "Done"/"Yes") so
readers are not misled; ensure the page points readers to the authoritative
auto-generated guide and that any retained examples are labeled "historical"
with a timestamp or version note.
In `@docs/INDEX.md`:
- Around line 3-4: The deprecation blockquote contains an extra blank line
causing markdownlint MD028; edit the block that starts with "**Deprecated:**
This document is superseded by the auto-generated [Documentation
Index](../docs-live/INDEX.md)..." and remove the empty line so the entire
deprecation note is a single uninterrupted blockquote (no blank lines inside the
blockquote).
In `@docs/PROCESS-API.md`:
- Around line 3-4: Update the top banner in PROCESS-API.md so it accurately
reflects the page contents: either move the normative AI-agent startup guidance
and generated-reference navigation into the generated recipes/reference pages,
or soften the banner wording to state that this file primarily contains
operational reference (JSON envelope, exit codes) while also retaining some
startup guidance and navigation for backward compatibility; reference the
existing banner line beginning "**Deprecated:** The full CLI documentation is
now auto-generated..." and ensure the revised text clearly matches the actual
sections present on the page.
In `@src/cli/process-api.ts`:
- Around line 878-930: Several API_DISPATCH handlers coerce args[0] (via
Number(...) or String(...)) without validating it, causing missing CLI args to
turn into NaN/"undefined" instead of producing an INVALID_ARGUMENT error; update
each affected entry in the API_DISPATCH map (e.g., getPatternsByPhase,
getPhaseProgress, getPatternsByNormalizedStatus, getPatternsByStatus,
isValidTransition, checkTransition, getPattern, getPatternDependencies,
getPatternRelationships, getRelatedPatterns, getApiReferences,
getPatternDeliverables, getPatternsByCategory, getPatternsByQuarter,
getRecentlyCompleted) to first assert the required arg exists (e.g., if (args[0]
=== undefined || args[0] === '') throw new QueryApiError('INVALID_ARGUMENT',
'missing query arg')), then coerce and call the API; mirror the pattern used in
handlePattern (the if (!name) throw ...) for consistent error type and message.
In `@src/cli/validate-patterns.ts`:
- Around line 29-32: parseArgs() is called before the try block and main() is
invoked without a promise-level catch, so CLI parse errors can escape the
unified handler; move the parseArgs() call inside the try that calls main() (or
otherwise wrap parseArgs() in its own try) and ensure the top-level invocation
of main() is awaited or has a .catch(...) that forwards the error to
handleCliError; specifically update the code around parseArgs, main, and the
top-level invocation so any thrown error from parseArgs() or main() is passed to
handleCliError instead of becoming an unhandled rejection.
In `@src/generators/pipeline/relationship-resolver.ts`:
- Around line 1-15: Add `@libar-docs-uses` annotations to the top header comment
block of this RelationshipResolver module so its runtime dependencies are
discoverable; update the comment above RelationshipResolver (the /** ... */
block) to include one or more `@libar-docs-uses` lines naming the actual runtime
modules/functions this file calls (e.g., the TransformDataset consumer or any
utility/services imported here), ensuring you use `@libar-docs-uses` (not
`@libar-docs-depends-on`) and follow the existing `@libar-docs` tag style and
placement.
In `@src/generators/pipeline/transform-dataset.ts`:
- Around line 206-210: The fallback branch currently classifies any
non-".feature" file as TypeScript, which incorrectly buckets ".feature.md" files
into bySource.typescript; update the conditional in transform-dataset (the block
that checks pattern.source.file.endsWith('.feature')) to also detect
".feature.md" (e.g. treat files where pattern.source.file.endsWith('.feature')
|| pattern.source.file.endsWith('.feature.md') as Gherkin) so those inputs go
into bySource.gherkin instead of bySource.typescript.
In `@src/generators/pipeline/transform-types.ts`:
- Around line 25-109: Replace the plain TS interfaces for the serializable
contracts with Zod schemas in src/validation-schemas and derive the exported
types from those schemas: create Zod schemas for MalformedPattern,
DanglingReference, ValidationSummary, TransformResult, and RawDataset (move
schema-able fields into the new validation files and export types via z.infer),
then update this file to import those inferred types instead of declaring
interfaces; keep RuntimeMasterDataset as the runtime-only augmentation that
explicitly adds readonly workflow?: LoadedWorkflow (do NOT move LoadedWorkflow
into validation), and ensure TransformResult.dataset uses the existing
RuntimeMasterDataset while TransformResult.validation and
RawDataset.patterns/tagRegistry use the schema-derived types so runtime
validation can be performed.
- Around line 1-20: Add the missing libar docs relationship tags to the module
header: update the TransformTypes file's top comment block to include
`@libar-docs-uses` entries that list its runtime dependencies (e.g.,
MasterDataset, LoadedWorkflow, ExtractedPattern, TagRegistry,
ContextInferenceRule) so the module's relationships are queryable; locate the
header comment above the imports in the transform-types module (the
TransformTypes docblock) and append appropriate `@libar-docs-uses` lines naming
those types/modules.
In `@src/git/branch-diff.ts`:
- Around line 58-65: In sanitizeBranchName, reject branch names that start with
a hyphen to prevent git option injection by adding a check that throws an error
when branch.startsWith('-'); keep the existing validations (regex and '..'
check) and ensure the thrown message references the branch and that it "starts
with hyphen" so callers of sanitizeBranchName will refuse values like --help or
-c before any git invocation.
In `@src/git/index.ts`:
- Around line 1-15: Add a relationship tag to the `@libar-docs` header to register
this module's dependency on the branch diff helper: update the module docblock
that currently annotates GitModule to include an appropriate relationship tag
such as `@libar-docs-depends-on` or `@libar-docs-uses` referencing the
'./branch-diff.js' export (the symbol exposed via getChangedFilesList) so the
file's dependency is discoverable; modify the top comment block above the export
of getChangedFilesList to include that tag.
In `@src/renderable/codecs/architecture.ts`:
- Around line 207-214: The Overview text currently always describes the output
as "key components with explicit architectural roles" even when
options.diagramKeyComponentsOnly is false; update the code that calls
buildSummarySection (using diagramIndex and filteredIndex) to pass or compute a
conditional description based on options.diagramKeyComponentsOnly and ensure the
same conditional wording is used in the later summary generation (the block
around the code that builds the overview/summary, including the logic that reads
diagramIndex, filteredIndex, and any text produced in the 359–383 region). In
practice, change the buildSummarySection invocation (or the string it uses) to
select between a "key components with explicit architectural roles" message when
options.diagramKeyComponentsOnly is true and a different message describing the
full annotated set when false, and mirror that conditional text in the other
summary construction so the Overview accurately reflects the option.
- Around line 307-349: The function filterToKeyComponents currently filters
archIndex.all, byContext, byRole and byLayer but leaves archIndex.byView
unmodified; update filterToKeyComponents so that it also filters
archIndex.byView using the same hasRole predicate (build a filteredByView
Record<string, ExtractedPattern[]>, only include keys with non-empty arrays) and
return that filteredByView in place of archIndex.byView so diagramIndex.byView
cannot reintroduce patterns lacking archRole.
In `@tests/features/types/normalized-status.feature`:
- Around line 8-19: The feature description claims normalizeStatus maps only raw
FSM states but the examples assert behavior for already-normalized values,
undefined, and unknown strings; update the Background/Rule prose to accurately
reflect the full contract of normalizeStatus (or remove those extra example
cases) by explicitly stating that normalizeStatus accepts raw FSM states and
already-normalized inputs, and that undefined or unknown inputs default to
"planned" (also mirror this wording where the spec repeats on lines 26-33);
reference the normalizeStatus behavior in the rule text so docs and tests are
consistent.
In `@tests/steps/cli/data-api-cache.steps.ts`:
- Around line 82-86: Remove the committed describe.skip branch controlled by
skipCacheCliCoverage and instead ensure coverage runs still execute the cache
tests: delete the if/else that returns describe.skip when
process.env.NODE_V8_COVERAGE is set (the skipCacheCliCoverage/describe.skip
branch shown) and modify the child CLI spawn logic used by these steps to remove
NODE_V8_COVERAGE from the env passed to the spawned process (strip
env.NODE_V8_COVERAGE before calling the CLI spawn helper or spawn function so
the child runs with cache code exercised).
- Around line 143-147: The test currently asserts an absolute timing
(metadata.pipelineMs < 500) which is flaky; update the assertion in the step
that reads state via getCacheState(state) and parses
parseMetadata(s.secondResult!) to either compare the second run's pipelineMs
against the first run's pipelineMs (e.g.,
expect(secondMetadata.pipelineMs).toBeLessThan(firstMetadata.pipelineMs)) or
simply assert cache hit (e.g.,
expect(parsedSecond.metadata?.cache?.hit).toBe(true)); remove the hard-coded
500ms check and use state.firstResult / cache.hit as the correctness baseline
instead.
In `@tests/steps/types/normalized-status.steps.ts`:
- Around line 1-15: Add a top-of-file `@libar-docs` annotation block to this
TypeScript step file (replace the prose header) so it opts into annotation-based
discovery: include `@libar-docs`, a short description, and relationship tags
referencing the normalized-status module and exported symbols (e.g.,
`@libar-docs-depends-on` ../../../src/taxonomy/normalized-status.js and
`@libar-docs-uses` normalizeStatus, isPatternComplete, isPatternActive,
isPatternPlanned). Place the block at the very top of
tests/steps/types/normalized-status.steps.ts above the imports so tooling can
discover it.
- Around line 67-77: The step definitions in the RuleScenarioOutline blocks use
Cucumber expression `{string}` placeholders but the feature uses literal
ScenarioOutline placeholders; update the step patterns to use literal
angle-bracket placeholders so the variables object works correctly.
Specifically, change the patterns in the block using normalizeStatus to
When('normalizing status <rawStatus>') and Then('the normalized status is
<normalizedStatus>') (keep state!.normalizedResult and variables access), and
update the three predicate blocks that call isPatternComplete, isPatternActive,
and isPatternPlanned to use When('checking isPatternComplete for <status>') /
Then('the predicate result is <expected>'), When('checking isPatternActive for
<status>') / Then('the predicate result is <expected>'), and When('checking
isPatternPlanned for <status>') / Then('the predicate result is <expected>')
respectively so the ScenarioOutline placeholders map to variables.
In `@tests/steps/types/tag-registry-builder.steps.ts`:
- Around line 1-8: Add library docs annotations to the top of this TypeScript
step file so it is discoverable by the Process Data API: insert an `@libar-docs`
tag and appropriate relationship tags such as `@libar-docs-uses` (and/or
`@libar-docs-depends-on`) referencing the relevant symbols used in this file
(e.g., buildRegistry, TagRegistry, and any Transform functions) in the file
header comment so tooling can pick up the file for docs graph queries.
In `@tests/steps/utils/git-branch-diff.steps.ts`:
- Around line 1-20: Add a leading `@libar-docs` annotation block at the top of
this TypeScript step file so it is discoverable by annotation-based tooling:
insert a multi-line comment containing `@libar-docs` with a brief
title/description and include relationship tags `@libar-docs-depends-on` and
`@libar-docs-uses` listing any modules referenced here (for example the git
helpers getChangedFilesList and parseGitNameStatus and the file-system helpers
createTempDir/writeTempFile) so the file is indexed; place the block above the
existing prose header/comment.
In `@tests/steps/validation/codec-utils.steps.ts`:
- Around line 1-18: This file lacks the required `@libar-docs` metadata block and
relationship tags which are required for repository discoverability; add a
top-of-file JSDoc-style `@libar-docs` annotation including a short description and
include relationship tags such as `@libar-docs-depends-on` and `@libar-docs-uses`
that reference the modules/types this test exercises (mentioning
createJsonInputCodec, formatCodecError, CodecError, JsonInputCodec and Result)
so API queries can discover the file and its dependencies; place the block at
the top of tests/steps/validation/codec-utils.steps.ts above the imports.
In `@tests/steps/validation/tag-registry-schemas.steps.ts`:
- Around line 1-15: Add the repository discoverability annotations to the top of
this new step module by inserting a file-level comment that includes the
`@libar-docs` tag and an `@libar-docs-uses` listing the runtime dependency module(s)
used by the steps (for example referencing
'../../../src/validation-schemas/tag-registry.js' and the exported symbols used:
TagRegistrySchema, createDefaultTagRegistry, mergeTagRegistries, TagRegistry);
ensure you add only `@libar-docs-uses` (not `@libar-docs-depends-on`) per guidelines
and place the comment above the existing imports so the file opts into
repo-level discoverability.
In `@tests/steps/validation/workflow-config-schemas.steps.ts`:
- Around line 1-17: Add a top-of-file `@libar-docs` annotation block to this
TypeScript test module: insert an annotation comment that includes a short
description and the relationship tags `@libar-docs-depends-on` and
`@libar-docs-uses` referencing the validation module and the specific exports used
(WorkflowConfigSchema, createLoadedWorkflow, isWorkflowConfig, WorkflowConfig,
LoadedWorkflow) so the file opts into repo-wide metadata; place the block above
the existing prose header and imports, following the same annotation format used
elsewhere in the repo.
---
Outside diff comments:
In `@src/cli/lint-steps.ts`:
- Around line 3-9: Add the missing `@libar-docs` JSDoc tag to the top-of-file
JSDoc block in src/cli/lint-steps.ts (the existing comment describing the CLI
for vitest-cucumber step/feature compatibility linting); update that header
comment to include "@libar-docs" so the file becomes discoverable by API queries
while leaving the existing description intact.
In `@tests/features/behavior/architecture-diagrams/layered-diagram.feature`:
- Around line 106-125: The feature's Rule prose still describes the Overview as
an "annotated source file count" while the Scenario "Summary section for layered
view" asserts "2 key components"; update the Rule text (the header and the two
sentences that read "The generated layered diagram document must include an
Overview section with annotated source file count" and "The generated document
starts with an overview section specific to layered architecture
visualization.") to instead describe an Overview that reports the annotated key
components count (e.g., "Overview section with annotated key components count"
and similar wording) so the documentation prose matches the executable
expectation.
🪄 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: db905336-d1c4-4c88-84c6-a0df39121046
⛔ Files ignored due to path filters (35)
.gitignoreis excluded by none and included by nonedelivery-process.config.tsis excluded by none and included by nonedocs-live/ARCHITECTURE.mdis excluded by none and included by nonedocs-live/BUSINESS-RULES.mdis excluded by none and included by nonedocs-live/CHANGELOG-GENERATED.mdis excluded by none and included by nonedocs-live/INDEX.mdis excluded by none and included by nonedocs-live/PRODUCT-AREAS.mdis excluded by none and included by nonedocs-live/_claude-md/annotation/annotation-reference.mdis excluded by none and included by nonedocs-live/_claude-md/architecture/architecture-codecs.mdis excluded by none and included by nonedocs-live/_claude-md/authoring/gherkin-authoring-guide.mdis excluded by none and included by nonedocs-live/_claude-md/configuration/configuration-guide.mdis excluded by none and included by nonedocs-live/_claude-md/core-types/core-types-overview.mdis excluded by none and included by nonedocs-live/_claude-md/validation/process-guard.mdis excluded by none and included by nonedocs-live/_claude-md/validation/validation-tools-guide.mdis excluded by none and included by nonedocs-live/business-rules/core-types.mdis excluded by none and included by nonedocs-live/business-rules/generation.mdis excluded by none and included by nonedocs-live/business-rules/validation.mdis excluded by none and included by nonedocs-live/product-areas/CORE-TYPES.mdis excluded by none and included by nonedocs-live/product-areas/GENERATION.mdis excluded by none and included by nonedocs-live/product-areas/VALIDATION.mdis excluded by none and included by nonedocs-live/reference/ANNOTATION-REFERENCE.mdis excluded by none and included by nonedocs-live/reference/ARCHITECTURE-CODECS.mdis excluded by none and included by nonedocs-live/reference/CONFIGURATION-GUIDE.mdis excluded by none and included by nonedocs-live/reference/GHERKIN-AUTHORING-GUIDE.mdis excluded by none and included by nonedocs-live/reference/PROCESS-GUARD-REFERENCE.mdis excluded by none and included by nonedocs-live/reference/REFERENCE-SAMPLE.mdis excluded by none and included by nonedocs-live/reference/VALIDATION-TOOLS-GUIDE.mdis excluded by none and included by nonedocs-sources/annotation-guide.mdis excluded by none and included by nonedocs-sources/configuration-guide.mdis excluded by none and included by nonedocs-sources/gherkin-patterns.mdis excluded by none and included by nonedocs-sources/process-guard.mdis excluded by none and included by nonedocs-sources/validation-tools-guide.mdis excluded by none and included by nonepackage.jsonis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yamland included by nonevitest.config.tsis excluded by none and included by none
📒 Files selected for processing (72)
.github/workflows/ci.ymldocs/ANNOTATION-GUIDE.mddocs/ARCHITECTURE.mddocs/CONFIGURATION.mddocs/GHERKIN-PATTERNS.mddocs/INDEX.mddocs/METHODOLOGY.mddocs/PROCESS-API.mddocs/PROCESS-GUARD.mddocs/SESSION-GUIDES.mddocs/TAXONOMY.mddocs/VALIDATION.mdsrc/cli/generate-docs.tssrc/cli/generate-tag-taxonomy.tssrc/cli/lint-patterns.tssrc/cli/lint-process.tssrc/cli/lint-steps.tssrc/cli/process-api.tssrc/cli/repl.tssrc/cli/validate-patterns.tssrc/config/defaults.tssrc/config/project-config-schema.tssrc/config/project-config.tssrc/config/resolve-config.tssrc/config/types.tssrc/generators/orchestrator.tssrc/generators/pipeline/build-pipeline.tssrc/generators/pipeline/context-inference.tssrc/generators/pipeline/index.tssrc/generators/pipeline/relationship-resolver.tssrc/generators/pipeline/transform-dataset.tssrc/generators/pipeline/transform-types.tssrc/git/branch-diff.tssrc/git/index.tssrc/git/name-status.tssrc/lint/process-guard/detect-changes.tssrc/renderable/codecs/architecture.tstests/features/behavior/architecture-diagrams/component-diagram.featuretests/features/behavior/architecture-diagrams/layered-diagram.featuretests/features/behavior/context-inference.featuretests/features/types/deliverable-status.featuretests/features/types/normalized-status.featuretests/features/types/tag-registry-builder.featuretests/features/utils/file-cache.featuretests/features/utils/git-branch-diff.featuretests/features/validation/codec-utils.featuretests/features/validation/tag-registry-schemas.featuretests/features/validation/workflow-config-schemas.featuretests/fixtures/dataset-factories.tstests/steps/api/architecture-queries/arch-queries.steps.tstests/steps/api/context-assembly/context-assembler.steps.tstests/steps/architecture/arch-index.steps.tstests/steps/behavior/context-inference.steps.tstests/steps/behavior/implementation-links.steps.tstests/steps/behavior/transform-dataset.steps.tstests/steps/cli/data-api-cache.steps.tstests/steps/generation/design-review-generator.steps.tstests/steps/generators/business-rules-generator.steps.tstests/steps/generators/pr-changes-options.steps.tstests/steps/generators/prd-implementation-section.steps.tstests/steps/generators/table-extraction.steps.tstests/steps/types/deliverable-status.steps.tstests/steps/types/normalized-status.steps.tstests/steps/types/tag-registry-builder.steps.tstests/steps/utils/file-cache.steps.tstests/steps/utils/git-branch-diff.steps.tstests/steps/validation/codec-utils.steps.tstests/steps/validation/tag-registry-schemas.steps.tstests/steps/validation/workflow-config-schemas.steps.tstests/support/helpers/cli-runner.tstests/support/helpers/design-review-state.tstests/support/helpers/process-api-state.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/git/helpers.ts`:
- Around line 57-66: The sanitizeBranchName function currently allows branch
names that begin with '-' which can be interpreted as git options; update
sanitizeBranchName (the branch parameter) to reject any branch that starts with
a hyphen by adding an explicit check (e.g., if (branch.startsWith('-')) throw
new Error(...)) before returning/using the name, so leading '-' values like
"--help" or "-c" are refused; keep the existing pattern and the '..' check
intact.
🪄 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: aa2abee7-640f-45c6-b6e8-20d3da1424d8
⛔ Files ignored due to path filters (9)
docs-live/ARCHITECTURE.mdis excluded by none and included by nonedocs-live/CHANGELOG-GENERATED.mdis excluded by none and included by nonedocs-live/INDEX.mdis excluded by none and included by nonedocs-live/PRODUCT-AREAS.mdis excluded by none and included by nonedocs-live/_claude-md/process/process-overview.mdis excluded by none and included by nonedocs-live/product-areas/GENERATION.mdis excluded by none and included by nonedocs-live/product-areas/PROCESS.mdis excluded by none and included by nonedocs-live/product-areas/VALIDATION.mdis excluded by none and included by nonedocs-live/reference/REFERENCE-SAMPLE.mdis excluded by none and included by none
📒 Files selected for processing (5)
src/git/branch-diff.tssrc/git/helpers.tssrc/git/index.tssrc/lint/process-guard/detect-changes.tstests/steps/utils/git-branch-diff.steps.ts
- Reject branch names starting with hyphen in sanitizeBranchName (git option injection)
- Fix .feature.md files misclassified as TypeScript in bySource grouping
- Filter byView in filterToKeyComponents to prevent node reintroduction
- Add requireStringArg/requireNumberArg validation to process-api dispatch
- Add .catch() to validate-patterns entry point for unified error handling
- Remove describe.skip in cache tests; strip NODE_V8_COVERAGE via createChildEnv
- Replace hard-coded 500ms timing threshold with relative comparison
- Fix ScenarioOutline step patterns: use angle-bracket placeholders not {string}
- Add @libar-docs annotations to 9 files for discoverability
- Fix documentation accuracy in deprecation banners and feature descriptions
- Make architecture overview text conditional on diagramKeyComponentsOnly
- Regenerate docs with updated annotations
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
Structural cleanup before the upcoming product rebrand to reduce noise in the rename diff and improve module boundaries, test coverage, and documentation generation.
Why now? Module decomposition, test additions, and doc generation changes are nearly impossible to review when interleaved with hundreds of file renames. Doing this work first keeps the rename PR a clean find-replace.
Changes
Module Decomposition
Problem:
transform-dataset.ts(777 lines) andprocess-api.ts'sparseArgs()(320 lines) had grown into monoliths mixing unrelated concerns — type definitions, context inference, relationship resolution, and three categories of CLI flag handling.Solution:
transform-dataset.tsinto 4 focused modules:transform-types.ts— type exports (RuntimeMasterDataset,RawDataset,ValidationSummary, etc.)context-inference.ts— bounded-context auto-inference from file paths with segment-boundary matchingrelationship-resolver.ts— reverse lookup computation (implementedBy,extendedBy,enables,usedBy) and dangling reference detectiontransform-dataset.ts— core transformation loop only (777 → 416 lines)parseArgs()into 3 handler functions + routing loop:handlePositionIndependentFlag()— help, version, cache, modifiers, formathandleGlobalFlag()— input, features, base-dir, workflow, sessionhandlePositionalArg()— subcommand and subArgsapi as unknown as Record<string, (...a: unknown[]) => unknown>dynamic dispatch with a typedAPI_DISPATCHmap that guarantees compile-time completeness viaRecord<ApiMethodName, ...>Layer Decoupling
Problem: The generators layer imported
detectBranchChangesfrom the lint layer just to get a file list. This created a dependency from generation → validation that doesn't belong.Solution:
src/git/module with two files:branch-diff.ts— lightweightgetChangedFilesList()for PR-scoped generation (excludes deleted files since orchestrator only needs files that still exist)name-status.ts— sharedparseGitNameStatus()for NUL-delimited (-z) git diff outputbranch-diff.ts(generators) anddetect-changes.ts(lint) now use the shared parserTest Coverage (+320 tests)
Problem: Several core modules had zero test coverage — taxonomy helpers, validation schemas, and the file cache. These are foundational infrastructure consumed across the pipeline.
Solution: 7 new Gherkin feature files with step definitions covering:
taxonomy/normalized-statusnormalized-status.featuretaxonomy/deliverable-statusdeliverable-status.featuretaxonomy/tag-registry-buildertag-registry-builder.featurevalidation-schemas/codec-utilscodec-utils.featurevalidation-schemas/tag-registry-schemastag-registry-schemas.featurevalidation-schemas/workflow-configworkflow-config-schemas.featurecache/file-cachefile-cache.featuregit/branch-diffgit-branch-diff.featureDocumentation Generation
Problem: 10 of 11 manual docs (
docs/) had fallen behind their auto-generated equivalents indocs-live/. The inline preamble blocks indelivery-process.config.tswere growing unwieldy (the Process Guard preamble alone was 90+ lines of TypeScript object literals).Solution:
docs-sources/(markdown files parsed at config load time):process-guard.md,configuration-guide.md,validation-tools-guide.md,gherkin-patterns.md,annotation-guide.mdReferenceDocConfigentries generating:CONFIGURATION-GUIDE.md— presets, config files, sources, output, monorepo setupVALIDATION-TOOLS-GUIDE.md— lint-patterns, lint-steps, lint-process, validate-patternsGHERKIN-AUTHORING-GUIDE.md— roadmap specs, Rule blocks, DataTables, tag conventionsloadPreambleFromMarkdown()callsMETHODOLOGY.mdas the sole editorial document (design philosophy can't be auto-generated)Architecture Diagrams
Problem: The auto-generated architecture diagram included all 162 annotated patterns — barrel exports, type-only modules, ADRs, and test features cluttered the diagram without conveying architectural significance.
Solution:
diagramKeyComponentsOnlyoption onArchitectureCodec(default:true)archRoleannotationBug Fixes
git diff --name-statusoutput to NUL-delimited (-z) parsing. The old approach silently dropped renamed files (R100 old newparsed as statusR100instead ofR) and broke on filenames with spaces.matchPattern('src/validation/**', ...)now uses segment-boundary checks sosrc/validation2/file.tsno longer matchessrc/validation/**.R100/C087status variants (with similarity percentages) now parse correctly — previously only exactR/Cmatched.extendedByarrays inbuildReverseLookups()for consistent output across runs.CI & Dependencies
@vitest/coverage-v8with coverage reporting on Node 22 CI runs (artifact upload)test:coveragescriptpixelmatch,quickpickle@types/nodewith caret version (^20.10.0instead of pinned20.10.0)Stats
src/)src/)docs-live/,docs-sources/,docs/)Test plan
pnpm build— TypeScript compiles cleanlypnpm test— all tests pass (8,577 total including 320 new)pnpm typecheck— no type errorspnpm lint— no lint violationspnpm docs:all— documentation regenerates without errorspnpm validate:all— all validations passdocs-live/ARCHITECTURE.mdshows ~59 key components (not 162)docs/have deprecation notices pointing todocs-live/equivalentsSummary by CodeRabbit
New Features
Documentation
Tests