Skip to content

Commit ba01d81

Browse files
feat: post-rethink substrate batch — aliases / unused-type-members / suppressions / V8 coverage (#75)
* feat: post-rethink substrate batch + fallow purge Ships four roadmap items extracted from the dominance-rethink + non-goals reassessment, alongside a docs-governance cleanup that purges fallow references (research adoption-tracker no longer the codemap mission; roadmap is the north star). Substrate features (each independently changeset'd): 1. **Outcome-shaped CLI aliases** (XS, patch) — five thin wrappers over `query --recipe <id>`: `codemap dead-code` / `deprecated` / `boundaries` / `hotspots` / `coverage-gaps`. Every `query` flag passes through. Capped at 5; mapping in `src/cli/aliases.ts`. Closes verb-obviousness gap; Moat-A clean (recipe IS the SQL). 2. **`unused-type-members` recipe** (S, patch) — field-level enumeration of `type_members` whose owning type has no detectable importer. Sister to `unimported-exports`; STRICTLY ADVISORY — codemap can't see indexed access (`T['field']`), `keyof T`, mapped types, destructuring, or re-export chains. Bundled `.md` documents every false-positive class. 3. **`suppressions` substrate** (S, minor — schema 9→10) — opt-in markers parser extension recognises `// codemap-ignore-{next-line,file} <recipe-id>` (also `#`, `--`, `<!--`, `/*` leaders) → new `suppressions` table. Recipes opt in via `LEFT JOIN`; `untested-and-dead` (line + file) and `unimported-exports` (file only — exports has no `line_number`) honor today. Stays consistent with the no-opinionated-rule-engine Floor: no severity, no suppression-by-default, no universal-honor; consumer- chosen substrate, not policy. 4. **V8 runtime coverage parser** (S, minor) — `codemap ingest-coverage --runtime <dir>` reads `NODE_V8_COVERAGE=...`-style directories (one or more `coverage-*.json` per process), maps byte-offset ranges to per-line hits via innermost-wins range walking, dispatches to the existing `upsertCoverageRows` core. Format auto-detection unchanged for files; `--runtime` is the explicit V8 opt-in. Local-only — SaaS aggregation explicitly out of scope (different product class). Docs cleanup (no functional impact): - Deletes `docs/research/fallow.md` and `docs/research/competitive-scan-2026-04.md` (adoption-from-fallow framing no longer the codemap mission). - Slims fallow refs from `lsp-diagnostic-push.md`, `why-codemap.md`, `non-goals-reassessment-2026-05.md`, `audit-pr-architecture` skill, `docs-governance` rule + skill, lessons.md. - Adds `No remote-repo cloning` Floor to `roadmap.md` (recovered from the deleted competitive-scan; rejected in PR #23 as demoware that doesn't fit the SQL-index thesis). - Reframes `No opinionated rule engine` Floor to clarify suppressions ship as opt-in substrate, not as policy primitives. - Reframes `No runtime tracing` Floor to scope V8 protocol post-mortem ingestion as the static-side adjacent capability (live tracing / production beacons still out of scope). Schema bump: `SCHEMA_VERSION = 10` (added `suppressions` table). Auto- detected; consumers get a one-time full reindex on next codemap run. Tests: 858 pass (was 847 at branch base — +11 across `aliases.test.ts`, `markers.test.ts` suppression coverage, `coverage-engine.test.ts` V8 paths, `cmd-ingest-coverage.test.ts` `--runtime` flag). Golden-query suite gains `unused-type-members` scenario. Per [`docs/README.md` Rule 10](docs/README.md), bundled agent rule + skill in `templates/agents/` and the dev-side mirror in `.agents/` updated in lockstep — drift between the two pairs is CLI-prefix-only. * fix(db): remove `;` from suppressions DDL line comments Node's better-sqlite3 path in `runSql` splits the multi-statement DDL on `;` before calling `prepare()` — semicolons inside `--` line comments become statement boundaries, producing comment-only "statements" that better-sqlite3 rejects with `RangeError: The supplied SQL string contains no statements`. Bun's `bun:sqlite` accepts multi-statement strings directly so the bug only surfaces in the Node CI build job. Caveat already documented at `src/sqlite-db.ts` (`runSql`'s JSDoc): "do not put `;` inside `--` line comments in `db.ts` SQL strings". The slimmed suppressions DDL comment in 4541547 reintroduced three of them. Reworded to drop every semicolon while keeping the same intent (file vs next-line scope encoding, opt-in JOIN posture, source pointer to markers.ts). Caught by CI on PR #75 Build job; reproducible locally with `bun run build && node dist/index.mjs --full`. * fix(coverage,docs): apply CodeRabbit findings on PR #75 Three real bugs from the CodeRabbit pass; pushed back on three hallucinations (replies on the threads). * **coverage-engine.ts (T3)** — duplicate-URL scripts overwrote each other via shared `lineHits`. A multi-process test run where one process didn't hit a line could zero out another process's non-zero count. Fix: walk innermost-wins per script into a per-script accumulator, then merge across scripts with `Math.max`. New regression test exercises a "hot" + "cold" duplicate pair where last-writer-wins would have produced zero hits but Math.max-merge correctly preserves the hot count. * **cmd-ingest-coverage.ts (T5)** — `--runtime <dir>` accepted any top-level `*.json` and silently skipped non-V8 payloads, so pointing the flag at the wrong directory reported a successful ingest with zero rows. Two safeguards: (1) `coverage-*.json` filename filter so obvious mis-targets fail at directory resolution; (2) post-parse guard that throws when zero of the matched files contained a V8 `result` array — surfaces the "wrong directory" case loudly instead of returning success with no data. * **docs-governance/SKILL.md § Closing research (T1)** — § Closing research said "delete the research file unless its comparison framework is reusable", contradicting the canonical [docs/README.md Rule 8](docs/README.md) lifecycle which says "slim to a 'What shipped' appendix". Aligned the SKILL flow with Rule 8 (slim by default) and called out the § 6 anti-pattern exception explicitly: per-tool / peer-tool trackers get deleted because the framing was off-mission, not just stale (matches what this PR did to fallow.md and competitive-scan-2026-04.md). * **agents rules (T7)** — Coverage-ingest dispatch row read as if V8 was auto-detected alongside Istanbul/LCOV. Re-worded to make the Istanbul/LCOV auto-detect path vs the V8 `--runtime` opt-in path explicit. Lockstep update across `templates/agents/rules/codemap.md` and `.agents/rules/codemap.md` per Rule 10. * **coverage-engine.ts buildLineOffsets JSDoc (T4 corrected)** — my original comment said the line-offset table was a "byte-offset approximation" walking UTF-16 code units. That's wrong both about V8's units and about what the code does: V8's `Profiler.CoverageRange` source offsets are UTF-16 code units (per Chrome DevTools Protocol spec) — the same units `String.charCodeAt` walks. No encoding conversion needed. Updated the JSDoc to cite the spec so the next reviewer doesn't repeat the question. Replies posted on T2, T4, T6, and the "Docstring Coverage 32%" pre-merge warning (codebase convention is concise-comments per .agents/rules/concise-comments.md, not 80% blanket coverage).
1 parent 7889fed commit ba01d81

44 files changed

Lines changed: 1164 additions & 489 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/lessons.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ Each entry is a single bullet: `- **<topic>** — <lesson>`. Newest entries at t
1313
- **backticks inside SQL or help-text template literals** — never put a literal backtick inside a `` `...` `` template-literal string. `db.ts` SQL DDL strings (multi-line CREATE TABLE templates) and `printQueryCmdHelp()` (multi-line help text) are both `` `...` `` template literals; an inner backtick — typically a Markdown-style code-fence around a flag like `` `--full` `` — terminates the literal early and the parser blows up several lines later with cryptic "expected `,` or `)`" errors. **Use plain prose in those strings** (`--full` not `` `--full` ``), or escape (`` \` ``) if you really need the character. Hit twice (B.7 + B.6 PR #30); the lesson is general — applies to any TS template literal that gets pasted prose later, not just SQL / help text.
1414
- **STOP-before-Grep applies to symbol lookups too**`Grep` for symbol names like `printQueryResult`, `getCurrentCommit`, `dropAll` violates the [`codemap` rule](rules/codemap.md). The codemap query `SELECT file_path, line_start FROM symbols WHERE name = '<X>'` answers it faster and without scanning. Reach for `Grep` only when the question is content-shaped (regex over file bodies, finding pattern usages inside function bodies, etc.) — not when it's "where is X defined / who calls X / what does file Y export." This was a PR #30 self-correction.
1515
- **PR / issue / comment bodies always go through a temp file** — never pass markdown bodies via shell heredoc to `gh pr create --body "$(cat <<'EOF'…)"` / `gh pr edit --body …` / `gh pr comment --body …` / `gh issue create --body …` / `gh api` `--field body=…`. Backticks inside the heredoc (every code span and code fence) get shell-escaped to `\`` and render literally on GitHub — every recipe id, file path, flag, SQL fragment, and code fence in the rendered body comes out as `\`coverage\``instead of`coverage`. Pattern: write the body to a temp file (`Write`to`/tmp/pr-<n>-body.md`), pass `--body-file /tmp/pr-<n>-body.md`, then delete the temp file. Cost is one extra tool call; saves redoing every PR body that has more than a few backticks. Hit on PR #57 — final body was a wall of `\`` artifacts until rewritten via temp file.
16-
- **Never commit absolute local user paths** — no `/Users/<name>/…`, `/home/<name>/…`, `~/…`, or `file:///` URIs in any tracked doc, code, comment, or PR body. Reasons: (1) leaks the maintainer's directory structure / username to public mirrors; (2) every other contributor's paths differ — the reference is dead on their machine; (3) a `git clone` of someone else's machine isn't a fact we can cite as a "source for deep-dives" — public upstream URLs are. Pattern: cite `https://github.com/<org>/<repo>` (with optional `/tree/<sha>/<path>`) for upstream sources; use repo-relative paths (`docs/foo.md`, `src/bar.ts`) for in-tree references. Hit on PR #58 first draft — referenced the local fallow clone path in the research note before the user caught it.
16+
- **Never commit absolute local user paths** — no `/Users/<name>/…`, `/home/<name>/…`, `~/…`, or `file:///` URIs in any tracked doc, code, comment, or PR body. Reasons: (1) leaks the maintainer's directory structure / username to public mirrors; (2) every other contributor's paths differ — the reference is dead on their machine; (3) a `git clone` of someone else's machine isn't a fact we can cite as a "source for deep-dives" — public upstream URLs are. Pattern: cite `https://github.com/<org>/<repo>` (with optional `/tree/<sha>/<path>`) for upstream sources; use repo-relative paths (`docs/foo.md`, `src/bar.ts`) for in-tree references. Hit on PR #58 first draft — referenced a local peer-repo clone path in a research note before the user caught it.
1717
- **Prescriptive research notes pin every concrete claim before recommending a ship sequence** — when a research/plan-shape doc proposes work (effort estimates, capability inventories, "we already do X" framing), every concrete claim needs a `file:line` / `codemap query` / `rg` / `--recipes-json` reference a reviewer can re-run. Reasoning-from-substrate intuition without pinning ships errors: "the AST walker already counts nodes" / "fan-in detects orphans" / "the `re_export_source` column doesn't exist" — all real errors caught on PR #58 by triangulating against the codebase. Don't ship a peer / parallel "descriptive baseline" doc to triangulate against (Rule 1 violation — it duplicates `architecture.md` / `db.ts` / `--recipes-json`); instead, either (a) pin claims in the prescriptive doc itself, or (b) self-audit by re-running every claim against the canonical home before committing. Either path beats the "dual descriptive + prescriptive doc" pattern on docs-governance grounds.

0 commit comments

Comments
 (0)