Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 0
feat: align DB signals with backend archetype (enum has no db member)#22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # GRILL — archetype-enum-align (ADVISORY) | ||
| Plan under interrogation: `.dev/features/archetype-enum-align/PLAN.md`. | ||
| Spec-hash check: **MATCH** — `sha256(ARCHITECTURE.md)` recomputed = `bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e`, equals the plan's `spec_content_hash`. No drift (the block on drift is `/pharn-dev-build`'s floor-gate, not this stage — fix #4). | ||
| Registered grillers (membership, FLOOR): `node .dev/floor/count-grillers.mjs .` → `{"registered":0}`. No pluggable grillers; inline axes only. | ||
| All findings below are **advisory** and gate nothing (fix #3). Enum-gated fields (`type`/`rule_id`/`severity`/`file`) are my own membership/path assertions; free-text `problem`/`evidence` quote the plan and inherit its untrusted tag (rendered as DATA). | ||
| ## Findings | ||
| ### Axis: Eval coverage / honest scope (P1, P7) | ||
| ```yaml | ||
| - type: FINDING | ||
| rule_id: P1 | ||
| severity: important | ||
| file: ".dev/features/archetype-enum-align/PLAN.md:34" | ||
| problem: "The Files section under-scopes the test edits: it names only detect-archetype.test.ts:209-216 and 'add cases to archetype.test.ts', but an EXISTING passing test — tests/archetype.test.ts:37-41 — pins the OLD 'prisma adds no archetype' behavior and will flip to FAIL under the approved DB→backend change." | ||
| evidence: "PLAN Files: '- tests/archetype.test.ts — add package-name cases …'. But archetype.test.ts:37-41 is [ 'next + prisma + drizzle → ssr (libs add no archetype)', deps('next','prisma'), ['ssr'] ] — with prisma→backend this becomes ['ssr','backend'], so expected ['ssr'] fails and the label 'libs add no archetype' is now false." | ||
| ``` | ||
| Concretely, `/pharn-dev-build` must ALSO update `tests/archetype.test.ts:37-41`: change `expected` from `['ssr']` to `['ssr','backend']` and fix the label (drop "libs add no archetype"). If the build follows the plan literally (only add cases + flip 209-216), `npm test` REDs on this line. The build's own `npm test` floor-gate would catch it, but the plan should have named it — this is the reversal's second, unlisted test-pin. | ||
| ### Axis: Detection completeness for the in-scope libs (P7 — not speculation) | ||
| ```yaml | ||
| - type: FINDING | ||
| rule_id: P7 | ||
| severity: minor | ||
| file: ".dev/features/archetype-enum-align/PLAN.md:32" | ||
| problem: "The backend package allowlist adds prisma / @prisma/client / drizzle-orm, but a schema-/migration-only Drizzle project commonly carries ONLY drizzle-kit (a devDependency) and no drizzle-orm runtime dep — so such a project would miss the package signal (though a .sql file or migrations/ dir would still catch it via the file-tree path)." | ||
| evidence: "PLAN Files: 'add ORM package names (prisma, @prisma/client, drizzle-orm) to BACKEND_FRAMEWORKS'. tests/wizard-fixture.ts already uses detect: ['drizzle-orm'] for the drizzle option, so the plan is consistent with the wizard — the gap is only the drizzle-kit-only edge." | ||
| ``` | ||
| Advisory only, and staying in-scope (this is about the very libs the brief names, not adding new ORMs — excluding typeorm/mongoose/kysely is correctly out of scope per P7). For the human to weigh: add `drizzle-kit` to the backend allowlist, or accept that the file-tree `migrations/` / `.sql` signal covers the schema-only case. | ||
| ## Verified — NOT gaps (checked this run, so they are not raised as findings) | ||
| - **`lib`-base coverage is retained after flipping 209-216.** Grep shows many independent `archetypes: ['lib']` assertions (`detect-archetype.test.ts:59,64,71,91,99,107,194,202,256`; `archetype.test.ts:42,43`) that do not depend on the `.sql`/migrations test — the no-signal base stays covered. | ||
| - **P3 (one axis/file) is honored.** The DB concern lands in the two axis-appropriate files it already belongs to: file-tree names in `detect-archetype.ts` (classifyEntry), package names in `archetype.ts` (BACKEND_FRAMEWORKS). Neither file gains a second change-reason. | ||
| - **P2 trust posture unchanged** — the plan's Trust audit is concrete: names membership-tested only, no file body read, closed `Archetype[]` output; the residual adds only names to existing checks. | ||
| - **P0 guarantee audit is complete** — the one guarantee ("detection is deterministic") reduces to enum/regex membership; "DB → backend" is correctly labeled an advisory design choice, not a guarantee. | ||
| - **No enum member added** — DB folds onto existing `backend`, so `ARCHITECTURE.md §5` and the four `validate` maps are untouched (no `db` member introduced). | ||
| ## Summary | ||
| The plan is honest and well-grounded — notably it discovered the requested enum alignment is already implemented and correctly narrowed the increment to the one buildable residual (DB→backend), recording the human-approved reversal of decision #2. The guarantee/trust/determinism audits hold. The **one material concern** is a scope omission (Finding 1): the reversal breaks a second, unlisted test pin (`archetype.test.ts:37-41`) that the Files section doesn't name — the builder must update it, not only the 209-216 pin. Finding 2 is a minor, in-scope completeness nicety (drizzle-kit-only projects) for the human to weigh. | ||
| ADVISORY VERDICT: 2 concerns raised (1 important, 1 minor; 0 blocking) — for the human to weigh before /pharn-dev-build. This grill-log gates nothing; the deterministic backstops remain /pharn-dev-build's floor-gates (spec-hash drift, unresolved HALT questions) and npm test / validate. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| # PLAN — archetype-enum-align | ||
| - spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 (ARCHITECTURE.md) | ||
| - increment: Map DB signals (`.sql` files, `migrations/` dirs, and `prisma` / `@prisma/client` / `drizzle-orm` deps) to the existing `backend` archetype — the one residual after discovery found the enum alignment already complete. **Approved by the human at the halt (this run); knowingly reverses the tested decision #2 of increment #21.** (The enum↔pharn-oss alignment itself was already implemented across #17/#20/#21 — confirmed, no change needed.) | ||
| - layer(s): pharn-cli `src/lib` capability-resolver (ARCHITECTURE.md §5, "Archetype + map-consistency") — no `pharn-contracts` / `pharn-core` module files touched | ||
| - constitution_refs: [P6, P7, P5, P2, P3] | ||
| ## Discovery finding — READ FIRST (the increment's premise is stale) | ||
| Everything the request describes as a *change* is **already in the tree** (verified by reads this run, not memory — P6). The task's premise — _"pharn-cli's archetype.ts currently uses {frontend, db, …} — mismatched"_ — is **false against live state**: | ||
| | Requested change | Live state | Evidence | | ||
| | --- | --- | --- | | ||
| | enum → `{ssr, backend, spa, lib}` | **already so** — no `frontend`/`db` member exists | `src/types.ts:195` (`Archetype`), `src/lib/archetype.ts:54` (`ARCHETYPE_ORDER`) | | ||
| | `universal` handled by resolver as wildcard | **already so** — always selected, `continue`s | `src/lib/resolve-capabilities.ts:46-52` | | ||
| | `applies` matches iff `intersect(applies, detected)` non-empty | **already so** | `src/lib/resolve-capabilities.ts:54-63` | | ||
| | `.tsx`/`.jsx`, react/vue → spa | **already so** (`spa = clientUi ∧ ¬ssr`) | `detect-archetype.ts:84`, `archetype.ts:42-49,122-130` | | ||
| | next.config / next-in-deps → ssr | **already so** | `detect-archetype.ts:76`, `archetype.ts:21-29` | | ||
| | api routes/handlers / express / fastify → backend | **already so** | `detect-archetype.ts:68-82`, `archetype.ts:32-39` | | ||
| | library (no app entry) → lib | **already so** (no signal → `lib`) | `archetype.ts:128` | | ||
| | `.sql` / migrations / drizzle / prisma → **backend** | **NOT done** — deliberately maps to **nothing** | `detect-archetype.ts:67-68`, pinned test `tests/detect-archetype.test.ts:209-216` | | ||
| The grep for `frontend` / `db` as enum values across `src` + `tests` returns **only comments and a test label** — zero live enum occurrences. `ARCHITECTURE.md §5` (line 179) already reads `archetype ∈ {ssr, backend, spa, lib}`. **The DISCOVERY ask — "confirm the 5 enum values match pharn-oss exactly" — is CONFIRMED: `{universal (wildcard sentinel), ssr, backend, spa, lib}` already match.** The alignment landed across increments #17 (resolver scaffolding), #20 (I/O boundary), #21 (file-tree scan). | ||
| **So the only buildable residual is the DB→backend signal — and it is not a "fix a mismatch", it is a _reversal_.** Increment #21 (`archetype-file-tree-scan`) recorded, at its own discovery halt, **decision #2**: _"Drop `.sql` / `migrations/` (P7 — smallest coherent increment). That signal maps to no existing archetype; … Revisit `db` as its own increment when a real need triggers it."_ A pinning test enforces it (`tests/detect-archetype.test.ts:209-216`, "pinned so a future `db` archetype is a deliberate edit"). Reversing it is a real design decision, and per **P6/P7** the human — not the agent — must trigger it. Hence the HALT below. | ||
| ## Files | ||
| > Contingent on Open-question Q1 = "implement the DB→backend residual". If Q1 resolves to "no / already aligned", this increment is a **no-op** (P7 — no speculative addition) and **no file changes**. | ||
| - `src/lib/detect-archetype.ts` — in `classifyEntry`, add: a file whose name ends `.sql` → `backend` signal; a dir named `migrations` → `backend` signal; update the "contribute NOTHING" comment (lines 66-68) to record the reversal — layer: I/O boundary | ||
| - `src/lib/archetype.ts` — add ORM package names (`prisma`, `@prisma/client`, `drizzle-orm`) to `BACKEND_FRAMEWORKS`; update the set's comment — layer: pure classification rules | ||
| - `tests/detect-archetype.test.ts` — **flip** the pinned decision-#2 test (209-216): `.sql` + `migrations/` now → `['backend']`, not `['lib']`; rewrite its comment to cite this plan's reversal — layer: test/spec (P1) | ||
| - `tests/archetype.test.ts` — add package-name cases: `drizzle-orm` dep → `backend`; `prisma` / `@prisma/client` dep → `backend`; a DB-only project → `['backend']` — layer: test/spec (P1) | ||
| **Not touched (already correct):** `src/lib/resolve-capabilities.ts`, `src/types.ts` (enum + `universal` wildcard), `tests/resolve-capabilities.test.ts`. `ARCHITECTURE.md §5` needs **no** edit — DB→backend adds no enum member, so it stays `{ssr, backend, spa, lib}` (and §5 is hook-protected / human-only regardless). | ||
| ## Contracts satisfied | ||
| - ARCHITECTURE.md §5 "Archetype + map-consistency" — detection stays a **deterministic membership test** over merged package.json + file-tree **name** signals; the residual only adds two names/patterns to the existing `backend` signal, mapping DB onto an existing enum member (no `db` member introduced, so the four `validate` maps are unaffected). Cite, not restate (P4). | ||
| - ARCHITECTURE.md §8 finding/trust split — unchanged: detection emits the closed `Archetype[]` enum + a boolean; no untrusted free text crosses the boundary. | ||
| ## Evals to write (P1) | ||
| - `.sql` file present (e.g. `db/schema.sql`) → detection includes `backend` | ||
| - `migrations/` dir present → detection includes `backend` | ||
| - `drizzle-orm` in deps → `backend` | ||
| - `prisma` (or `@prisma/client`) in deps → `backend` | ||
| - DB signal **+** a `.tsx` file → `['backend', 'spa']` (merge-then-rule, order preserved) | ||
| - DB signal **+** `next` → `['ssr', 'backend']` (SSR not suppressed by DB) | ||
| - DB-only project (no UI/SSR/API) → `['backend']`, not `['lib']` (the reversal, replacing the old `→ lib` pin) | ||
| ## Guarantee audit (P0) | ||
| - "archetype detection is deterministic" → **floor: enum/regex** — every added branch is a name-membership test (`lower.endsWith('.sql')`, `lower === 'migrations'`, `names.has('prisma')`); no classification/guess (P5). | ||
| - "DB concern belongs to the `backend` archetype" → **advisory design choice** (a mapping decision, not a guarantee). It introduces no new guaranteed invariant; the *mechanism* underneath it is the floor membership test above. | ||
| - No `pharn update` / legacy-pin surface is touched (P7) — this is pure CLI-side detection with no config-schema or manifest impact. | ||
| ## Trust audit (P2) | ||
| - Inputs: file/dir **names** in the project tree and dependency **key names** in `package.json` — both untrusted project input. The residual adds only `.sql` / `migrations` / `prisma` / `drizzle-orm` to the existing name-membership checks. Names are tested for membership **only** — never executed, interpolated, forwarded, or logged; no file **body** is read (package.json remains the sole file read). Output stays the closed `Archetype[]` enum + boolean, so **no untrusted free text escapes** the boundary. Taint posture is unchanged. | ||
| ## Decisions resolved at the approval halt (this run) — P6 | ||
| Both open questions were put to the human as an interactive form and resolved; no open questions remain. | ||
| 1. **Scope → "Map DB → backend."** Build the residual as specced. This knowingly **reverses decision #2** of increment #21 and flips its pinned test (`tests/detect-archetype.test.ts:209-216`, currently `→ lib`, becomes `→ backend`). | ||
| 2. **DB home → `backend`, no new archetype.** DB folds onto the existing enum member per the brief's "DB concern lives in backend archetype"; a dedicated `db` archetype was explicitly declined as out-of-scope / cross-cutting (would touch §5 + the four `validate` maps + `pharn-contracts`). | ||
| Plan **approved as written** — GATE 1 passed. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # REGRESSION — archetype-enum-align | ||
| **Verdict (floor — `.dev/floor/check-regress.mjs verdict`, exit 0):** | ||
| `REGRESSIONS: none — no deterministically-detectable breakage outside the feature.` | ||
| - **Base:** `80d7a33` (working-tree dogfood build → `base = HEAD`; `git status --porcelain` was non-empty). | ||
| - **Inside (the feature's product changes, ⊆ the plan's `## Files`):** | ||
| `src/lib/archetype.ts`, `src/lib/detect-archetype.ts`, `tests/archetype.test.ts`, | ||
| `tests/detect-archetype.test.ts`. | ||
| The changed-set is the build's **product** changes (git diff vs base + untracked), with the pipeline's | ||
| own bookkeeping dirs (`.dev/**`, `.pharn/**`) excluded — each stage writes those under **its own** | ||
| per-stage writes-scope; they are not part of THIS increment's build `## Files`. `scope` confirmed | ||
| **escaped = []** (no fix #7 breach). | ||
| ## Outside-scope gates (identical set at base and head) | ||
| | gate | base | head | result | | ||
| | ---------- | ---- | ---- | -------------------------------------------------------------------------- | | ||
| | `tests` | 0 | 0 | OK — 663/663 floor tests (`node --test` over 44 `*.test.mjs`/`*.test.cjs`) pass at both | | ||
| | `validate` | 0 | 0 | OK — `.dev/floor/validate.mjs .` GREEN at both (0 markdown capabilities) | | ||
| - **`regressions[]`:** none. | ||
| - **`pre_existing[]`:** none (the baseline was fully GREEN). | ||
| - **Style gates** (`lint` / `format:check` / `lint:md`): **skipped** deterministically — `inside` touches | ||
| no shared style config (`eslint.config.mjs`, `.prettierrc`, `.prettierignore`, `.markdownlint-cli2.jsonc`), | ||
| so a style flip over the byte-identical outside files is provably impossible (`npm ci` cost avoided). | ||
| - **Committed eval pairs** (`structural:*`): none tracked (`evals/expected/*.json` empty), so no | ||
| structural gate ran. | ||
| ## Notes (honest, per P0/P7) | ||
| - The first capture pass mis-fired identically at base and head — `node --test $VAR` under **zsh** does | ||
| not word-split an unquoted parameter, so `node --test` received one bogus filename (a spurious exit 1 | ||
| on **both** sides, so no false regression could arise). Re-run through `bash -c` for normal | ||
| word-splitting: **663/663 pass, exit 0 at both** base and head. This matches the documented gotcha in | ||
| the prior increment's REGRESSION.md. | ||
| - The `tests` gate is a **single aggregate** exit code over 44 files; its guarantee is "no pass→fail flip | ||
| in the floor suite," at suite granularity. Per-file precision for THIS feature lives in the CLI's own | ||
| vitest suite (`npm run check`, 409/409 green), which is **inside** scope (the feature changed its test | ||
| files) and is therefore verified at `/pharn-dev-build` and `/pharn-dev-verify`, not re-compared here. | ||
| **Residual (named, not hidden):** `/pharn-dev-regress` catches exactly what its deterministic suite catches — | ||
| nothing more. "No regressions" means **no deterministically-detectable breakage outside the feature**, NOT | ||
| "nothing broke." This certifies the comparison, not the increment as a whole (P0). |
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the existing
next + prismapin to the plan.The Files section misses
tests/archetype.test.ts:37-41, which flips to['ssr', 'backend']under the approved DB→backend change. Update the plan scope or it will understate the required test edit.🤖 Prompt for AI Agents