From 0c5bcecf4b4aa5bed7e17b8f02fbdcad916e5f1d Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Tue, 18 Aug 2026 19:42:24 +0200 Subject: [PATCH] bench: commit curated-1 discrimination report pages scored against golden-curated-3 The four-model full passes (opus xhigh/full reference, deepseek, m3, m2.7 all high/full on curated-1) were scored against the current committed golden (golden-curated-3, 158 entries) and their report pages written to bench/reports/. These pages are the reproducible artifact for the PR Review Bench discrimination result: recall 0.826 (opus run 1) vs 0.109 (deepseek), 0.072 (m3), 0.014 (m2.7) - a monotone ladder far beyond the measured 3sigma noise floor. Report pages were previously claimed committed but none existed for the curated-1 discrimination runs. --- ...22286b7a8881ea5bd1967fad49275b83a8f1ba8.md | 257 ++++ ...345bb4007db9cb94a69c64e2e859c7d6e8b2dab.md | 1074 +++++++++++++++++ ...c7a1787058cc5cd26a2a2d6c35a545465de79ca.md | 333 +++++ ...50482923a65761bd4d66501c153a402f5f8f8eb.md | 413 +++++++ 4 files changed, 2077 insertions(+) create mode 100644 bench/reports/7b305997dc2b9b2f734f1499922286b7a8881ea5bd1967fad49275b83a8f1ba8.md create mode 100644 bench/reports/d4558caff3dfefdff574d9a14345bb4007db9cb94a69c64e2e859c7d6e8b2dab.md create mode 100644 bench/reports/d5a15f168212fe61bdf112b08c7a1787058cc5cd26a2a2d6c35a545465de79ca.md create mode 100644 bench/reports/edb935210ee850e34cb521e7c50482923a65761bd4d66501c153a402f5f8f8eb.md diff --git a/bench/reports/7b305997dc2b9b2f734f1499922286b7a8881ea5bd1967fad49275b83a8f1ba8.md b/bench/reports/7b305997dc2b9b2f734f1499922286b7a8881ea5bd1967fad49275b83a8f1ba8.md new file mode 100644 index 0000000..eed5d70 --- /dev/null +++ b/bench/reports/7b305997dc2b9b2f734f1499922286b7a8881ea5bd1967fad49275b83a8f1ba8.md @@ -0,0 +1,257 @@ +7b305997dc2b9b2f734f1499922286b7a8881ea5bd1967fad49275b83a8f1ba8 + +## Configuration +- model: m2 +- effort: high +- mode: full +- config_hash: 7b305997dc2b9b2f734f1499922286b7a8881ea5bd1967fad49275b83a8f1ba8 +- rules_commands_hash: ecc803331f860b845a6a1b8a103e889bce02520e8cc04ea88102de74c8d5600d +- prs_version: curated-1 +- coding version: 0.45.0 +- golden baseline coding version: v0.35.6 +- golden version: golden-curated-3 +- runner_version: 1 +- rows skipped: 0 +- cost: not recorded — the ledger carries no cost field. + +*golden-curated-3* carries 20 `rejected` entries, so precision is measurable: reporting one costs precision. With so few adjudicated, a single hit still moves the number a long way — read it as directional, not calibrated. + +No signature embeds a line reference, so a re-report of the same issue at a different line still matches. `recall` measures issue detection. + +## Runs +| run | span | PRs | complete | golden in scope | findings | hits | misses | matched rejected | gap candidates | recall | precision | wall time (s) | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | +| 1 | 2026-08-17T21:48:56.232580+00:00 … 2026-08-17T23:27:32.198724+00:00 | 20/20 | complete | 158 | 53 | 2 | 136 | 0 | 51 | 0.014 | 1.000 | 6507 | + +## Per-PR +| run | pr_id | golden in scope | hits | misses | findings | gap candidates | dropped items | missing sections | duration (s) | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | +| 1 | backup#15 | 4 | 0 | 2 | 3 | 3 | 1 | — | 207 | +| 1 | bw#41 | 3 | 0 | 2 | 0 | 0 | 0 | — | 185 | +| 1 | dark-factory#71 | 2 | 0 | 2 | 1 | 1 | 0 | — | 411 | +| 1 | discord-assistant#1 | 9 | 0 | 9 | 3 | 3 | 0 | — | 196 | +| 1 | discord-assistant#5 | 13 | 0 | 13 | 4 | 4 | 0 | — | 116 | +| 1 | distill#6 | 10 | 0 | 7 | 1 | 1 | 0 | — | 111 | +| 1 | github-pr-review-agent#11 | 16 | 0 | 14 | 0 | 0 | 0 | — | 210 | +| 1 | github-releaser-agent#8 | 13 | 0 | 9 | 10 | 10 | 0 | — | 956 | +| 1 | helm#3 | 4 | 0 | 3 | 0 | 0 | 0 | — | 68 | +| 1 | node-skeleton#2 | 12 | 0 | 12 | 1 | 1 | 0 | — | 493 | +| 1 | python-skeleton#3 | 17 | 0 | 16 | 3 | 3 | 0 | — | 554 | +| 1 | quant#109 | 10 | 0 | 10 | 1 | 1 | 5 | — | 258 | +| 1 | recurring-task-creator#30 | 6 | 0 | 4 | 3 | 3 | 0 | — | 377 | +| 1 | tts-mcp#10 | 9 | 0 | 7 | 6 | 6 | 3 | — | 398 | +| 1 | tts-mcp#13 | 10 | 2 | 8 | 5 | 3 | 0 | — | 577 | +| 1 | tts-mcp#16 | 6 | 0 | 5 | 4 | 4 | 0 | — | 504 | +| 1 | tts-mcp#20 | 1 | 0 | 1 | 1 | 1 | 0 | — | 248 | +| 1 | tts-mcp#3 | 6 | 0 | 5 | 3 | 3 | 0 | — | 146 | +| 1 | vault-cli#68 | 1 | 0 | 1 | 0 | 0 | 0 | — | 127 | +| 1 | vault-ui#39 | 6 | 0 | 6 | 4 | 4 | 0 | — | 364 | + +*Effective fixture: **20 of 20** PRs produced a scored row.* **9** findings carried no matching key and never reached scoring. + +## Gap-triage candidates +These findings matched no golden entry. They are **not a precision failure** — the golden set is a bootstrap from one strong-model run, and a finding it does not describe is evidence the set is incomplete. + +### run 1 — backup#15 — Dockerfile:10 + +**`Dockerfile:10` / `Makefile:127`** — `precommit` fails with 2 unignored vulnerabilities in `golang.org/x/mod`: + +### run 1 — backup#15 — Makefile:61 + +`GO-2026-6180` — `golang.org/x/mod@v0.37.0 → v0.40.0` — ignore unrelated, unauthenticated hashes in Lookup Both are fixed in `v0.40.0`. Add to `VULNCHECK_IGNORE` in `Makefile:61` while the upgrade is in flight, or bump `golang.org/x/mod` to `v0.40.0` in `go.mod`. *(rule: precommit gate)* + +### run 1 — backup#15 — Dockerfile:8 + +**`Dockerfile:8`** — Informational: the service's `Dockerfile`/`Makefile` build target does not declare any of the three canonical provenance build args (`BUILD_GIT_VERSION`, `BUILD_GIT_COMMIT`, `BUILD_DATE`) wired via `-ldflags "-X"`. This predates this PR and is out of scope for it — flagging here so it is not lost. No action needed to unblock this change. *(rule: `go-build-args/three-args-required`, informational only)* + +### run 1 — dark-factory#71 — CHANGELOG.md:15 + +**`CHANGELOG.md:15`** — CHANGELOG bullet says "each **Blocker/Critical** issue" but `agents/prompt-auditor.md` defines only one top severity: `## Critical Issues`. "Blocker" does not appear anywhere in the agent. Change to "Critical issue" so the changelog accurately reflects the report shape. *(rule: `changelog/conventional-prefix-required` / `changelog-guide.md` "Be specific")* + +### run 1 — discord-assistant#1 — CHANGELOG.md:5 + +**`CHANGELOG.md:5`** — The new `## Unreleased` bullet (survive gateway fault) has no conventional prefix. `/coding:commit` and dark-factory parse the prefix to determine the semver bump: a `feat:` triggers minor, everything else triggers patch. Without it, the release tooling will classify a feature as a patch. Prefix it `fix:` or `chore:`. *(rule: `changelog/conventional-prefix-required`)* + +### run 1 — discord-assistant#1 — None + +**`LICENSE` (root)** — No LICENSE file exists. Add one — every repository needs an explicit license grant. *(rule: `license-assistant`)* + +### run 1 — discord-assistant#1 — src/index.js:255 + +**`src/index.js:255`** — `connectedOnce` is a module-level mutable boolean read by the `uncaughtException` handler. This is not a factory-reading-state violation, but module-level mutable flags make unit testing harder (the handler cannot be driven into its `connectedOnce = true` branch without either modifying the global or patching the event emitter). An injectable `GatewayConnectionState` interface (passed to the handler) would improve testability. The current shape is acceptable for a bot of this scale. *(rule: `node/architecture/inject-dependencies`)* + +### run 1 — discord-assistant#5 — local.env.example:15 + +**`local.env.example:15`** — `DISCORD_TOKEN_KEY=aL5p3O` is a placeholder TeamVault key id left in the committed example file. The file's own header warns "Never add a secret to this file: Make includes it", and the value here is not a real secret — but the pattern of a raw string literal in a committed example is exactly the kind of thing that gets copy-pasted into a live config. The value `aL5p3O` reads as a plausible TeamVault key id; consider using a clearly fabricated placeholder like `YOUR_TEAMVAULT_KEY_ID` or leaving the value empty with a comment explaining what goes there. + +### run 1 — discord-assistant#5 — src/config.js:188 + +**`src/config.js:188–194`** — `config.check()` does not validate the new `SHIM_WAKE_PHRASES` value. An empty string after quote-stripping (`""`) silently disables the gate, which is intentional — but an unparseable value (e.g. a single phrase with no commas and no surrounding whitespace) would be accepted without warning. The existing `check()` already validates the token and the allowlist; consider adding a note that an empty `wakePhrases` array means the gate is off. + +### run 1 — discord-assistant#5 — src/config.js:31 + +**`src/config.js:31`** — `UTTERANCE_GAP_MS` is resolved from `config.utteranceGapMs` at module-load time. If `process.env.UTTERANCE_GAP_MS` changes at runtime (not currently expected, but possible via hot reload), the constant would be stale. Low risk given the module's load-once lifecycle. + +### run 1 — discord-assistant#5 — shim/claude_openai_shim.py:799 + +**`shim/claude_openai_shim.py:799`** — `WAKE_PHRASES` is parsed with `.strip("\"'")` to handle Makefile quoting, matching the bot's `config.js`. If the environment value arrives with nested quotes (`"\"hey bot\""`), only the outer pair is stripped. The risk is low (nested quoting is rare) and the symptom is cosmetic (wrong phrase, not a crash), but the comment at line 795–798 could note this boundary. + +### run 1 — distill#6 — CHANGELOG.md:7 + +**`CHANGELOG.md:7`** — `feat(hooks):` is not a valid conventional prefix per `changelog/conventional-prefix-required`. The rule requires the first token to be exactly one of: `feat:`, `fix:`, `refactor:`, `test:`, `docs:`, `chore:`, `perf:`. `feat(hooks):` includes a scope `(hooks)` which is not in the allowed set. Fix: drop the scope to `feat:` (preferred for a genuine new feature) or use `chore:` (appropriate for a tool/hook addition, matching the pattern of `chore(dev):` used elsewhere in this file). *(rule: `changelog/conventional-prefix-required`)* + +### run 1 — github-releaser-agent#8 — pkg/githubtags/tags.go:137 + +**`pkg/githubtags/tags.go:137`** — The `collectTags` pagination loop follows the server-supplied `Link` header `rel=next` URL verbatim and re-attaches the bearer token (`Authorization: Bearer `) on every page fetch — with **no validation that the next URL's host stays on the expected api.github.com**. A compromised or MITM'd GitHub API response could redirect the bearer token to an attacker-controlled host. *(rule: `go-security/credential-forwarding-via-pagination`)* *Fix:* Before following a pagination link, parse `nextURL` and require its scheme+host to match `f.apiBase`'s scheme+host; refuse and error otherwise. + +### run 1 — github-releaser-agent#8 — pkg/plan_output.go:97 + +**`pkg/plan_output.go:97`** — `PlanOutcomeReady` and `PlanOutcomeNeedsInput` lack individual name-starting doc comments; only the block comment `// Outcome values for PlanOutput.Outcome.` covers them, while later constants in the same block each have their own. *(rule: `go-doc/comment-starts-with-name`)* *Fix:* Add per-constant comments, e.g. `// PlanOutcomeReady signals planning succeeded; Bump/NextVersion are populated.` + +### run 1 — github-releaser-agent#8 — pkg/steps_planning.go:803 + +**`pkg/steps_planning.go:803`** — `publishNothingToRelease` writes `status=completed` and `phase=done` to frontmatter directly **and** returns `Status=Done, NextPhase="done"` — double-signaling the same terminal transition. Every other path in the file relies on the framework deliverer switch (spec 047/048). The direct write is redundant with what the return value already drives. *(rule: `go-state-machine/status-phase-separation`)* *Fix:* Drop the two `md.Frontmatter` writes and rely solely on the returned `Result{Status: Done, NextPhase: "done"}`. + +### run 1 — github-releaser-agent#8 — pkg/githubtags/tags.go:137 + +**`pkg/githubtags/tags.go:137`** — On a `P1_unreleased_not_first` failure, `resolveCurrentVersion` already ran `LatestSemverTag` (full pagination) before validation, then `releaseTagCommit` → `CommitSHAForTag` re-paginates the entire tag list via `collectTags`. Two full GitHub paginations for one decision. + +### run 1 — github-releaser-agent#8 — pkg/githubtags/tags.go:5 + +**`pkg/githubtags/tags.go:5`** — The `githubtags` package comment lives in `tags.go` rather than `doc.go`. A package comment in a regular source file is silently dropped if that file is deleted or renamed. *(rule: `go-doc/package-comment-in-doc-go`)* *Fix:* Move the `// Package githubtags …` block into `pkg/githubtags/doc.go`. + +### run 1 — github-releaser-agent#8 — pkg/githubtags/tags.go:35 + +**`pkg/githubtags/tags.go:35`** — The `TagsFetcher` interface doc (`"resolves a remote GitHub repo's highest-semver tag"`) under-describes the type after `CommitSHAForTag` was added — the interface now has two distinct capabilities. *(rule: `go-doc/third-person-no-signature-repeat`)* *Fix:* Broaden the interface comment to cover both capabilities. + +### run 1 — github-releaser-agent#8 — pkg/plan_output.go:122 + +**`pkg/plan_output.go:122`** — `PreconditionP1UnreleasedNotFirst`, `PreconditionP2UnreleasedEmpty`, `PreconditionBadCurrentVersion` lack per-name comments — only the block comment covers them. *(rule: `go-doc/comment-starts-with-name`)* + +### run 1 — github-releaser-agent#8 — pkg/githubtags/tags.go:73 + +**`pkg/githubtags/tags.go:73`** — Hardcoded `15 * time.Second` HTTP client timeout appears twice (production constructor and test-only constructor) as an identical magic literal. *(rule: `go-time/inject-getter-not-create`)* *Fix:* Accept a configurable timeout or extract to a named constant. + +### run 1 — github-releaser-agent#8 — specs/completed/002-planning-close-nothing-to-release.md:48 + +**`specs/completed/002-planning-close-nothing-to-release.md:48`** — Spec is marked `status: completed` but all 15 acceptance-criteria checkboxes remain `- [ ]`. *(rule: `markdown-todo/lowercase-x-for-complete`)* + +### run 1 — github-releaser-agent#8 — None + +**Git commits** — Dark-factory auto-generated commit subjects use noun phrases (`004-spec-002-tag-commit-seam`, `005-spec-002-planning-nothing-to-release`) rather than imperative mood. Acceptable for machine-generated bookkeeping, but the final commit `51736bf dark-factory: complete spec(s)` is already correct. *(rule: `git-commit/imperative-mood`)* + +### run 1 — node-skeleton#2 — src/handlers/health.ts:6 + +**`src/handlers/health.ts:6`** — `healthRouter` reads `config.build` from a module-level `require('../config.ts')` to serve `/version`, instead of receiving the build metadata as an injected parameter. The asymmetry is visible in the same module: `isReady` is injected through `HealthRouterOptions` (line 27), and `createApp` in `server.ts` already follows the DI pattern. Because this is the skeleton every future service clones, the inconsistency propagates. The fix is a one-line addition to `HealthRouterOptions` (`build: BuildInfo`), dropping the `config` import from `health.ts`, and passing it at the `healthRouter({ isReady })` call site in `server.ts:57`. *(rule: `node/architecture/inject-dependencies`)* + +### run 1 — python-skeleton#3 — CHANGELOG.md:18 + +**`CHANGELOG.md:18`** — `ci:` is not in the allowed prefix set (`feat:`, `fix:`, `refactor:`, `test:`, `docs:`, `chore:`, `perf:`). The dark-factory version-bump detector would treat it as unrecognized. *(rule: `changelog/conventional-prefix-required`)* → Fix: `ci: install trivy in CI` → `chore: install trivy in CI` (`chore:` covers "Dependency updates, build, tooling"). + +### run 1 — python-skeleton#3 — Makefile.precommit + +**`Makefile.precommit`** — `PIP_AUDIT_VERSION ?= 2.9.0` pins a ~18-month-old release. The gate silently audits against a stale advisory database. Wire the pin to dependabot/renovate, or bump periodically. + +### run 1 — python-skeleton#3 — pyproject.toml:46 + +**`pyproject.toml:46`** — `target-version = "py314"` and `requires-python = ">=3.14"`, but `classifiers` (line 13) still advertises `Python :: 3.12` and CI installs `python-version: "3.12"` (ci.yml line 26). The metadata and the runtime the gates exercise don't match. Not introduced by this PR, but worth aligning. + +### run 1 — quant#109 — task/recurring-schedules/prod/plan-weekend.yaml + +**`task/recurring-schedules/prod/plan-weekend.yaml`** — This PR touched the file (line 67: added `due_date: "{{next_sun_date}}"`) but left the DoD at line ~62 unchanged: `"Weekend note for {{current_week}} exists in ... with Must / Should / Could"`. The sibling `plan-next-week.yaml` received a full migration to the new terminology ("priorities + acceptance criteria + explicit deferrals"). For consistency, `plan-weekend.yaml` should receive the same treatment in a follow-up. *(Not blocking this PR — the file got the due_date fix correctly.)* + +### run 1 — recurring-task-creator#30 — go.mod:95 + +**`go.mod:95`** — stale `github.com/maxbrunsfeld/counterfeiter/v6 v6.12.2 // indirect` entry would be removed by `go mod tidy`. Counterfeiter is a `go:generate` CLI tool, never imported by the build, absent from the `tool (...)` block. The grpc bump commit (`e78cdb3`) ran `go get` without a subsequent `go mod tidy`. Run `go mod tidy` and commit the resulting go.mod/go.sum before merge. *(rule: `go-mod-dependency-fix/tidy-after-get-update`)* + +### run 1 — recurring-task-creator#30 — pkg/publisher/period_token.go:105 + +**`pkg/publisher/period_token.go:105, 109, 113`** — the `firstOfMonth := time.Date(base.Year(), base.Month(), 1, 0, 0, 0, 0, base.Location())` clamp is duplicated identically in the Monthly, Quarterly, and Yearly switch cases. Factor into a small private helper (e.g. `func firstOfMonth(t time.Time) time.Time`), then `shifted := firstOfMonth(base).AddDate(...)` in each branch — makes the shared anchoring invariant explicit and avoids the triple repetition. + +### run 1 — recurring-task-creator#30 — pkg/publisher/period_token.go:30 + +**`pkg/publisher/period_token.go:30`** — `weekdayAbbrev` GoDoc says the abbreviations are "spelled per the conventional time package abbreviations" (which are capitalized, e.g. `Mon`), but the function returns lowercase (`mon`). Minor inaccuracy; reword to "the conventional time package abbreviations, lowercased" for accuracy. + +### run 1 — tts-mcp#10 — CHANGELOG.md:16 + +**`CHANGELOG.md:16,17,21`** — All three new `## Unreleased` bullets lack a conventional prefix. The rule (`changelog/conventional-prefix-required`) requires every bullet under `## Unreleased` to start with `feat:` / `fix:` / `refactor:` / `test:` / `docs:` / `chore:` / `perf:`. Current first tokens are `Bumped`, `The`, and `` `make `` — none passes. This repo has `.maintainer.yaml` with `release.autoRelease: true`; unprefixed entries break the automated version-bump determination on release. *(rule: `changelog/conventional-prefix-required`)* + +### run 1 — tts-mcp#10 — mcp/package-lock.json + +**`mcp/package-lock.json`** — `npm audit` against the committed lockfile reports **1 HIGH** (`fast-uri 3.1.4`, vulnerable `>=3.0.0 <3.1.5`, fix available at `3.1.5`) and **1 MODERATE** (`hono 4.12.32`, fix at `4.12.34`), yet `make precommit` is green. The PR's own CHANGELOG (L16) claims it "cleared" the `fast-uri` high — the claim is false for the committed tree. This is the direct consequence of the next finding. + +### run 1 — tts-mcp#10 — Makefile.precommit:30 + +**`Makefile.precommit:30`** — `check: lint typecheck mcp-typecheck` has no dependency-vulnerability scan. `npm audit` is not wired into `check`, so the `fast-uri` HIGH and `hono` MODERATE above shipped undetected. The author ran `npm audit` manually (documented in the CHANGELOG), but a gate that must be remembered is a gate that is skipped. *(rule: `node/make/security-gates-in-check`)* + +### run 1 — tts-mcp#10 — CHANGELOG.md:16 + +**`CHANGELOG.md:16-17`** — The "two remaining advisories" described at L17 (`@hono/node-server <2.0.5`) do not match the committed tree. `@hono/node-server` is at `1.19.15` and clean. The actual two remaining findings are `fast-uri` HIGH and `hono` MODERATE (see above). The security accounting needs correcting to match what the lockfile actually contains. + +### run 1 — tts-mcp#10 — Makefile.precommit:23 + +**`Makefile.precommit:23-30`** — The `mcp-typecheck` addition is the correct highest-value target, but the `mcp/` Node component's gate is still typecheck-only. No `lint` target (no eslint config exists under `mcp/`), no `test` target (no test files under `mcp/`). The full `install / precommit / format / lint / test / check` target set is only partially present. *(rule: `node/make/required-targets`)* + +### run 1 — tts-mcp#10 — CHANGELOG.md + +**`CHANGELOG.md`** — Commit subject "Bump mcp/ transitive deps and make CI actually cover the relay" (62 chars) exceeds the `<50 chars` guideline from `git-workflow.md`. Valid imperative mood; just long. *(rule: `git-commit/imperative-mood`)* + +### run 1 — tts-mcp#13 — CHANGELOG.md:12 + +**`CHANGELOG.md:12-26`** — All 9 bullets under `## Unreleased` lack Conventional Commits prefixes. The `changelog/conventional-prefix-required` rule mandates `^- ([a-z]+:)` for every entry. The auto-release bot consumes this block. Fix mapping: | Current | Prefix | |---|---| | `- Pluggable TTS engines. A new required...` | `feat:` | | `- \`POST /say\` and the MCP \`say\` tool accept...` | `feat:` | | `- \`language:\` config key, required by...` | `feat:` | | `- \`scripts/download-model.sh\` offers...` | `feat:` | | `- \`tests/test_engine.py\` covers...` | `test:` | | `- Qwen3 speakers are read from...` | `refactor:` | | `- \`discover_voices()\` moved off...` | `refactor:` | | `- Model-capability validation moved...` | `refactor:` | | `- \`generate_speech(model_id, text, voice)\` — an exported helper...` | `refactor:` | + +### run 1 — tts-mcp#13 — mcp/tts-mcp.ts + +**`mcp/tts-mcp.ts`** — `loadServerUrl()` (config validator) is never called at startup. The server boots "healthy" with a broken `config.yaml` and fails only on first tool request (`node/config/validate-before-serving`). Call `loadServerUrl()` once before `server.connect(transport)` and `process.exit(1)` on failure. *(rule: `node/config/validate-before-serving`)* + +### run 1 — tts-mcp#13 — mcp/tts-mcp.ts + +**`mcp/tts-mcp.ts`** — Each tool invocation re-reads and re-parses `config.yaml` from disk twice (`healthCheck()` then `request()`). Resolve `{host, port}` into a module-level constant at startup and reuse it. + +### run 1 — tts-mcp#16 — skills/voice/SKILL.md:3 + +**`skills/voice/SKILL.md:3`** — frontmatter `description` is invalid YAML. The sequence `Args: on | narrate | interview | off | status | restart` contains `:` followed by a space inside the plain scalar — both PyYAML and js-yaml reject it with a scanner/indentation error. If a strict parser loads this skill, it fails entirely; a lenient reader truncates at the first `: `, silently dropping the activation info. Fix: either quote the description or replace `Args:` with `Args:` on the same line (e.g. `Args — on | narrate | …` or `description: "…"`). *(rule: `skill-writing/skill-md-frontmatter-required`)* + +### run 1 — tts-mcp#16 — skills/voice/SKILL.md + +**`skills/voice/SKILL.md`** — add a `## Success Criteria` section. The guide expects one; a short closing block (mode confirmed on-screen and spoken, `status` reflects the arg, `restart` passes `/tts-mcp:voice-selfcheck`) makes completion verifiable. + +### run 1 — tts-mcp#16 — skills/voice/SKILL.md + +**`skills/voice/SKILL.md`** — add a `## Prerequisites` section listing the launchd service, health endpoint, and required MCP tools — currently scattered inline. + +### run 1 — tts-mcp#16 — commands/voice.md + +**`commands/voice.md`** — the skill documents `/voice` activation but no matching thin-wrapper command exists in `commands/`. Pre-existing, not introduced by this diff, but worth resolving for consistency. + +### run 1 — tts-mcp#20 — CHANGELOG.md:8 + +**`CHANGELOG.md:8`** — The PR changes a non-vendored file (`mcp/package-lock.json`) but `CHANGELOG.md` has no `## Unreleased` section. This repo sets `release.autoRelease: true` in `.maintainer.yaml`; the release agent promotes `## Unreleased` → `## vX.Y.Z` post-merge, so with no bullet the release no-ops and the fast-uri bump never ships. *(rule: `changelog/unreleased-entry-required`)* + +### run 1 — tts-mcp#3 — CHANGELOG.md:10 + +**`CHANGELOG.md:10–16`** — `## Unreleased` entries lack conventional prefixes. The coding plugin's `changelog/conventional-prefix-required` rule (owner: `agent-auditor`) requires every bullet under `## Unreleased` to begin with `feat:` / `fix:` / `chore:` / etc. These entries use bare prose: ``` - `commands/voice-test.md` ships a `/tts-mcp:voice-test` slash command … - `/tts-mcp:voice on` and `interview` no longer run a startup selftest … ``` They should be prefixed as `feat:` (new capability) and `chore:` (behavioural simplification) respectively. Without the prefix, dark-factory's release-bump detector cannot classify the version bump automatically. *(rule: `changelog/conventional-prefix-required`)* + +### run 1 — tts-mcp#3 — CHANGELOG.md:10 + +**`CHANGELOG.md:10–16`** — `## Unreleased` uses `### Added` / `### Changed` subcategory headers. The changelog guide specifies a flat list under `## Unreleased` with no `###` sub-headers. Keep a Changelog format and the coding plugin's flat-list convention are not compatible here; pick one and apply it consistently. *(rule: `changelog/preamble-frozen` style note — subcategory structure vs. flat list)* + +### run 1 — tts-mcp#3 — commands/voice-test.md + +**`commands/voice-test.md`** — The frontmatter has `description:` but no `name:` field. The template in the skill-writing guide shows both `name:` and `description:` as required frontmatter fields. Adding `name: voice-test` would align with the template and make the command self-documenting in IDE hover/tooling. + +### run 1 — vault-ui#39 — tests/test_api.py:1617 + +**`tests/test_api.py:1617`** — `test_list_goals_today_defer_date_included` fails: the test asserts `defer_date=today` produces `upcoming=False`, but `defer_date` is parsed as `YYYY-MM-DD 00:00:00+UTC` while `now = datetime.now(UTC)` is the current wall-clock UTC time. In timezones east of UTC, `date.today()` advances a day ahead of `datetime.now(UTC)` before midnight UTC — so `defer_dt` ends up in the `[now, cutoff]` window and `upcoming=True`. The test passes on `origin/bench-base-39` because `upcoming_hours=8` still covers the current UTC date at test-run time; it will fail predictably near UTC midnight. Fix: use `datetime.now(UTC).date().isoformat()` or mock `datetime.now` in the test. *(not introduced by this PR — revealed by precommit)* + +### run 1 — vault-ui#39 — src/vault_ui/api/tasks.py:705 + +**`src/vault_ui/api/tasks.py:705–709`** (and `:1794–1798`) — `derive_claude_project_dir(...)` is called once per task/goal row inside the response builder. For a vault with hundreds of tasks, this recomputes the path string and encodes it each time. Move the call to the top of `_task_to_response` / `_goal_to_response` before the loop (or share it via a `vault_config` field) to avoid O(n) redundant string operations. *(not introduced by this PR — a pre-existing optimisation opportunity in the same functions this PR touches)* + +### run 1 — vault-ui#39 — src/vault_ui/activity.py:36 + +**`src/vault_ui/activity.py:36`** — `_claude_projects_root()` uses `Path.home()` which reads `HOME` at call time. If `HOME` changes mid-process (e.g. a privilege drop), the function returns an inconsistent path. Accept `projects_root: Path | None` from the caller (already available in `compute_activity_date`'s signature) and remove the module-level fallback. The current `None`-guard in `transcript_mtime` works but makes the fallback path untestable in isolation. + +### run 1 — vault-ui#39 — src/vault_ui/static/app.js:1108 + +**`src/vault_ui/static/app.js:1108`** — `formatActivityAge` recomputes `Date.now()` on every card render. With hundreds of cards, this adds unnecessary jitter to the render loop. Compute once at module level on load and expose it as `const _now = Date.now()` refreshed via `setInterval(1000)` for the ticking-clock behaviour that `title` already provides. diff --git a/bench/reports/d4558caff3dfefdff574d9a14345bb4007db9cb94a69c64e2e859c7d6e8b2dab.md b/bench/reports/d4558caff3dfefdff574d9a14345bb4007db9cb94a69c64e2e859c7d6e8b2dab.md new file mode 100644 index 0000000..41ecaa4 --- /dev/null +++ b/bench/reports/d4558caff3dfefdff574d9a14345bb4007db9cb94a69c64e2e859c7d6e8b2dab.md @@ -0,0 +1,1074 @@ +d4558caff3dfefdff574d9a14345bb4007db9cb94a69c64e2e859c7d6e8b2dab + +## Configuration +- model: opus +- effort: xhigh +- mode: full +- config_hash: d4558caff3dfefdff574d9a14345bb4007db9cb94a69c64e2e859c7d6e8b2dab +- rules_commands_hash: ecc803331f860b845a6a1b8a103e889bce02520e8cc04ea88102de74c8d5600d +- prs_version: curated-1 +- coding version: 0.45.0 +- golden baseline coding version: v0.35.6 +- golden version: golden-curated-3 +- runner_version: 1 +- rows skipped: 0 +- cost: not recorded — the ledger carries no cost field. + +*golden-curated-3* carries 20 `rejected` entries, so precision is measurable: reporting one costs precision. With so few adjudicated, a single hit still moves the number a long way — read it as directional, not calibrated. + +No signature embeds a line reference, so a re-report of the same issue at a different line still matches. `recall` measures issue detection. + +## Runs +| run | span | PRs | complete | golden in scope | findings | hits | misses | matched rejected | gap candidates | recall | precision | wall time (s) | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | +| 1 | 2026-08-09T20:54:31.750374+00:00 … 2026-08-09T23:20:57.512212+00:00 | 20/20 | complete | 158 | 133 | 114 | 24 | 19 | 0 | 0.826 | 0.857 | 8967 | +| 2 | 2026-08-10T14:29:34.932356+00:00 … 2026-08-12T11:12:44.846706+00:00 | 20/20 | complete | 158 | 148 | 24 | 114 | 0 | 124 | 0.174 | 1.000 | 8675 | +| 3 | 2026-08-12T10:26:55.009865+00:00 … 2026-08-12T12:27:46.663498+00:00 | 19/20 | partial | 156 | 143 | 22 | 114 | 0 | 121 | 0.162 | 1.000 | 7148 | + +## Per-PR +| run | pr_id | golden in scope | hits | misses | findings | gap candidates | dropped items | missing sections | duration (s) | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | +| 1 | tts-mcp#20 | 1 | 1 | 0 | 1 | 0 | 0 | — | 204 | +| 1 | github-pr-review-agent#11 | 16 | 8 | 6 | 9 | 0 | 0 | — | 1005 | +| 1 | quant#109 | 10 | 6 | 4 | 6 | 0 | 0 | — | 249 | +| 1 | node-skeleton#2 | 12 | 6 | 6 | 6 | 0 | 0 | — | 640 | +| 1 | python-skeleton#3 | 17 | 11 | 5 | 12 | 0 | 0 | — | 546 | +| 1 | vault-cli#68 | 1 | 1 | 0 | 1 | 0 | 0 | — | 367 | +| 1 | backup#15 | 4 | 2 | 0 | 4 | 0 | 0 | — | 270 | +| 1 | bw#41 | 3 | 2 | 0 | 3 | 0 | 0 | — | 309 | +| 1 | dark-factory#71 | 2 | 2 | 0 | 2 | 0 | 0 | — | 381 | +| 1 | tts-mcp#16 | 6 | 5 | 0 | 6 | 0 | 0 | — | 385 | +| 1 | tts-mcp#3 | 6 | 5 | 0 | 6 | 0 | 0 | — | 374 | +| 1 | distill#6 | 10 | 7 | 0 | 10 | 0 | 0 | — | 450 | +| 1 | vault-ui#39 | 6 | 6 | 0 | 6 | 0 | 0 | — | 342 | +| 1 | recurring-task-creator#30 | 6 | 3 | 1 | 5 | 0 | 0 | — | 400 | +| 1 | tts-mcp#10 | 9 | 7 | 0 | 9 | 0 | 0 | — | 392 | +| 1 | discord-assistant#5 | 13 | 12 | 1 | 12 | 0 | 0 | — | 440 | +| 1 | tts-mcp#13 | 10 | 9 | 1 | 9 | 0 | 0 | — | 299 | +| 1 | github-releaser-agent#8 | 13 | 9 | 0 | 13 | 0 | 0 | — | 1175 | +| 1 | discord-assistant#1 | 9 | 9 | 0 | 9 | 0 | 0 | — | 556 | +| 1 | helm#3 | 4 | 3 | 0 | 4 | 0 | 0 | — | 184 | +| 2 | tts-mcp#20 | 1 | 1 | 0 | 2 | 1 | 0 | — | 217 | +| 2 | github-pr-review-agent#11 | 16 | 4 | 10 | 12 | 8 | 0 | — | 641 | +| 2 | quant#109 | 10 | 2 | 8 | 5 | 3 | 0 | — | 232 | +| 2 | node-skeleton#2 | 12 | 2 | 10 | 12 | 10 | 0 | — | 544 | +| 2 | python-skeleton#3 | 17 | 4 | 12 | 8 | 4 | 0 | — | 325 | +| 2 | vault-cli#68 | 1 | 0 | 1 | 0 | 0 | 0 | — | 285 | +| 2 | backup#15 | 4 | 0 | 2 | 3 | 3 | 0 | — | 278 | +| 2 | bw#41 | 3 | 1 | 1 | 3 | 2 | 0 | — | 361 | +| 2 | tts-mcp#16 | 6 | 0 | 5 | 3 | 3 | 0 | — | 545 | +| 2 | tts-mcp#3 | 6 | 0 | 5 | 6 | 6 | 0 | — | 372 | +| 2 | distill#6 | 10 | 3 | 4 | 11 | 8 | 0 | — | 732 | +| 2 | vault-ui#39 | 6 | 0 | 6 | 9 | 9 | 1 | — | 444 | +| 2 | recurring-task-creator#30 | 6 | 1 | 3 | 8 | 7 | 0 | — | 473 | +| 2 | tts-mcp#10 | 9 | 1 | 6 | 11 | 10 | 0 | — | 552 | +| 2 | discord-assistant#5 | 13 | 1 | 12 | 13 | 12 | 0 | — | 620 | +| 2 | tts-mcp#13 | 10 | 2 | 8 | 16 | 14 | 0 | — | 455 | +| 2 | github-releaser-agent#8 | 13 | 0 | 9 | 8 | 8 | 0 | — | 534 | +| 2 | discord-assistant#1 | 9 | 0 | 9 | 9 | 9 | 0 | — | 575 | +| 2 | helm#3 | 4 | 2 | 1 | 5 | 3 | 0 | — | 205 | +| 2 | dark-factory#71 | 2 | 0 | 2 | 4 | 4 | 0 | — | 283 | +| 3 | tts-mcp#20 | 1 | 0 | 1 | 1 | 1 | 0 | — | 221 | +| 3 | github-pr-review-agent#11 | 16 | 2 | 12 | 10 | 8 | 0 | — | 317 | +| 3 | quant#109 | 10 | 2 | 8 | 8 | 6 | 0 | — | 254 | +| 3 | node-skeleton#2 | 12 | 1 | 11 | 11 | 10 | 0 | — | 591 | +| 3 | python-skeleton#3 | 17 | 3 | 13 | 8 | 5 | 0 | — | 401 | +| 3 | vault-cli#68 | 1 | 0 | 1 | 1 | 1 | 0 | — | 324 | +| 3 | backup#15 | 4 | 0 | 2 | 4 | 4 | 0 | — | 345 | +| 3 | bw#41 | 3 | 0 | 2 | 4 | 4 | 4 | — | 296 | +| 3 | tts-mcp#16 | 6 | 0 | 5 | 7 | 7 | 0 | — | 393 | +| 3 | tts-mcp#3 | 6 | 1 | 4 | 7 | 6 | 0 | — | 302 | +| 3 | distill#6 | 10 | 2 | 5 | 7 | 5 | 0 | — | 315 | +| 3 | vault-ui#39 | 6 | 0 | 6 | 8 | 8 | 0 | — | 533 | +| 3 | recurring-task-creator#30 | 6 | 0 | 4 | 6 | 6 | 0 | — | 425 | +| 3 | tts-mcp#10 | 9 | 3 | 4 | 7 | 4 | 0 | — | 338 | +| 3 | discord-assistant#5 | 13 | 1 | 12 | 10 | 9 | 0 | — | 435 | +| 3 | tts-mcp#13 | 10 | 3 | 7 | 17 | 14 | 0 | — | 422 | +| 3 | github-releaser-agent#8 | 13 | 1 | 8 | 11 | 10 | 0 | — | 661 | +| 3 | discord-assistant#1 | 9 | 1 | 8 | 11 | 10 | 0 | — | 393 | +| 3 | helm#3 | 4 | 2 | 1 | 5 | 3 | 0 | — | 182 | + +*Effective fixture: **59 of 60** PRs produced a scored row.* The 1 absent row is not a zero-finding result — they were rejected before scoring, so they lower confidence without lowering recall. **5** findings carried no matching key and never reached scoring. + +## Gap-triage candidates +These findings matched no golden entry. They are **not a precision failure** — the golden set is a bootstrap from one strong-model run, and a finding it does not describe is evidence the set is incomplete. + +### run 2 — tts-mcp#20 — mcp/package-lock.json + +**`mcp/package-lock.json`** — commit message `Bump fast-uri from 3.1.4 to 3.1.5 in /mcp` is the Dependabot default and omits the security rationale (GHSA-7p8r-x3mc-p8w7). The repo's prior dep-bump commit documented advisory IDs, verification method, and what was deliberately left unfixed. + +### run 2 — github-pr-review-agent#11 — pkg/factory/runner.go:66 + +**`pkg/factory/runner.go:66-73`** — `ResolvePosters` places a boot-time conditional inside `pkg/factory`, contradicting this repo's own stated invariant ("Factory functions are pure composition — no conditionals, no I/O") and the package doc at `pkg/factory/factory.go:5-9` ("zero business logic"). The branch is wiring *policy*, not composition. Fix: delete the helper and inline the branch in `RunAgent` (an orchestrator, where conditionals are already idiomatic — see lines 105-131), keeping `Create*` functions unconditional. *(rule: `go-factory/no-conditional-in-body`, `go-factory/no-impl-in-factory-pkg`)* + +### run 2 — github-pr-review-agent#11 — pkg/factory/runner.go:57 + +**`pkg/factory/runner.go:57-60`** — the `SkipPost` doc says "suppresses all GitHub write calls" unconditionally, but the effect only materialises when `RunConfig.Agent == nil` (the call site at line 133-136 is inside `if agent == nil`). Production `main.go` sets `Agent` via `CreateAgentProvider`, so `SkipPost: true` there is silently inert — a fail-open trap for any future caller. Fix: scope the comment to "only honoured when `Agent` is nil; `CreateAgentProvider`-built agents ignore it," or thread the flag through `CreateAgentProvider` too. + +### run 2 — github-pr-review-agent#11 — pkg/skip_post_boundary_test.go:224 + +**`pkg/skip_post_boundary_test.go:224`** — case 4's only load-bearing assertion is a non-discriminator. `appendVerifyDiagnostic` (`pkg/steps_review.go:119-126`) runs *only* when `callVerifier` returns non-nil, i.e. only on verification **failure** — a verifier that ran and *succeeded* also leaves `## Diagnostics` free of `"ai_review verify:"`. The comment at lines 220-221 ("if callVerifier were reached despite the suppressed verifier, it would have appended one") is therefore false, and the test would pass with the guard deleted. Fix: assert on a real mock verifier's `VerifyReviewCallCount() == 0`, which does distinguish the two states. *(rule: `test-pyramid/push-down-when-unsure`)* + +### run 2 — github-pr-review-agent#11 — pkg/skip_post_boundary_test.go:102 + +**`pkg/skip_post_boundary_test.go:102-167`** — case 3 does not exercise the contract its comment claims. It discards the `ResolvePosters` verifier (`poster, _ :=` at line 124) and substitutes `&mocks.ReviewVerifier{}`; and on the `pass`-verdict path `tryDismissHallucinated` is never called, so the nil poster is never touched either. What remains duplicates the pre-existing "verification runs and succeeds" case at `pkg/steps_review_test.go:298-321` — same fixture shape, same `VerifyResult`, same two assertions. Fix: delete, or rewrite to pass the verifier from `ResolvePosters`. *(rule: `test-pyramid/push-down-when-unsure`)* + +### run 2 — github-pr-review-agent#11 — pkg/factory/runner_test.go:37 + +**`pkg/factory/runner_test.go:37-45`** — the `SkipPost: false` case passes `CurrentDateTime: libtime.NewCurrentDateTime()`, but the real `cmd/run-task` wiring never sets that field (verified: `CurrentDateTime` appears in prod `main.go:188` and nowhere in `cmd/run-task/main.go`). The fixture is more populated than production, so it cannot surface the nil-getter path it nominally covers. + +### run 2 — github-pr-review-agent#11 — cmd/run-task/main.go:76 + +**`cmd/run-task/main.go:76`** — `default:"false"` restates the bool zero value and is the only `default:` tag in the struct; the other 20+ fields omit it. + +### run 2 — github-pr-review-agent#11 — CHANGELOG.md:10 + +**`CHANGELOG.md:10`** — "which blocked the dark-factory preflight baseline" is internal tooling detail in a user-facing Keep-a-Changelog file; the GO-2026-5841 half is correctly specific. + +### run 2 — github-pr-review-agent#11 — prompts/cancelled/001-run-task-skip-post-flag.md + +**`prompts/cancelled/001-run-task-skip-post-flag.md`** — number `001` is reused by `prompts/completed/001-run-task-skip-post-flag.md`; the two differ only in frontmatter status. Ambiguous for any tooling that keys on the number. + +### run 2 — quant#109 — task/recurring-schedules/prod/weekly-review.yaml:64 + +**`task/recurring-schedules/prod/weekly-review.yaml:64`** — same Sunday due date as `plan-next-week.yaml:67`, but this PR newly makes the two ordered: `plan-next-week.yaml:40` adds a pre-check requiring the previous week's note to have `reviewed: true`, which `/complete-week` produces — and `/complete-week` is a step inside `weekly-review` (line 39). Prerequisite and dependent now share one deadline, so the ordering the new pre-check documents isn't encoded anywhere. Consider `{{current_date}}` for `weekly-review` so it lands before `plan-next-week`. + +### run 2 — quant#109 — task/recurring-schedules/prod/plan-next-week.yaml:29 + +**`task/recurring-schedules/prod/plan-next-week.yaml:29`** — "Month link present (target may not exist yet if the monthly plan is deferred)" is a success criterion that can pass with a dangling link. Reads as intentional (relaxing the old "Linked to current month's plan"), but a checkbox that permits a broken wikilink will not catch the case where the month link is simply wrong rather than deferred. + +### run 2 — quant#109 — task/recurring-schedules/prod/plan-next-week.yaml:40 + +**`task/recurring-schedules/prod/plan-next-week.yaml:40`** — this pre-check restates a gate the diff says `/plan-week` already enforces ("`/plan-week` is gated on both"). Useful as a heads-up, but it will drift silently if the command's gating changes. + +### run 2 — node-skeleton#2 — src/server.ts:31 + +**`src/server.ts:31`** — `CreateApp` is declared but never applied to the implementation at `src/server.ts:40`, so `tsc` never checks that the published type matches the function. Verified: making `options` required while leaving the alias optional gives **`tsc --noEmit` exit 0** and a runtime `Cannot destructure property 'isReady' of 'undefined'`. Fix verified to close it: `export type CreateApp = typeof createApp;` (re-running the same drift then fails typecheck with 4 errors, and is clean without drift). + +### run 2 — node-skeleton#2 — src/handlers/health.ts:12 + +**`src/handlers/health.ts:12`** — same pattern: `HealthRouter` is never applied to `healthRouter` at `src/handlers/health.ts:27`. Same fix. + +### run 2 — node-skeleton#2 — src/index.ts:8 + +**`src/index.ts:8-10`** — the `const x: T = require('./y.ts')` idiom used across `src/log.ts:5`, `src/server.ts:6-9`, `src/handlers/health.ts:5-6`, `test/health.test.ts:5-7` is an *unchecked* assertion, not a verified type — `require()` erases to `any`. Verified: rebinding `const log: Logger = require('./config.ts')` typechecks green and would crash on first `log.error`. `Config` and `Logger` are safe only because their producers (`src/config.ts:34`, `src/log.ts:42`) annotate the literal; the consumer side is unverified everywhere. Worth an explicit comment in the template, since this is the one place the conversion's safety claim doesn't hold. + +### run 2 — node-skeleton#2 — src/log.ts:24 + +**`src/log.ts:24-25`** — `LEVELS_BY_NAME[config.logLevel] ?? LEVELS.info` reads through the prototype chain. Verified with `LOG_LEVEL=constructor`: `threshold` becomes a *function*, `??` doesn't fall back, and `LEVELS['debug'] > threshold` is `false` — **every level emits, including debug, in production**. The added comment at `src/log.ts:21-23` claims `noUncheckedIndexedAccess` resolves this; it doesn't, for inherited keys. `k8s/skeleton-deploy.yaml:33` pins `info`, so this needs an operator typo to trigger. Fix: validate `logLevel` against the four levels in `config.check()`, or use `Object.hasOwn`. + +### run 2 — node-skeleton#2 — README.md:20 + +**`README.md:20`** — Quick start never states the minimum Node version for the sources to run, nor that there is no build step / no `dist/`. Both are new user-facing facts created by this PR; `CLAUDE.md:11` documents them for agents only. Fix: one Requirements line under Quick start. + +### run 2 — node-skeleton#2 — eslint.config.js:44 + +**`eslint.config.js:44-46`** — the comment explaining the `erasableSyntaxOnly` pairing sits directly above `no-explicit-any` and `no-non-null-assertion`, which have nothing to do with erasable syntax. Move it above the `no-require-imports` block or the tsconfig reference. + +### run 2 — node-skeleton#2 — test/health.test.ts:22 + +**`test/health.test.ts:22`** — falling back to port `0` when `server.address()` isn't an `AddressInfo` turns a listen failure into a confusing connection error. Throw instead. + +### run 2 — node-skeleton#2 — src/config.ts:52 + +**`src/config.ts:52`** — `check()` accepts `SHUTDOWN_TIMEOUT_MS=0`, which forces immediate exit and defeats the drain path; it also doesn't enforce the documented `< terminationGracePeriodSeconds` invariant. + +### run 2 — node-skeleton#2 — CHANGELOG.md:13 + +**`CHANGELOG.md:13`** — "startup-validated config (`config.check()`) instead of a type assertion" mischaracterizes the old code, which threw at module load (`src/config.js`, deleted) rather than asserting. Also, the new README License section isn't covered by any bullet. + +### run 2 — node-skeleton#2 — README.md:42 + +**`README.md:42`** — the `make typecheck` row's "the only step that validates types" rationale duplicates the `CLAUDE.md:11` invariant; sibling rows stay descriptive. + +### run 2 — python-skeleton#3 — .github/workflows/ci.yml:33 + +**`.github/workflows/ci.yml:33`** — `$(lsb_release -sc)` resolves to `noble` on `ubuntu-latest`, but the Aqua deb repo consolidated every release under a single `generic` suite. `deb …/trivy-repo/deb noble main` 404s on `InRelease`, so `apt-get update` errors and `apt-get install -y trivy` then fails — the job dies before `make precommit`. Upstream also moved the host to `get.trivy.dev`. Fix: `deb [signed-by=/usr/share/keyrings/trivy.gpg] https://get.trivy.dev/deb generic main`, or drop the hand-rolled block for `aquasecurity/setup-trivy@v0.2.x`. This never surfaced because `on:` restricts CI to `master`/`main` (`ci.yml:4-7`), so no run on this branch exercised the new step. + +### run 2 — python-skeleton#3 — pyproject.toml:64 + +**`pyproject.toml:64-68`** — `S104` is suppressed globally, yet only two call sites need it (`src/skeleton/server.py:9`, `src/skeleton/__main__.py:33`). The file already demonstrates the narrower mechanism two lines below for `S101`; scoping S104 via `per-file-ignores` keeps the check live for code added later. The stated reason ("Overridable via the HOST env var") is accurate — verified at `__main__.py:33` and `example.env:2`. + +### run 2 — python-skeleton#3 — .github/workflows/ci.yml:33 + +**`.github/workflows/ci.yml:33`** — `tee -a` appends to `trivy.list`; plain `tee` is idempotent if the step is ever re-run in a reused workspace. + +### run 2 — python-skeleton#3 — README.md:82 + +**`README.md:82`** — "a third advisory database over ground pip-audit and trivy already cover" reads as a typo; likely "over ground that pip-audit and trivy already cover". + +### run 2 — backup#15 — Dockerfile:8 + +**`Dockerfile:8`** — the default registry host is duplicated with `Makefile:6`. The exact failure this PR fixes (host decommissioned → every image build 404s) now has two places that must be updated in lockstep, and the Dockerfile's copy is the one nobody exercises since `make build` always passes `--build-arg`. It also leaves a bare `docker build .` failing off the nuke network, which contradicts the PR's own stated goal ("target … public npm without editing the Dockerfile"). Fix: `ARG NPM_REGISTRY=https://registry.npmjs.org` and let `Makefile:6` remain the single source for the verdaccio default. + +### run 2 — backup#15 — Dockerfile:13 + +**`Dockerfile:13`** — `npm install --verbose --include=optional` now runs against a caller-overridable registry. The line itself is unchanged, but the risk is introduced by this diff: `npm install` rewrites `frontend/package-lock.json` on mismatch instead of failing, so a build with a different `NPM_REGISTRY` can silently resolve versions the lockfile does not pin. Fix: `npm ci --include=optional` so a registry/lockfile mismatch fails loudly. + +### run 2 — backup#15 — Makefile:127 + +**`Makefile:127`** — the `docker build` line carries no `BUILD_GIT_VERSION` / `BUILD_GIT_COMMIT` / `BUILD_DATE` build args, and the Go stage has no matching `-ldflags "-X"` wiring. Pre-existing, not introduced here; noted because this diff is what touched the line. *(rule: `go-build-args/three-args-required`)* + +### run 2 — bw#41 — groups/meta/k3s.py:2 + +**`groups/meta/k3s.py:2-4`** — Coverage gap: membership is keyed on the node *name* (`r'.*k3s.*'`), not on `k3s.enabled`. `hm.hell` has `'k3s': {'enabled': True}` (`nodes/hm.hell.py:66`) but its name doesn't match the pattern, so it does not get the fix. Verified empirically: ``` bw metadata hm.hell -k sysctl → {net.ipv4.ip_forward, vm.swappiness} # no gc_thresh bw metadata hm.nuke-k3s-prod-0 -k sysctl → …gc_thresh1/2/3 present # ok ``` hell runs pods, so it has the same one-veth-per-pod pressure that overflowed the global `arp_tbl` on `nuke-k3s-prod-0` — it is silently left on the 128/512/1024 defaults. Every other sysctl provider in this repo gates on the bundle's own flag (`netplan`, `kvm-guest`, `kvm-host`, `intellij` all use `metadata.get('', {}).get('enabled', False)`). Moving this block into `bundles/k3s/metadata.py` as a `@metadata_reactor.provides('sysctl/options')` gated on `k3s.enabled` covers hell automatically and matches the established idiom. + +### run 2 — bw#41 — bundles/intellij/metadata.py:28 + +**`bundles/intellij/metadata.py:28-29`** — Unrelated whitespace-only change in an incident-fix PR. It is its own commit (`de4f030 fmt`), so the cost is low, but it widens the diff of a change that will likely be cherry-picked or referenced later. + +### run 2 — tts-mcp#16 — skills/voice/SKILL.md:8 + +**`skills/voice/SKILL.md:8`** — states as fact "there is no competing always-on rule in `~/.claude/CLAUDE.md`". That file lives outside this repo and differs on every install, so the skill cannot know this. For a user who *does* have such a rule, the skill asserts a falsehood — and Claude, reading the skill, would trust the assertion over the actual memory file. That is the same silent-outranking bug this PR sets out to kill, pointed the other way. Line 94 already has the correct framing ("If you want a persistent default back … then this file stops being the single source of truth"); line 8 should match it. Fix: claim ownership, not absence — e.g. *"This skill owns spoken-output volume. If a rule in `~/.claude/CLAUDE.md` also sets it, that rule wins and this file is no longer the single source of truth."* The frontmatter clause at **`skills/voice/SKILL.md:3`** ("Sole authority on spoken-output volume — no always-on rule competes with it") needs the same softening. *(rule: `agent-cmd/single-source-of-truth`)* + +### run 2 — tts-mcp#16 — CHANGELOG.md:16 + +**`CHANGELOG.md:16`** — `feat:` prefix is semantically off. The bullet describes consolidating existing rules into one file plus fixing a precedence conflict; that reads as `fix:` or `refactor:`, not a new capability (the actual new capability is the `narrate` bullet at line 12). Net impact is nil here — line 12's genuine `feat:` already forces the minor bump — and `v0.4.0:23` sets precedent for `feat:` under `### Changed`, so this is a consistency nit rather than a bump hazard. *(rule: `changelog/conventional-prefix-required`)* + +### run 2 — tts-mcp#16 — skills/voice/SKILL.md:69 + +**`skills/voice/SKILL.md:69-74`** — `narrate` and `interview` are mutually exclusive by construction (each is defined as "`on` plus X"), but they address the same user: away from the screen. Someone driving by voice who wants both spoken answers *and* every question read aloud has no way to ask for it. Worth either documenting the exclusivity deliberately or allowing `/voice narrate interview`. + +### run 2 — tts-mcp#3 — commands/voice-test.md:22 + +**`commands/voice-test.md:22`** — introduces a circular procedure with `skills/voice/SKILL.md:32-33`: restart step 3 says "Verify: run `/tts-mcp:voice-test`", and voice-test's last troubleshooting bullet says "Still silent → run `/tts-mcp:voice restart` … then re-run this test". SKILL.md:33 makes it explicit — the *already-restarted-and-still-broken* branch routes into a procedure whose terminal step is another restart. There's no attempt counter, and per SKILL.md:35 each restart drops in-flight messages for **every** Claude session sharing the server. The pre-PR text terminated cleanly (old handshake troubleshooting ended at "report the failure, fall back to on-screen only" — it never restarted). Fix: make voice-test's restart bullet conditional ("only if you have not already restarted this session; otherwise go to step 5"), or have SKILL.md:33 point at voice-test steps 4a–4c only. + +### run 2 — tts-mcp#3 — commands/voice-test.md:1 + +**`commands/voice-test.md:1-3`** — frontmatter declares only `description`; `allowed-tools` is missing, leaving the command's tool permissions unbounded. The tool surface is small and known: `mcp__tts__say`, `mcp__tts__get_voices`, `mcp__tts__get_status`. `argument-hint` is correctly omitted (no arguments). *(rule: `agent-cmd/command-frontmatter`)* + +### run 2 — tts-mcp#3 — skills/voice/SKILL.md:20 + +**`skills/voice/SKILL.md:20-24`** — the PR's own mitigation isn't wired into the path that needs it. `interview` is defined at SKILL.md:13 as the mode "used when the user is away from the keyboard", and `commands/voice-test.md:11` names "the user is about to walk away and rely on voice alerts" as a reason to run the test — yet activation now just flips the mode and nothing prompts the user toward `/tts-mcp:voice-test`. The one-line confirmation at SKILL.md:18 doesn't mention it either. Dropping the gate for `on` is defensible; dropping it for `interview` reinstates precisely the "walks away trusting voice, never gets alerted" failure the deleted section called the worst case. Fix: have `interview` either run the test or append a suggestion to its confirmation line. + +### run 2 — tts-mcp#3 — CHANGELOG.md:12 + +**`CHANGELOG.md:12`** — `## Unreleased` bullets carry no conventional prefix (`feat:` / `docs:` / …). *(rule: `changelog/conventional-prefix-required`)* Low priority and arguably correct as-is: the file's preamble declares Keep a Changelog, and the released `## v0.1.0` section uses the same `### Added` / `### Changed` + prose-bullet shape. `.maintainer.yaml` has `autoRelease: true`, but per CHANGELOG:29 the releaser cuts from the `## Unreleased` block rather than parsing prefixes. Changing this PR alone would make the file internally inconsistent — migrate wholesale in a follow-up or leave it. + +### run 2 — tts-mcp#3 — commands/voice-test.md:5 + +**`commands/voice-test.md:5`** — inconsistent command naming within one file: `/voice on` here and `/voice restart` at line 10, versus the prefixed `/tts-mcp:voice restart` at line 22. + +### run 2 — tts-mcp#3 — skills/voice/SKILL.md:3 + +**`skills/voice/SKILL.md:3`** — `description` is now 453 chars; the appended sentence is a caveat about what activation *doesn't* do, not an activation trigger, so it adds match-surface without adding discovery value. Body sections 20–24 already say it. + +### run 2 — distill#6 — hooks/deny-generated-file-edits.sh:6 + +**`hooks/deny-generated-file-edits.sh:6`** — The hook denies edits to *itself*. Its own header comment contains the literal string `"AUTO-GENERATED by distill"` at byte 255, well inside the 2048-byte window that line 34 greps, so `grep -qi 'AUTO-GENERATED by distill'` matches. Verified empirically: feeding `{"tool_name":"Edit","tool_input":{"file_path":".../hooks/deny-generated-file-edits.sh"}}` into the script returns `permissionDecision: "deny"`. Once installed per `README.md:128`, no agent can edit the hook — including to fix this — and the denial reason falsely asserts the script "is AUTO-GENERATED by distill". Same trap applies to the installed copy at `~/.claude/hooks/`. Fix: anchor detection structurally instead of on a bare substring — require line 1 to be `