dtourolle
96abc3afef
The sixteen specs folded in last commit were folded because someone noticed they had gone stale, not because anything said they should be. Without the rule written down the directory drifts straight back to a mix of promises and descriptions, and neither can be trusted: you cannot tell from a file whether it describes the build or proposes a change to it. So: docs/specs/ holds only unshipped work, there is no "Implemented" resting state, and the fold-in and the deletion happen in the same commit. The template now asks for the destination architecture doc **up front**, which is a design check rather than bookkeeping — a feature that fits no existing doc usually has an unclear layer assignment, and it is cheaper to find that out at spec time. It also tells the author which half of what they are writing is durable (invariants, rejected alternatives, the defect a decision prevents) and which half dies with the file (phases, migration steps, acceptance criteria). The review checklist gains a Lifecycle section, including the case that gets lost otherwise: out-of-scope work worth doing has to be written where it will still be found after the spec is gone.
82 lines
4.3 KiB
Markdown
82 lines
4.3 KiB
Markdown
# Spec review checklist
|
|
|
|
Run a spec past this before accepting it. It exists because JellyTau's
|
|
backend/frontend boundary is a **stated rule with, historically, no gate** — the
|
|
rule lived in the architecture docs, but nothing forced a spec author to check a
|
|
new design against it, and a "minimal-change" spec quietly leaked domain
|
|
taxonomy into the frontend (see [scoped-search-boundary.md](scoped-search-boundary.md)).
|
|
This checklist is the human gate. The CI check
|
|
(`scripts/check-frontend-boundary.sh`) is only a crude tripwire for one leak
|
|
signature — it does **not** replace this.
|
|
|
|
Copy the boxes into the review comment (or the PR) and tick them.
|
|
|
|
## Boundary (the one that bites)
|
|
|
|
- [ ] **The spec has a filled-in "Layer assignment" table**, and it assigns
|
|
*logic*, not files. A spec without this section is not ready to review.
|
|
- [ ] **No domain vocabulary is placed in the frontend.** In particular: Jellyfin
|
|
item-type sets that define a *category* (what "Music"/"TV"/"Movies" means),
|
|
query-shaping rules, business rules, reachability/sync policy. If the
|
|
frontend names a *set* of item types to define a category, that is a leak —
|
|
it belongs behind an opaque enum the backend expands.
|
|
- [ ] **"The backend already accepts this parameter" was not used as the reason**
|
|
to place the deciding logic in the frontend. Accepting a parameter ≠ owning
|
|
the decision of its value.
|
|
- [ ] **The `Scope:` / effort framing is not optimizing for "least backend
|
|
change."** "Frontend only, no Rust changes" is a description, never a goal.
|
|
The goal is *correct layer placement*; sometimes that is more Rust work.
|
|
- [ ] Ran the litmus test on each borderline responsibility: *would it change if
|
|
Jellyfin's API changed?* → Rust. *Only if the UI were redesigned?* →
|
|
frontend. Borderline defaults to Rust.
|
|
- [ ] Single-type presentation (`itemType: "Movie"`, "this page shows albums")
|
|
is **not** over-corrected into the backend. The rule targets category
|
|
*taxonomy*, not every mention of a type. Don't invent a backend enum per
|
|
list page.
|
|
|
|
## IPC contract
|
|
|
|
- [ ] Anything crossing the boundary has its wire shape specified.
|
|
- [ ] camelCase rule accounted for: top-level params auto-convert; nested structs
|
|
get `#[serde(rename_all = "camelCase")]`; tagged unions match tags on both
|
|
sides; events are kebab-case. (CLAUDE.md §IPC,
|
|
[04-type-sync-and-threading.md](../architecture/04-type-sync-and-threading.md).)
|
|
- [ ] Any result that arrives *twice* (command return **and** a later event —
|
|
e.g. the search cache/server merge) has **both** payloads in the new shape.
|
|
- [ ] `bindings.ts` is regenerated from Rust, not hand-edited.
|
|
|
|
## Requirements & traceability
|
|
|
|
- [ ] Linked to existing URs, or new URs/DRs are allocated in
|
|
[requirements.md](../requirements.md).
|
|
- [ ] Requirement-implementing code will carry `// TRACES:` comments (CLAUDE.md).
|
|
- [ ] Traceability coverage stays ≥ 88% (the CI gate — a ratchet, so check
|
|
`bun run traces:coverage` rather than trusting this number).
|
|
|
|
## Lifecycle
|
|
|
|
- [ ] **"Destination on completion" names a real architecture doc and section.**
|
|
This spec file is deleted when it ships; something has to absorb the
|
|
design. If nothing fits, the layer assignment is probably unclear — go back
|
|
to that table.
|
|
- [ ] The spec separates the **durable half** (invariants, rejected alternatives,
|
|
the defect a decision exists to prevent) from the **disposable half**
|
|
(phases, migration steps, acceptance criteria). Only the first is folded in.
|
|
- [ ] Anything listed as out of scope but still worth doing is written where it
|
|
will be found after this file is gone — beside the code it concerns.
|
|
|
|
## Conflicts & hygiene
|
|
|
|
- [ ] If this spec revises/supersedes another, the older spec gets a banner
|
|
pointing here — no two specs silently contradicting.
|
|
- [ ] Acceptance criteria include the standard gates: `bun run check`,
|
|
`bun run test`, `bun run check:boundary`, and (if Rust changed)
|
|
`cargo fmt`/`cargo clippy`/`bun run test:rust`.
|
|
- [ ] Notes flag that a parallel Claude session may be active in the repo.
|
|
|
|
---
|
|
|
|
**If any Boundary box can't be ticked, the spec is not ready** — fix the layer
|
|
assignment first. Every other section can be negotiated; that one is the whole
|
|
reason this file exists.
|