feat(core): add architecture/directory-naming - #325
Conversation
…idence enumeration
…partly-mistyped value
…y cannot shadow a live one
…architecture rules
The extraction brief condensed the tie-break rationale and the bare-prefix guard's self-inert consequence out of matchKeys's doc comment. Both document real review findings from the original rule and need to survive the move.
…e three tests bite
…ake the depth test bite
…plugin Proves architecture/directory-naming is reached from both entry points that build the sourceFiles inventory it depends on (packages/cli/src/index.ts and packages/vite/src/analyze.ts) rather than only from a provider-level test that would still pass with the rule never running. The vite-side test uses its own temp project rather than the existing unit-entry-file wiring fixture: a directory name starting with an uppercase letter (Fair_Summary) also satisfies unit-entry-file's isPascalCase gate, so sharing that fixture would double-count and break both tests.
…e a silent declaration architecture/unit-entry-file's violation result keyed both `route` and `location` on the same file, so a directory with no direct child (falling back to a file in its subtree) and a nested directory taking that file as its own direct child could collide on `findingKey` (id::route::location) and share a baseline/suppression identity. Key `route` on the directory instead, matching the fix directory-naming already had for itself. Also close a silent gap in architecture/directory-naming: a declaration value naming no casing at all (e.g. '|' or blank) was dropped from matching but slipped past both classification passes, so it checked nothing without ever being reported for it. Update declarations.ts's shared doc comments (reportAt, matchKeys, createKeyCompiler) to state what's actually invariant now that both rules key `route` on the directory, and to explain the exclude/bareGuard interaction that was lost when the module was extracted. Rename a test and its comments from "longest matching key" to "more path segments" to match the branch's segment-count specificity metric. Document the cross-rule interaction on the unit-entry-file rule pages (en/ja) to match directory-naming's existing note.
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds the configurable ChangesDirectory naming and declaration matching
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Analyzer
participant RuleContext
participant DirectoryNaming
participant Declarations
participant Casing
Analyzer->>RuleContext: provide sourceFiles and rule options
RuleContext->>DirectoryNaming: run check
DirectoryNaming->>Declarations: match declarations and exclusions
DirectoryNaming->>Casing: decode segments and validate casing
Casing-->>DirectoryNaming: casing result
DirectoryNaming-->>Analyzer: return findings
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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: 1
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-07-29-directory-naming.md (1)
856-864: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLabel the literal output blocks as text.
The fences at Lines 856 and 862 trigger MD040. Use
textfence labels to keep Markdown checks clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-07-29-directory-naming.md` around lines 856 - 864, Label both fenced output blocks in the documented example with the text language identifier, preserving their contents and wording unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-07-29-directory-naming-design.md`:
- Line 256: Add a language identifier to both fenced code blocks in the
directory naming design document, using text or another appropriate language
marker, so the markdownlint MD040 violations are resolved while preserving the
excerpt contents.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-07-29-directory-naming.md`:
- Around line 856-864: Label both fenced output blocks in the documented example
with the text language identifier, preserving their contents and wording
unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 14c84ca0-19ea-4b57-ac0c-7eda2982c997
📒 Files selected for processing (33)
.changeset/directory-naming.md.changeset/unit-entry-file.mddocs/src/content/docs/guides/(setup)/configuration.mdxdocs/src/content/docs/ja/guides/(setup)/configuration.mdxdocs/src/content/docs/ja/rules/architecture/directory-naming.mddocs/src/content/docs/ja/rules/architecture/index.mdxdocs/src/content/docs/ja/rules/architecture/unit-entry-file.mddocs/src/content/docs/ja/rules/index.mdxdocs/src/content/docs/rules/architecture/directory-naming.mddocs/src/content/docs/rules/architecture/index.mdxdocs/src/content/docs/rules/architecture/unit-entry-file.mddocs/src/content/docs/rules/index.mdxdocs/superpowers/plans/2026-07-29-directory-naming.mddocs/superpowers/specs/2026-07-28-unit-entry-file-design.mddocs/superpowers/specs/2026-07-29-directory-naming-design.mdpackages/cli/test/analyze-project.test.tspackages/cli/test/fixtures/directory-naming-project/package.jsonpackages/cli/test/fixtures/directory-naming-project/src/app.htmlpackages/cli/test/fixtures/directory-naming-project/src/lib/Fair_Summary/index.tspackages/cli/test/fixtures/directory-naming-project/src/routes/+page.sveltepackages/cli/test/fixtures/directory-naming-project/svelte-vitals.config.mjspackages/core/src/index.tspackages/core/src/rules/architecture/casing.tspackages/core/src/rules/architecture/declarations.tspackages/core/src/rules/architecture/directory-naming.tspackages/core/src/rules/architecture/unit-entry-file.tspackages/core/src/rules/index.tspackages/core/test/casing.test.tspackages/core/test/declarations.test.tspackages/core/test/directory-naming-example.test.tspackages/core/test/directory-naming.test.tspackages/core/test/unit-entry-file.test.tspackages/vite/test/analyze-source-files.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/superpowers/specs/2026-07-28-unit-entry-file-design.md (1)
344-355: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale inert-finding checklist.
This section now says unmatched keys are combined into one project-scoped finding, but Line 407 still says there is one finding per key. Update the checklist so the specification describes a single result shape consistently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-28-unit-entry-file-design.md` around lines 344 - 355, Update the inert-finding checklist near the stale “one finding per key” statement to specify one combined project-scoped finding for all unmatched option keys. Keep the result shape consistent with the surrounding specification: no route, no location, and presence set to none.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-07-29-directory-naming-design.md`:
- Around line 166-170: Constrain the bracket-segment decoding rules in the
route-directory naming design to directories beneath a recognized routes root,
so literal non-route directories such as src/lib/[foo]/ retain their actual
names for casing validation. Update the documented behavior and add coverage for
both route-root decoding and non-route literal directories, unless the
implementation intentionally keeps global decoding and explicitly documents that
behavior.
---
Outside diff comments:
In `@docs/superpowers/specs/2026-07-28-unit-entry-file-design.md`:
- Around line 344-355: Update the inert-finding checklist near the stale “one
finding per key” statement to specify one combined project-scoped finding for
all unmatched option keys. Keep the result shape consistent with the surrounding
specification: no route, no location, and presence set to none.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6711360a-7f7a-478d-9bc1-b7eda52a2a31
📒 Files selected for processing (20)
.changeset/directory-naming.mddocs/src/content/docs/ja/rules/architecture/directory-naming.mddocs/src/content/docs/rules/architecture/directory-naming.mddocs/superpowers/plans/2026-07-28-private-scope-import.mddocs/superpowers/plans/2026-07-28-unit-entry-file.mddocs/superpowers/plans/2026-07-29-directory-naming.mddocs/superpowers/specs/2026-07-28-private-scope-import-design.mddocs/superpowers/specs/2026-07-28-unit-entry-file-design.mddocs/superpowers/specs/2026-07-29-directory-naming-design.mdpackages/cli/test/analyze-project.test.tspackages/cli/test/fixtures/directory-naming-project/src/lib/Price_Table/index.tspackages/core/src/rules/architecture/casing.tspackages/core/test/casing.test.tspackages/core/test/declarations.test.tspackages/core/test/directory-naming-example.test.tspackages/core/test/directory-naming.test.tspackages/core/test/private-scope-import.test.tspackages/core/test/unit-entry-file-example.test.tspackages/core/test/unit-entry-file.test.tspackages/vite/test/analyze-source-files.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/core/test/declarations.test.ts
- .changeset/directory-naming.md
- packages/cli/test/analyze-project.test.ts
- packages/core/test/directory-naming-example.test.ts
- packages/core/test/directory-naming.test.ts
- packages/core/test/unit-entry-file.test.ts
- packages/core/src/rules/architecture/casing.ts
- docs/src/content/docs/ja/rules/architecture/directory-naming.md
- docs/superpowers/plans/2026-07-29-directory-naming.md
- packages/core/test/casing.test.ts
| | `[itemId]` | `itemId` | | ||
| | `[itemId=integer]` | `itemId` | | ||
| | `[...rest]` | `rest` | | ||
| | `[[optional]]` | `optional` | | ||
| | `(app)` | `app` | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope SvelteKit decoding to route directories.
Shape-only decoding can reinterpret a literal directory such as src/lib/[foo]/ as foo, causing a configured non-route directory to bypass its intended casing check. Decode bracket syntax only under a recognized routes root, or explicitly document and test the global behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-29-directory-naming-design.md` around lines
166 - 170, Constrain the bracket-segment decoding rules in the route-directory
naming design to directories beneath a recognized routes root, so literal
non-route directories such as src/lib/[foo]/ retain their actual names for
casing validation. Update the documented behavior and add coverage for both
route-root decoding and non-route literal directories, unless the implementation
intentionally keeps global decoding and explicitly documents that behavior.
|
Fixed in afba84f. The stale Chasing it turned up two more statements in the same spec that this branch had superseded and the review had no way to see, since they fall outside its diff:
All three were documentation lagging behind code, not behaviour — 1,981 tests unchanged. |
1 participant
Adds
architecture/directory-naming, the charter's M2 mechanism: a directory should be named in the casing its location declares. Like the other Architecture convention rules it is L3 — it emits nothing at all untildirectoriesis set.Four casing names are recognised —
camelCase,PascalCase,kebab-case,snake_case— and a value may name several, joined by|, for a location that legitimately holds more than one kind of directory. Each is tested against the whole name rather than its first character, which is what lets a project distinguishclear-cachefromclearCache.SvelteKit route syntax is decoded before the check, so a declaration reaching into
src/routes/is usable:[itemId=integer]is judged asitemId,(app)asapp,[...rest]asrest. A compound segment such as[foo]-[bar]names no single identifier and is skipped, as is any name with no letter in it —2024carries no casing, and a year-archive route cannot be renamed without changing its URL.Two corrections to
architecture/unit-entry-fileBoth rules now share the glob machinery the first one grew, and extracting it exposed two defects in it. Neither has been released — that rule's changeset is still pending — so no user has seen the old behaviour.
Declaration keys are ordered by depth, not string length. Raw length inverts specificity whenever
*and**compete at the same position:src/lib/features/**is one character longer thansrc/lib/features/*, so the broader key won and the narrower declaration silently did nothing. The order is now more path segments, then fewer**segments, then longer key, then lexicographically first.A declaration whose every match is removed by
excludeis reported. Bookkeeping ran before the exclusion check, so such a key counted as used even though it evaluates nothing. An excluded directory is one the rule is forbidden to look at, which is different in kind from one it looked at and had nothing to say about. The finding now names which of the two happened, because the remedies differ — a typo in the glob, or a contradiction between two options you can both see.A violation now keys
routeon the directory andlocationon a file inside it.findingKeyisid::route::location, and a violating directory nested inside another can resolve to the same file — the outer by falling back to the subtree, the inner by taking its direct child. With both fields set to that file the two findings shared an identity, so baselining either silently took both. This lands here rather than later because merging this branch is what publishes the rule and freezes that key's shape.Notes
locationstill points at a file in every case:filterToChangedFileskeeps only paths git lists as changed, and git never lists a directory, so a directory-keyed finding would vanish from every--diffrun.The rule emits no pass results.
computeScoreseeds every distinctrouteat 100 and averages, and a pass per directory would add hundreds of 100s from a single'src/routes/**'declaration.The two rules deliberately disagree about what "PascalCase" means — the older one asks only whether the first character is A-Z, because it is asking whether a directory looks like a unit, not whether its name conforms. Both rule pages state their own definition.
Verification
lint, format and typecheck clean; 1,981 tests pass (core 992, cli 768, vite 196, mcp 25). The documented configuration example is held to being real by a test that asserts it examines directories and leaves no declaration reported, and the
excludeexample is asserted in both directions.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
architecture/directory-namingrule to validate configured directory casing conventions.Bug Fixes
Documentation