fix: taxonomy cleanup and extraction bug fixes#39
Merged
darko-mijic merged 1 commit intomainfrom Apr 1, 2026
Merged
Conversation
…shape directives Remove @architect-brief (dead relic) and @architect-core flag (zero consumers), fix hardcoded 'ddd' category fallback in Gherkin extractor, validate category inference against registry, and filter shape-only directives at scanner level. BREAKING CHANGE: @architect-brief tag removed, isCore field removed from schemas
📝 WalkthroughWalkthroughThis PR removes support for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 docstrings
🧪 Generate unit tests (beta)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/extractor/doc-extractor.ts (1)
3-3:⚠️ Potential issue | 🟡 MinorSelf-referential use of removed
@architect-coretag.This file's JSDoc uses
@architect-core@architect-extractor`` at line 3. The@architect-coretag is removed by this PR.📝 Proposed fix
* `@architect` - * `@architect-core` `@architect-extractor` + * `@architect-extractor` * `@architect-pattern` Document Extractor🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/extractor/doc-extractor.ts` at line 3, Update the JSDoc in src/extractor/doc-extractor.ts to stop referencing the removed tag: remove the obsolete "@architect-core" token from the top-level comment so it only contains "@architect-extractor" (or other remaining valid tags); locate the JSDoc block at the top of doc-extractor.ts and edit that comment to delete "@architect-core" wherever it appears.src/scanner/ast-parser.ts (1)
3-3:⚠️ Potential issue | 🟡 MinorSelf-referential use of removed
@architect-coretag.Similar to
registry-builder.ts, this file's JSDoc uses@architect-core@architect-scanner`` at line 3. The@architect-coreportion will produce an unknown-tag warning after this PR.📝 Proposed fix
* `@architect` - * `@architect-core` `@architect-scanner` + * `@architect-scanner` * `@architect-pattern` TypeScript AST Parser🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scanner/ast-parser.ts` at line 3, Remove the obsolete JSDoc tag from the file's header by editing the doc comment that currently contains "@architect-core `@architect-scanner`" in src/scanner/ast-parser.ts so it only includes the valid "@architect-scanner" tag; locate the JSDoc block at the top of the file (the same pattern used in registry-builder.ts) and delete the "@architect-core" token to avoid the unknown-tag warning.src/taxonomy/registry-builder.ts (2)
110-110:⚠️ Potential issue | 🟡 MinorStale documentation: comment still references removed tags.
The JSDoc comment at line 110 says
core: Essential pattern identification (pattern, status, core, usecase, brief)but the actual array at line 123 is now['pattern', 'status', 'usecase']. Update the comment to reflect the removal ofcoreandbrief.📝 Proposed fix
- * - core: Essential pattern identification (pattern, status, core, usecase, brief) + * - core: Essential pattern identification (pattern, status, usecase)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/taxonomy/registry-builder.ts` at line 110, Update the JSDoc comment that still mentions "core" and "brief" so it matches the actual tags array (now ['pattern', 'status', 'usecase']); remove references to core and brief and change the example/comment text to "pattern: Essential pattern identification (pattern, status, usecase)" or similar to reflect only pattern, status, and usecase in the JSDoc block above the tags array.
3-3:⚠️ Potential issue | 🟡 MinorSelf-referential use of removed
@architect-coretag.This file's own JSDoc annotation uses
@architect-coreat line 3, which is the very tag being removed by this PR. This will likely cause an "unknown-tag" warning when the scanner processes this file.📝 Proposed fix: remove the deprecated tag
* `@architect` - * `@architect-core` * `@architect-pattern` TagRegistryBuilder🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/taxonomy/registry-builder.ts` at line 3, Remove the deprecated JSDoc tag `@architect-core` from the top-of-file JSDoc in registry-builder.ts (the self-referential tag at the file header); edit the file header comment to delete the "@architect-core" token so the scanner no longer emits an "unknown-tag" warning while leaving the rest of the JSDoc intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/extractor/doc-extractor.ts`:
- Line 3: Update the JSDoc in src/extractor/doc-extractor.ts to stop referencing
the removed tag: remove the obsolete "@architect-core" token from the top-level
comment so it only contains "@architect-extractor" (or other remaining valid
tags); locate the JSDoc block at the top of doc-extractor.ts and edit that
comment to delete "@architect-core" wherever it appears.
In `@src/scanner/ast-parser.ts`:
- Line 3: Remove the obsolete JSDoc tag from the file's header by editing the
doc comment that currently contains "@architect-core `@architect-scanner`" in
src/scanner/ast-parser.ts so it only includes the valid "@architect-scanner"
tag; locate the JSDoc block at the top of the file (the same pattern used in
registry-builder.ts) and delete the "@architect-core" token to avoid the
unknown-tag warning.
In `@src/taxonomy/registry-builder.ts`:
- Line 110: Update the JSDoc comment that still mentions "core" and "brief" so
it matches the actual tags array (now ['pattern', 'status', 'usecase']); remove
references to core and brief and change the example/comment text to "pattern:
Essential pattern identification (pattern, status, usecase)" or similar to
reflect only pattern, status, and usecase in the JSDoc block above the tags
array.
- Line 3: Remove the deprecated JSDoc tag `@architect-core` from the top-of-file
JSDoc in registry-builder.ts (the self-referential tag at the file header); edit
the file header comment to delete the "@architect-core" token so the scanner no
longer emits an "unknown-tag" warning while leaving the rest of the JSDoc
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eaad54ac-0aa7-4bea-bdbb-936052b4fab6
📒 Files selected for processing (14)
src/extractor/doc-extractor.tssrc/extractor/dual-source-extractor.tssrc/extractor/gherkin-extractor.tssrc/scanner/ast-parser.tssrc/taxonomy/registry-builder.tssrc/validation-schemas/doc-directive.tssrc/validation-schemas/dual-source.tssrc/validation-schemas/extracted-pattern.tstests/features/behavior/pattern-tag-extraction.featuretests/features/types/tag-registry-builder.featuretests/fixtures/pattern-factories.tstests/fixtures/scanner-fixtures.tstests/steps/behavior/pattern-tag-extraction.steps.tstests/support/helpers/assertions.ts
💤 Files with no reviewable changes (8)
- tests/features/types/tag-registry-builder.feature
- src/extractor/dual-source-extractor.ts
- src/validation-schemas/dual-source.ts
- src/validation-schemas/extracted-pattern.ts
- tests/fixtures/scanner-fixtures.ts
- tests/fixtures/pattern-factories.ts
- tests/support/helpers/assertions.ts
- src/validation-schemas/doc-directive.ts
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
@architect-brieftag — dead relic from early development (zero production consumers)@architect-coreflag — eliminate flag/category dual-registration;coreis now a category only'ddd'category fallback — use registry-based default from active presetinferCategoryfallback against registry@architect-shapefrom creating false lint violations14 files changed, 45 insertions, 106 deletions. Net -61 lines.
Approach: Root Cause Fixes Over Symptom Patches
This PR was triggered by 6 bugs found during tutorial review. The minimal fix would have been 6 individual patches (adding guards to lint rules, patching flag processing, updating metadata text). Instead, this PR addresses classes of issues through removals and structural changes:
@architect-shapeisShapecheck to 3 lint rules@architect-coresets flag not category in Gherkincorefrom metadata flags entirelyisCorehad zero production consumers. Removal eliminates the flag/category collision class.@architect-briefwrong metadata@architect-brieffrom taxonomy'ddd'fallback'core'config.tagRegistry?.categories[0]?.tagstatusto exclude listpriorityMap(registry categories)This approach should be the standard for all work in this package. The package is mission-critical infrastructure for a consumer monorepo (~600 files, 45K+ lines). A bug here means incorrect documentation/validation across ALL platform packages. Quality over convenience, always.
Why this matters for the product
From the product vision: "Architect provides the missing infrastructure: structured specifications that evolve into production code, a single source of truth."
A taxonomy system with dead tags, dual registrations, and hardcoded fallbacks undermines the "single source of truth" principle at the infrastructure level. If the tool that enforces architectural discipline has undisciplined internals, the entire value proposition is weakened. This PR aligns package internals with the product principle.
Implications for Consumer Repos
Breaking changes requiring action
@architect-briefremovedunknown tagwarning during extraction@architect-briefannotations — the description belongs inline in the@architectJSDoc block or in the Feature descriptionisCorefield removed fromExtractedPatternandDocDirective.isCorewill get a TypeScript errorpattern.isCorewithpattern.category === 'core'@architect-coreis now category-onlycategory: "core"(was silently ignored for category). In TypeScript: unchanged behavior.corefor libar-generic) instead ofdddcategory === 'ddd'— they may need updating if that category came from the fallbackTutorial repo (
delivery-process-tutorials)@architect-briefannotations from TypeScript source filesnpx architect-lint-patternsto verify 0 errors, 0 warningsConsumer monorepo (
libar-platform)@architect-briefin all packages — remove annotations.isCorein any custom tooling — replace with category checkpnpm architect:query -- overviewto verify no regressionsDeeper Refactoring Opportunities
During exploration, I noticed these opportunities that follow the same "root cause over patch" approach:
sequence-erroris the only remaining flag tag. The flag metadata format exists solely for this one tag. Consider whethersequence-errorshould instead be an enum value on asequence-roletag (alongsidesequence-orchestrator,sequence-step,sequence-module), which would allow removing theflagformat entirely from the registry schema.Gherkin
extractPatternTags()uses a module-levelTAG_LOOKUPbuilt frombuildRegistry()(line 88 ofgherkin-ast-parser.ts). This always uses the full DDD_ES_CQRS registry regardless of the active preset. It works because the scanner's job is to extract all possible metadata, but threading the actual registry through would be more precise and enable preset-specific validation at scan time.The
inferCategoryfallback chain indoc-extractor.ts(lines 457-518) has multiple fallback layers: priority-based matching → subsequence matching → first-tag extraction →'uncategorized'. The subsequence matching (checking all contiguous subsequences of hyphenated tag parts) is complex and could potentially match unintended categories. A simpler approach: exact match only, then'uncategorized'.The
ProcessMetadataSchemaindual-source.tsstill has several fields that may be dead or underused (similar to howbriefwas). A usage audit ofquarter,effort,effortActual,risk,workflow, andprioritywould identify candidates for the same treatment.Test plan
pnpm typecheck— passes cleanpnpm lint— passes cleanpnpm test— 8712 tests pass, 141 test filesnpx architect-lint-patterns— 0 errors after removing@architect-briefnpx architect list --fields patternName,category— correct categoriesSummary by CodeRabbit
Release Notes
Removals
@architect-coremetadata flag from pattern directives@architect-briefmetadata tag from pattern directivesImprovements