dtourolle
75bae2556c
Audit of the principles in CLAUDE.md and docs/architecture/ against the actual code. Principles with a working automated check (poison-tolerant locking, Android source sync, one-directional playback state, graceful backend init, reachability-from-traffic) all held up. The two that drifted are exactly the two whose checks were broken or too narrow: - traceability-gate-repair: CI divided by hardcoded denominators (UR/39, IR/24, DR/48, JA/3, total 114) while requirements.md had grown to 211, reporting 158% coverage — the 50% threshold was unreachable and the job could not fail. - req-coverage-script-removal: check-req-coverage.sh reports "1 requirement" and prints "all requirements have implementations". - scoped-search-boundary-implementation: the founding boundary incident was specced but never built; the leak is still live. - boundary-tripwire-hardening: check:boundary passes on that same leak — the pattern is anchored to the query site, so a named const evades it. - player-facade-enforcement: 52 direct commands.player* call sites outside the facade, and no automated check at all. Each spec follows SPEC-TEMPLATE.md with a filled-in Layer assignment table and is checked against SPEC-REVIEW-CHECKLIST.md.
162 lines
6.9 KiB
Markdown
162 lines
6.9 KiB
Markdown
# Spec: Remove the broken `check-req-coverage.sh`
|
|
|
|
**Status:** Implemented
|
|
**Requirements:** supports DR-093 (see [traceability-gate-repair.md](traceability-gate-repair.md))
|
|
**UX spec:** n/a — developer tooling.
|
|
**Supersedes / revises:** n/a
|
|
|
|
## Summary
|
|
|
|
`scripts/check-req-coverage.sh` is broken, orphaned, and actively misleading: it
|
|
reports `Total Requirements: 1`, zeros in every category, and then prints
|
|
**"✨ All requirements have implementations!"**. Nothing references it — not CI,
|
|
not `package.json`, not the docs. This spec deletes it, with a narrowly-scoped
|
|
alternative (repair it) documented and rejected below.
|
|
|
|
## Motivation
|
|
|
|
Running it today produces:
|
|
|
|
```
|
|
Category Breakdown:
|
|
UR: 0 requirements
|
|
IR: 0 requirements
|
|
DR: 0 requirements
|
|
JA: 0 requirements
|
|
|
|
Summary:
|
|
Total Requirements: 1
|
|
✅ Fully Implemented: 0 (0%)
|
|
|
|
✨ All requirements have implementations!
|
|
```
|
|
|
|
Every number is wrong (the real totals are UR 61, IR 29, DR 89, JA 32), and the
|
|
concluding message is the *opposite* of a warning — a developer running this to
|
|
sanity-check coverage is told everything is fine.
|
|
|
|
This is worse than having no script. It is a trap, and it sits in `scripts/`
|
|
next to tools that do work, with nothing marking it as dead.
|
|
|
|
Verification that it is genuinely orphaned:
|
|
|
|
```console
|
|
$ grep -rn "check-req-coverage" . --include='*.yml' --include='*.json' \
|
|
--include='*.sh' --include='*.md' | grep -v node_modules
|
|
(no output)
|
|
```
|
|
|
|
## Layer assignment
|
|
|
|
Developer tooling only; no application logic and nothing crosses the IPC
|
|
boundary.
|
|
|
|
| Logic / responsibility | Layer | Why it belongs there |
|
|
|------------------------|-------|----------------------|
|
|
| Requirement-coverage reporting | Build tooling — `extract-traces.ts` | One tool should own coverage analysis. A second, divergent implementation is how the two answers ("1 requirement" vs "211") came to disagree unnoticed. |
|
|
|
|
## Design
|
|
|
|
**Delete `scripts/check-req-coverage.sh`.**
|
|
|
|
Coverage reporting is owned by [scripts/extract-traces.ts](../../scripts/extract-traces.ts),
|
|
which is correct, is what CI runs, and gains a first-class local coverage mode
|
|
in [traceability-gate-repair.md](traceability-gate-repair.md):
|
|
|
|
```bash
|
|
bun run traces:coverage # the supported way to check coverage locally
|
|
```
|
|
|
|
Then check the sibling scripts for the same rot. `scripts/` also contains
|
|
`check-test-coverage.sh` and `find-req-implementations.sh`, neither of which is
|
|
referenced from `package.json`. An unreferenced script is never run and so rots
|
|
silently — that is the actual failure mode being fixed here, and fixing only the
|
|
one instance found by audit leaves the others to be rediscovered later.
|
|
|
|
### Findings (investigation, 2026-07)
|
|
|
|
All three scripts turned out to share a **single root cause**, and all three are
|
|
deleted:
|
|
|
|
| Script | Defect |
|
|
|---|---|
|
|
| `check-req-coverage.sh` | Reads `README.md`, which has held **zero** requirement rows since they moved to `docs/requirements.md` → `total_reqs=1`, every category 0, "✨ All requirements have implementations!" Also greps `src-tauri/` unscoped. |
|
|
| `check-test-coverage.sh` | Greps `src-tauri/` unscoped — including **40 GB** of `target/` build artifacts. Hangs indefinitely; produces no output at all. |
|
|
| `find-req-implementations.sh` | Same unscoped `src-tauri/` grep. Same hang. |
|
|
|
|
So none of them were subtly wrong — two could never terminate, and the third
|
|
inverted its own conclusion.
|
|
|
|
They were nonetheless *salvageable*: scoping the greps to `src-tauri/src` and
|
|
repointing at `docs/requirements.md` would be a few lines, and the `@req:` /
|
|
`@req-test:` tags they read are still present in the tree (**146** and **76**
|
|
occurrences).
|
|
|
|
**Decision: delete all three anyway.** The tags are an undocumented parallel
|
|
convention — `@req:` appears in no doc, and CLAUDE.md describes only `TRACES:`.
|
|
Repairing the scripts would re-establish a second traceability system to keep in
|
|
sync with the first, which is the same two-sources-of-truth condition that let
|
|
"1 requirement" and "211 requirements" coexist unnoticed. `TRACES:` plus the
|
|
repaired coverage engine ([traceability-gate-repair.md](traceability-gate-repair.md))
|
|
already cover this ground.
|
|
|
|
The existing `@req:` / `@req-test:` comments are left in place: they are
|
|
harmless as prose, several encode genuinely useful test intent, and stripping
|
|
222 comments across the tree is a large diff with no functional gain. They are
|
|
simply no longer read by any tool.
|
|
|
|
### Alternative considered: repair rather than delete
|
|
|
|
Rejected. The script's output format duplicates what `traces:markdown` already
|
|
generates, it has no tests, no caller, and no documented purpose distinct from
|
|
`extract-traces.ts`. Repairing it recreates the two-sources-of-truth condition
|
|
that produced the contradiction. If a shell-based coverage check is ever wanted,
|
|
it should shell out to `traces:json` and `jq` rather than re-parse
|
|
`requirements.md` independently.
|
|
|
|
## Out of scope
|
|
|
|
- The CI workflow denominators — [traceability-gate-repair.md](traceability-gate-repair.md).
|
|
- Any change to `extract-traces.ts`'s output (that spec owns it).
|
|
- Auditing scripts that *are* referenced from `package.json` — they run
|
|
regularly and would fail visibly.
|
|
|
|
## Acceptance criteria
|
|
|
|
- [ ] `scripts/check-req-coverage.sh` no longer exists.
|
|
- [ ] `grep -rn "check-req-coverage" .` (excluding `node_modules` and this spec)
|
|
returns nothing — no dangling reference in CI, docs, or `package.json`.
|
|
- [ ] `scripts/check-test-coverage.sh` and `find-req-implementations.sh` have each
|
|
been run and either wired into `package.json` or deleted; the decision and
|
|
reason are recorded in `scripts/README.md`. **Outcome: all three deleted —
|
|
see Findings.**
|
|
- [ ] `scripts/README.md` documents `bun run traces:coverage` as the supported
|
|
way to check requirement coverage locally.
|
|
- [ ] `bun run test:all` passes (confirms nothing invoked the deleted script).
|
|
- [ ] `bun run check` and `bun run test` pass.
|
|
- [ ] `bun run check:boundary` passes.
|
|
|
|
## Testing
|
|
|
|
No unit tests — this is a deletion. Verification is the grep in the acceptance
|
|
criteria plus a green `bun run test:all`, which exercises the script paths that
|
|
actually run.
|
|
|
|
## TRACES
|
|
|
|
No new requirement. The deletion is covered by **DR-093**
|
|
([traceability-gate-repair.md](traceability-gate-repair.md)), which establishes
|
|
`extract-traces.ts` as the single owner of coverage reporting. Note the removal
|
|
in that DR's text when both land.
|
|
|
|
## Notes for the implementer
|
|
|
|
- A parallel Claude session may be active in this repo — `git diff` before
|
|
"repairing" unexpected changes (CLAUDE.md §Gotchas).
|
|
- Land this **after** or alongside [traceability-gate-repair.md](traceability-gate-repair.md),
|
|
so `bun run traces:coverage` exists before the broken script is removed and
|
|
developers are never left without a coverage command.
|
|
- Check `docs/traceability-ci.md` and `docs/traces-quick-ref.md` for prose
|
|
references to the deleted script; the grep above covers `.md`, but read the
|
|
surrounding sentence rather than deleting the line mechanically.
|