feat: detect archetypes from file-tree structural signals - #21
Conversation
Split archetype classification into mergeable signals and a single rule pass, then scan the project tree alongside package.json so frameworkless repos can still be classified. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 52 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughArchetype detection is refactored into pure, composable signal functions (packageSignals, mergeSignals, archetypesFromSignals) and extended with a bounded, symlink-safe file-tree scan (scanFileTreeSignals, classifyEntry) merged with package.json signals in detectArchetypesFromProject. Tests are expanded accordingly, and governance/process markdown and JSON artifacts document the change. ChangesArchetype signal-based detection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant detectArchetypesFromProject
participant readPackageSignals
participant scanFileTreeSignals
participant mergeSignals
participant archetypesFromSignals
Caller->>detectArchetypesFromProject: detectArchetypesFromProject(cwd)
detectArchetypesFromProject->>readPackageSignals: readPackageSignals(cwd)
readPackageSignals-->>detectArchetypesFromProject: pkgSig, packageJsonFound
detectArchetypesFromProject->>scanFileTreeSignals: scanFileTreeSignals(cwd)
scanFileTreeSignals-->>detectArchetypesFromProject: fileSignals
detectArchetypesFromProject->>mergeSignals: mergeSignals(pkgSig, fileSignals)
mergeSignals-->>detectArchetypesFromProject: merged signals
detectArchetypesFromProject->>archetypesFromSignals: archetypesFromSignals(merged)
archetypesFromSignals-->>detectArchetypesFromProject: archetypes
detectArchetypesFromProject-->>Caller: {archetypes, packageJsonFound}
Related Issues: None found in the provided context. Related PRs: None found in the provided context. Suggested labels: enhancement, tests, documentation Suggested reviewers: None specified in the provided context. 🐰 A rabbit hopped through trees of code, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Reframe CONSTITUTION.md for the installer’s threat model and expand ARCHITECTURE.md §5 to describe merged package.json and file-tree archetype signals; remove promoted draft files. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
tests/detect-archetype.test.ts (1)
160-166: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThis test doesn't verify
app/-scoping ofroute.tsdetection.
app/users/route.ts → backendconfirms the positive case, but there's no companion test for aroute.tsplaced outsideapp/(e.g. top-levelroute.ts) to pin that it should (per the documentedapp/**/route.tscontract) NOT triggerbackend. Given the scope gap flagged indetect-archetype.ts'sclassifyEntry, such a test would currently fail to catch the over-broad match since the implementation doesn't check ancestry.🤖 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 `@tests/detect-archetype.test.ts` around lines 160 - 166, The current `app/**/route.ts` test only covers the positive `detectArchetypesFromProject` case and misses the required `app/` scoping check. Add a companion assertion in `tests/detect-archetype.test.ts` that creates a top-level `route.ts` (outside `app/`) and verifies `detectArchetypesFromProject` does not return `backend`. This should pin the intended behavior of `classifyEntry` and catch any over-broad `route.ts` matching that ignores ancestry.src/lib/detect-archetype.ts (1)
47-49: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider extending
SKIP_DIRSto common generated/build directories.
.next,coverage,.turbo,out,.cacheare common in JS/TS repos and add scan cost (and potential noise) without any classification value, similar to the already-skippeddist/build.♻️ Proposed addition
-const SKIP_DIRS = new Set(['node_modules', '.git', 'dist', 'build']); +const SKIP_DIRS = new Set([ + 'node_modules', + '.git', + 'dist', + 'build', + '.next', + 'coverage', + '.turbo', + 'out', + '.cache', +]);🤖 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 `@src/lib/detect-archetype.ts` around lines 47 - 49, Extend the SKIP_DIRS set in detect-archetype.ts to include common generated/build directories such as .next, coverage, .turbo, out, and .cache. Update the directory-skipping logic used by the archetype detection flow so these paths are treated the same as node_modules, .git, dist, and build, keeping the check case-insensitive and avoiding recursion into them.
🤖 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 @.dev/features/archetype-file-tree-scan/PLAN.md:
- Around line 80-82: The current plan text overstates the fallback in the
directory walk; `walkDirectory` only handles `readdirSync(dir)` failures at the
directory level and does not recover per-entry `stat` errors. Update the wording
in this section to describe the actual behavior of `walkDirectory` and its
short-circuiting logic without claiming narrowly caught per-entry recovery.
- Around line 45-47: Clarify the route-handler rule in the archetype scan plan:
the current heuristic text in PLAN.md makes it sound like only App Router files
under app/**/route.ts should trigger backend, but the classifier matches any
route.{ts,tsx,js,mjs} entry name anywhere in the tree. Update the wording to
explicitly state the broader filename-based behavior, or change the scan logic
to use path-aware matching if that narrower scope is intended, referencing the
route-handler/backend signal guidance in the plan.
- Around line 98-114: The eval plan is missing coverage for the manifest-less
frontend path and the literal-file variants, so add representative fixtures to
lock those branches down. In the archetype scan test plan, include a case for
packageJsonFound:false with a .tsx signal resolving to spa, and add one fixture
each for next.config.ts/cjs and route.tsx/js/mjs so those literal branches are
explicitly exercised. Keep the additions aligned with the existing
detect-archetype.test.ts / archetype.test.ts coverage style so the new cases
validate the same signal-merge behavior.
In @.dev/features/archetype-file-tree-scan/REVIEW.md:
- Around line 19-21: The coverage summary in the archetype-file-tree-scan review
is overstating the test coverage: it says every new function/behavior is tested,
but the same review notes gaps for route/next.config variants and the truncation
path. Update the summary text to match the actual coverage level, keeping the
wording consistent with the later coverage notes in REVIEW.md and the overall
regression-spec assessment.
In `@src/lib/detect-archetype.ts`:
- Around line 99-135: `scanFileTreeSignals` still materializes and sorts every
directory entry before `MAX_ENTRIES` or `allFound()` can stop traversal, so the
defensive cap is not actually limiting worst-case work. Update the directory
walk in `scanFileTreeSignals` (and its `readEntries`/`walk` flow) to use an
incremental directory iterator instead of `readdirSync` plus full-array
`sort()`, so processing can stop early without reading the entire listing; keep
the existing `classifyEntry`, `SKIP_DIRS`, and symlink checks in the per-entry
loop.
- Around line 68-86: The backend signal in classifyEntry() is too broad because
it marks any bare route.ts/tsx/js/mjs file as an App Router handler. Update the
detection to require app/** context before setting backend for route.* entries,
and thread the parent path or under-app flag from scanFileTreeSignals() into
classifyEntry() so only routes inside app/ are classified as backend.
---
Nitpick comments:
In `@src/lib/detect-archetype.ts`:
- Around line 47-49: Extend the SKIP_DIRS set in detect-archetype.ts to include
common generated/build directories such as .next, coverage, .turbo, out, and
.cache. Update the directory-skipping logic used by the archetype detection flow
so these paths are treated the same as node_modules, .git, dist, and build,
keeping the check case-insensitive and avoiding recursion into them.
In `@tests/detect-archetype.test.ts`:
- Around line 160-166: The current `app/**/route.ts` test only covers the
positive `detectArchetypesFromProject` case and misses the required `app/`
scoping check. Add a companion assertion in `tests/detect-archetype.test.ts`
that creates a top-level `route.ts` (outside `app/`) and verifies
`detectArchetypesFromProject` does not return `backend`. This should pin the
intended behavior of `classifyEntry` and catch any over-broad `route.ts`
matching that ignores ancestry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 840fad05-736e-498f-98a2-3a85f3c68ff3
📒 Files selected for processing (15)
.dev/features/archetype-file-tree-scan/GRILL.md.dev/features/archetype-file-tree-scan/PLAN.md.dev/features/archetype-file-tree-scan/REGRESSION.md.dev/features/archetype-file-tree-scan/REVIEW.md.dev/features/archetype-file-tree-scan/SHIP.md.dev/features/archetype-file-tree-scan/VERIFY.md.dev/features/archetype-file-tree-scan/regression-report.json.dev/features/archetype-file-tree-scan/verify-report.json.pharn/pharn-dev-verify/results.json.pharn/writes-scope.jsonsrc/lib/archetype.tssrc/lib/detect-archetype.tssrc/types.tstests/archetype.test.tstests/detect-archetype.test.ts
| - a directory named `api` (covers top-level `api/` and `pages/api`), **or** a `route.{ts,tsx,js,mjs}` file | ||
| (App-Router route handlers, `app/**/route.ts`) → `backend` signal → `backend`. | ||
| - `.sql` / `migrations/` → **not scanned** (decision #2). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify the route-handler heuristic.
This reads as if only app/**/route.ts should count, but the classifier only sees entry names. In practice, any route.{ts,tsx,js,mjs} file anywhere in the tree will trip backend. If that broader heuristic is intentional, say so; otherwise the scanner needs path-aware matching.
Suggested wording change
- - a directory named `api` (covers top-level `api/` and `pages/api`), **or** a `route.{ts,tsx,js,mjs}` file
- (App-Router route handlers, `app/**/route.ts`) → `backend` signal → `backend`.
+ - a directory named `api` (covers top-level `api/` and `pages/api`), **or** any `route.{ts,tsx,js,mjs}` file
+ anywhere in the tree → `backend` signal → `backend`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - a directory named `api` (covers top-level `api/` and `pages/api`), **or** a `route.{ts,tsx,js,mjs}` file | |
| (App-Router route handlers, `app/**/route.ts`) → `backend` signal → `backend`. | |
| - `.sql` / `migrations/` → **not scanned** (decision #2). | |
| - a directory named `api` (covers top-level `api/` and `pages/api`), **or** any `route.{ts,tsx,js,mjs}` file | |
| anywhere in the tree → `backend` signal → `backend`. | |
| - `.sql` / `migrations/` → **not scanned** (decision `#2`). |
🤖 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 @.dev/features/archetype-file-tree-scan/PLAN.md around lines 45 - 47, Clarify
the route-handler rule in the archetype scan plan: the current heuristic text in
PLAN.md makes it sound like only App Router files under app/**/route.ts should
trigger backend, but the classifier matches any route.{ts,tsx,js,mjs} entry name
anywhere in the tree. Update the wording to explicitly state the broader
filename-based behavior, or change the scan logic to use path-aware matching if
that narrower scope is intended, referencing the route-handler/backend signal
guidance in the plan.
| Short-circuit once all three booleans are true. A per-entry `readdir`/`stat` error is narrowly caught and | ||
| that subtree contributes no signal (a deterministic default, mirroring the existing malformed-`package.json` | ||
| → `lib` handling — **not** a blanket swallow). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Match the error-handling claim to the actual fallback.
The walk only catches readdirSync(dir) at the directory boundary. There is no per-entry stat recovery path here, so the current wording overstates how granular the fallback is.
Suggested wording change
- - **Deterministic (P5):** signals are booleans → OR-merge is order-independent; per-directory entries are
- **sorted by name** before traversal, so even a cap-truncated walk visits the same set on any filesystem.
- Short-circuit once all three booleans are true. A per-entry `readdir`/`stat` error is narrowly caught and
- that subtree contributes no signal (a deterministic default, mirroring the existing malformed-`package.json`
+ - **Deterministic (P5):** signals are booleans → OR-merge is order-independent; per-directory entries are
+ **sorted by name** before traversal, so even a cap-truncated walk visits the same set on any filesystem.
+ Short-circuit once all three booleans are true. A per-directory `readdir` error is narrowly caught and
+ that subtree contributes no signal (a deterministic default, mirroring the existing malformed-`package.json`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Short-circuit once all three booleans are true. A per-entry `readdir`/`stat` error is narrowly caught and | |
| that subtree contributes no signal (a deterministic default, mirroring the existing malformed-`package.json` | |
| → `lib` handling — **not** a blanket swallow). | |
| Short-circuit once all three booleans are true. A per-directory `readdir` error is narrowly caught and | |
| that subtree contributes no signal (a deterministic default, mirroring the existing malformed-`package.json` | |
| → `lib` handling — **not** a blanket swallow). |
🤖 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 @.dev/features/archetype-file-tree-scan/PLAN.md around lines 80 - 82, The
current plan text overstates the fallback in the directory walk; `walkDirectory`
only handles `readdirSync(dir)` failures at the directory level and does not
recover per-entry `stat` errors. Update the wording in this section to describe
the actual behavior of `walkDirectory` and its short-circuiting logic without
claiming narrowly caught per-entry recovery.
| - **`.tsx` present, NO `react` dep → `spa`** (frontend detected from files, not package.json) — the | ||
| description's headline case. | ||
| - **Pure-backend tree** (`api/` dir, no `.tsx`) → `backend`, **never `spa`** — the required inverse. | ||
| - **`next.config.js` in tree, no `next` dep → `ssr`.** | ||
| - **`route.ts` under `app/` → `backend`; `api/` dir → `backend`.** | ||
| - **Merge SSR-gating (the key correctness case):** package.json `react` (→ would be `spa`) + file tree | ||
| `next.config.js` (→ `ssr`) → merged **`['ssr']` only** — proves signals-merge-then-rule (decision #3), | ||
| not union-of-sets. | ||
| - **Merge additive:** package.json `express` + file tree `.tsx` → `['backend','spa']`. | ||
| - **Bounded walk:** a `.tsx` under `node_modules/` is **skipped** → no `spa` (proves the skip-list). | ||
| - **`.sql`/`migrations/` present → contributes nothing** (decision #2, pinned so a future `db` change is a | ||
| deliberate edit). | ||
| - **Determinism:** the same tree scanned twice → equal result. | ||
| - **Backward-compatible:** every existing `detect-archetype.test.ts` / `archetype.test.ts` case still passes | ||
| (package.json-only fixtures have no file signals → unchanged). | ||
| - **Pure units:** `archetypesFromSignals({clientUi:true, ssr:true})` → `['ssr']` (gating); | ||
| `mergeSignals` ORs field-wise. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Pin the no-manifest and literal-variant cases.
The eval list covers the headline spa case, but it still doesn't explicitly lock the manifest-less path the PR summary calls out (packageJsonFound:false + .tsx → spa). It also leaves several literal branches unpinned (next.config.ts/cjs, route.tsx/js/mjs). Please add one representative fixture for each family.
Suggested additions
- **`.tsx` present, NO `react` dep → `spa`** (frontend detected from files, not package.json) — the
description's headline case.
+- **No `package.json` + `.tsx` → `spa` and `packageJsonFound:false`.**
+- **Variant coverage:** `next.config.ts/cjs` and `route.tsx/js/mjs`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **`.tsx` present, NO `react` dep → `spa`** (frontend detected from files, not package.json) — the | |
| description's headline case. | |
| - **Pure-backend tree** (`api/` dir, no `.tsx`) → `backend`, **never `spa`** — the required inverse. | |
| - **`next.config.js` in tree, no `next` dep → `ssr`.** | |
| - **`route.ts` under `app/` → `backend`; `api/` dir → `backend`.** | |
| - **Merge SSR-gating (the key correctness case):** package.json `react` (→ would be `spa`) + file tree | |
| `next.config.js` (→ `ssr`) → merged **`['ssr']` only** — proves signals-merge-then-rule (decision #3), | |
| not union-of-sets. | |
| - **Merge additive:** package.json `express` + file tree `.tsx` → `['backend','spa']`. | |
| - **Bounded walk:** a `.tsx` under `node_modules/` is **skipped** → no `spa` (proves the skip-list). | |
| - **`.sql`/`migrations/` present → contributes nothing** (decision #2, pinned so a future `db` change is a | |
| deliberate edit). | |
| - **Determinism:** the same tree scanned twice → equal result. | |
| - **Backward-compatible:** every existing `detect-archetype.test.ts` / `archetype.test.ts` case still passes | |
| (package.json-only fixtures have no file signals → unchanged). | |
| - **Pure units:** `archetypesFromSignals({clientUi:true, ssr:true})` → `['ssr']` (gating); | |
| `mergeSignals` ORs field-wise. | |
| - **`.tsx` present, NO `react` dep → `spa`** (frontend detected from files, not package.json) — the | |
| description's headline case. | |
| - **No `package.json` + `.tsx` → `spa` and `packageJsonFound:false`.** | |
| - **Variant coverage:** `next.config.ts/cjs` and `route.tsx/js/mjs`. | |
| - **Pure-backend tree** (`api/` dir, no `.tsx`) → `backend`, **never `spa`** — the required inverse. | |
| - **`next.config.js` in tree, no `next` dep → `ssr`.** | |
| - **`route.ts` under `app/` → `backend`; `api/` dir → `backend`.** | |
| - **Merge SSR-gating (the key correctness case):** package.json `react` (→ would be `spa`) + file tree | |
| `next.config.js` (→ `ssr`) → merged **`['ssr']` only** — proves signals-merge-then-rule (decision `#3`), | |
| not union-of-sets. | |
| - **Merge additive:** package.json `express` + file tree `.tsx` → `['backend','spa']`. | |
| - **Bounded walk:** a `.tsx` under `node_modules/` is **skipped** → no `spa` (proves the skip-list). | |
| - **`.sql`/`migrations/` present → contributes nothing** (decision `#2`, pinned so a future `db` change is a | |
| deliberate edit). | |
| - **Determinism:** the same tree scanned twice → equal result. | |
| - **Backward-compatible:** every existing `detect-archetype.test.ts` / `archetype.test.ts` case still passes | |
| (package.json-only fixtures have no file signals → unchanged). | |
| - **Pure units:** `archetypesFromSignals({clientUi:true, ssr:true})` → `['ssr']` (gating); | |
| `mergeSignals` ORs field-wise. |
🤖 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 @.dev/features/archetype-file-tree-scan/PLAN.md around lines 98 - 114, The
eval plan is missing coverage for the manifest-less frontend path and the
literal-file variants, so add representative fixtures to lock those branches
down. In the archetype scan test plan, include a case for packageJsonFound:false
with a .tsx signal resolving to spa, and add one fixture each for
next.config.ts/cjs and route.tsx/js/mjs so those literal branches are explicitly
exercised. Keep the additions aligned with the existing detect-archetype.test.ts
/ archetype.test.ts coverage style so the new cases validate the same
signal-merge behavior.
| - **L-eval → P1:** Not a Capability (no `role:` frontmatter) → P1's Capability⇒eval does not bind; the | ||
| vitest suite is its regression-spec, and every new function/behavior is tested (401/401). Two minor | ||
| coverage gaps below. No missing eval-binding; floor agrees (validate GREEN). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tighten the coverage summary.
The headline says every new function/behavior is tested, but this same review later calls out untested route/next.config variants and the truncation path. Rephrase the summary so it matches the actual coverage level.
🤖 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 @.dev/features/archetype-file-tree-scan/REVIEW.md around lines 19 - 21, The
coverage summary in the archetype-file-tree-scan review is overstating the test
coverage: it says every new function/behavior is tested, but the same review
notes gaps for route/next.config variants and the truncation path. Update the
summary text to match the actual coverage level, keeping the wording consistent
with the later coverage notes in REVIEW.md and the overall regression-spec
assessment.
| function classifyEntry(name: string, isDir: boolean): ArchetypeSignals { | ||
| const lower = name.toLowerCase(); | ||
| if (isDir) { | ||
| // A dir named `api` (covers a top-level `api/` and `pages/api`) → backend. | ||
| return { ssr: false, backend: lower === 'api', clientUi: false }; | ||
| } | ||
| return { | ||
| // `next.config.{js,ts,mjs,cjs,…}` → an SSR meta-framework config. | ||
| ssr: lower.startsWith('next.config.'), | ||
| // App-Router route handlers (`app/**/route.ts`) → a backend surface. | ||
| backend: | ||
| lower === 'route.ts' || | ||
| lower === 'route.tsx' || | ||
| lower === 'route.js' || | ||
| lower === 'route.mjs', | ||
| // A `.tsx` / `.jsx` file anywhere → a client-UI (frontend) signal. | ||
| clientUi: lower.endsWith('.tsx') || lower.endsWith('.jsx'), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'src/lib/detect-archetype.ts' 'src/types.ts' 'src/**/*.test.ts' 'src/**/*.spec.ts' | sed 's#^`#-` #'
printf '\n== outline: src/lib/detect-archetype.ts ==\n'
ast-grep outline src/lib/detect-archetype.ts --view expanded || true
printf '\n== outline: src/types.ts ==\n'
ast-grep outline src/types.ts --view expanded || true
printf '\n== relevant text search ==\n'
rg -n "route\\.(ts|tsx|js|mjs)|app/\\*\\*/route|App-Router|backend surface|classifyEntry|detect-archetype" src -SRepository: pharn-dev/pharn-cli
Length of output: 4961
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/lib/detect-archetype.ts (lines 1-220) ==\n'
nl -ba src/lib/detect-archetype.ts | sed -n '1,220p'
printf '\n== src/types.ts (relevant range) ==\n'
nl -ba src/types.ts | sed -n '1,220p'Repository: pharn-dev/pharn-cli
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/lib/detect-archetype.ts (selected lines) ==\n'
sed -n '60,140p' src/lib/detect-archetype.ts | cat -n
printf '\n== src/types.ts (around archetype comments) ==\n'
sed -n '178,194p' src/types.ts | cat -n
printf '\n== docs/tests mentioning route handler scope ==\n'
rg -n "route\\.ts|route\\.tsx|route\\.js|route\\.mjs|app/\\*\\*/route|App-Router route handlers|backend surface" src README.md . -SRepository: pharn-dev/pharn-cli
Length of output: 6643
Scope route.* detection to app/**
classifyEntry() currently treats any bare route.ts/tsx/js/mjs file as backend, but this detector is meant to catch App Router handlers under app/**. Since scanFileTreeSignals() only passes the entry name, an unrelated route.* elsewhere in the tree will be misclassified. Thread the path/under-app context into the check before merging.
🤖 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 `@src/lib/detect-archetype.ts` around lines 68 - 86, The backend signal in
classifyEntry() is too broad because it marks any bare route.ts/tsx/js/mjs file
as an App Router handler. Update the detection to require app/** context before
setting backend for route.* entries, and thread the parent path or under-app
flag from scanFileTreeSignals() into classifyEntry() so only routes inside app/
are classified as backend.
| export function scanFileTreeSignals(root: string): ArchetypeSignals { | ||
| let acc: ArchetypeSignals = { ssr: false, backend: false, clientUi: false }; | ||
| let budget = MAX_ENTRIES; | ||
|
|
||
| const allFound = (): boolean => acc.ssr && acc.backend && acc.clientUi; | ||
|
|
||
| const readEntries = (dir: string) => { | ||
| try { | ||
| return readdirSync(dir, { withFileTypes: true }); | ||
| } catch { | ||
| return []; // unreadable subtree → no signal (deterministic default) | ||
| } | ||
| }; | ||
|
|
||
| const walk = (dir: string, depth: number): void => { | ||
| if (depth > MAX_DEPTH || budget <= 0 || allFound()) return; | ||
| const entries = readEntries(dir).sort((a, b) => | ||
| a.name < b.name ? -1 : a.name > b.name ? 1 : 0, | ||
| ); | ||
| for (const entry of entries) { | ||
| if (budget <= 0 || allFound()) return; | ||
| const name = entry.name; | ||
| // Never follow or classify a symlink → no escape past `root` (P2). | ||
| if (entry.isSymbolicLink()) continue; | ||
| const isDir = entry.isDirectory(); | ||
| if (isDir && SKIP_DIRS.has(name.toLowerCase())) continue; | ||
| if (!isDir && name.toLowerCase().startsWith('.env')) continue; | ||
| if (!isDir && !entry.isFile()) continue; // sockets/fifos/etc.: not signals | ||
| budget -= 1; | ||
| acc = mergeSignals(acc, classifyEntry(name, isDir)); | ||
| if (isDir) walk(join(dir, name), depth + 1); | ||
| } | ||
| }; | ||
|
|
||
| walk(root, 0); | ||
| return acc; | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
readdirSync + full-array sort() run before the entry-level cap can short-circuit.
MAX_ENTRIES/allFound() are only checked once per entry inside the for loop (lines 118-119), but readEntries(dir) (line 107) and .sort() (lines 115-117) always process the entire directory listing first. SKIP_DIRS protects node_modules-style trees since the skip check happens before recursion, but any other single directory with a very large number of direct entries (not on the skip list) still pays full read+sort cost before the cap engages — defeating the doc's stated intent that these caps are "a DEFENSIVE bound on a pathological tree, NOT a perf-only knob" over untrusted project input (P2).
A true per-directory bound would require a streaming read (e.g. fs.opendirSync + iterative readSync) that can stop before materializing/sorting the full listing.
🤖 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 `@src/lib/detect-archetype.ts` around lines 99 - 135, `scanFileTreeSignals`
still materializes and sorts every directory entry before `MAX_ENTRIES` or
`allFound()` can stop traversal, so the defensive cap is not actually limiting
worst-case work. Update the directory walk in `scanFileTreeSignals` (and its
`readEntries`/`walk` flow) to use an incremental directory iterator instead of
`readdirSync` plus full-array `sort()`, so processing can stop early without
reading the entire listing; keep the existing `classifyEntry`, `SKIP_DIRS`, and
symlink checks in the per-entry loop.
1 participant
Summary
ArchetypeSignals(packageSignals,mergeSignals,archetypesFromSignals) so classification runs once over combined facts.detect-archetype.tswith a bounded file-tree scan (.tsx/.jsx,next.config.*,api/dirs,app/**/route.ts, etc.) and merges those signals withpackage.jsondependency names.package.jsonis missing or incomplete (e.g..tsxalone →spa).Test plan
npm run check(format, lint, typecheck, test)npx vitest run tests/archetype.test.ts tests/detect-archetype.test.ts.tsxwithout package.json, backend fromapi/, ssr fromnext.config.jsreactdep +next.config→ssronly (spa suppressed)node_modules/anddist/ignoredMade with Cursor
Summary by CodeRabbit
New Features
package.json.Bug Fixes
Tests