fix(cleanup): the review's defects on the R-a diff
The trekcontinue-stats line_ref follows Phase 5 to 271-275 (the Phase 1.5 cut moved it). gold-eval.test.mjs and its run fixture come back: they are the one end-to-end check that the review contract keeps every seeded finding, and they stay with the scorer until R-b. CHANGELOG: the removed validator leaves the known-issues text; the review's other findings outside R-a's files are listed as known issues. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
fb937ee306
commit
5f42bbdb9e
4 changed files with 73 additions and 5 deletions
10
CHANGELOG.md
10
CHANGELOG.md
|
|
@ -57,7 +57,7 @@ Part B makes a WARN open the merge lock and the release check only when the oper
|
|||
- **`/trekendsession` is operator-only, and the planning and research spawn sites follow the profile.** The model can no longer start `/trekendsession` (`disable-model-invocation: true`). With `--project`, where the resolver runs, `/trekplan` and `/trekresearch` pass the resolved model at their spawn sites, so the `economy` profile gives Sonnet helpers unless the brief's phase signal for that phase names a model; without a resolved model the agent's own `model:` applies (opus for Claude Code's built-in agents), and `/trekexecute`'s step helper and controller stay on opus.
|
||||
- **`~` and `$HOME` in the arguments of `/trekendsession`, `/trekcontinue` and `/trekreview` are no longer expanded** (ARG, fix round 4.5b). The arguments reach the arg parser as text on stdin, not as shell words, so nothing in them is expanded: write the path out (`~/x` stays the three characters `~/x`).
|
||||
- **The autonomy gate is the `--gates` flag parser** (cleanup R-a, R9). `lib/util/autonomy-gate.mjs` keeps only `gatesValue` and `parseGatesArgs`: `--gates [value]` prints `{"ok":true,"gates":true|false}`, a bad value a usage error (exit 1). The state machine (`STATES`, `EVENTS`, `transition`), which no command used, is gone; the four commands call the parser with the flag only.
|
||||
- **The live tests left the round files** (cleanup R-a, R8). The tests of live behaviour in the `retterunde*`, `trial*`, `fix-round-4-5a-prose` and `fix-round-4-5b2-b-prose` files moved into the test file named after the source they test, each seen failing under a mutation of that source; the round files, the eight round end-to-end scripts and `tests/synthetic/plan-run-C.md` (now `tests/fixtures/plan-run-c.md`) are gone. `timing-under-load.test.mjs` is `timing.test.mjs`; `lib-imports.test.mjs` holds the import checks. The suite goes from 3336 to 2927 tests: duplicates and the tests of removed files left with them.
|
||||
- **The live tests left the round files** (cleanup R-a, R8). The tests of live behaviour in the `retterunde*`, `trial*`, `fix-round-4-5a-prose` and `fix-round-4-5b2-b-prose` files moved into the test file named after the source they test, each seen failing under a mutation of that source; the round files, the eight round end-to-end scripts and `tests/synthetic/plan-run-C.md` (now `tests/fixtures/plan-run-c.md`) are gone. `timing-under-load.test.mjs` is `timing.test.mjs`; `lib-imports.test.mjs` holds the import checks. The suite goes from 3336 to 2930 tests: duplicates and the tests of removed files left with them.
|
||||
|
||||
### Fixed
|
||||
|
||||
|
|
@ -225,11 +225,13 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
|
|||
|
||||
### Known issues
|
||||
|
||||
- **Some files wait for R-b** (cleanup R-a, decision 205). `docs/T1-cc26-delegated-orchestration.md` and `lib/review/gold-scorer.mjs` with its test stay: `scripts/end-state-registry.json`, whose hash is frozen, names them (D-06 probes the scorer). `scripts/synthesis-measure.mjs` and its test stay for R-b's measurement tooling (R1), although their default paths name the removed synthesis fixtures and results doc. `tests/fixtures/bakeoff-rich/` stays for V3 (decision 202); its README still describes the removed bakeoff harness as current.
|
||||
- **Some files wait for R-b** (cleanup R-a, decision 205). `docs/T1-cc26-delegated-orchestration.md` and `lib/review/gold-scorer.mjs` with its test stay: `scripts/end-state-registry.json`, whose hash is frozen, names them (D-06 probes the scorer). `tests/lib/gold-eval.test.mjs` and `tests/fixtures/bakeoff-rich/runs/run-perfect.json` stay with the scorer: they are the one end-to-end check that the review contract keeps every seeded finding. `scripts/comparison-gate.mjs` still lists the removed `settings.json` and `verify.sh` in a part's scope, and the profile calibration notes under `tests/synthetic/` still name the removed `examples/01-add-verbose-flag/brief.md`; both are R-b files. `scripts/synthesis-measure.mjs` and its test stay for R-b's measurement tooling (R1), although their default paths name the removed synthesis fixtures and results doc. `tests/fixtures/bakeoff-rich/` stays for V3 (decision 202); its README still describes the removed bakeoff harness as current.
|
||||
- **Part A's end-to-end script no longer runs** (cleanup R-a). `tests/e2e-fix-round-4-5b2-a.sh` stays, but its callers were the removed round scripts, and `npm test` runs only `*.test.mjs`; it is listed for an upgrade (A1).
|
||||
- **The path check has blind spots** (cleanup R-a). `tests/lib/living-paths.test.mjs` skips a line with an `@<sha>` or `MIT` on it or on the two lines above, so a live path next to a credit is not checked, and it reads a backticked span only when the whole span is a path, so `lib/x.mjs --flag` is not checked unless it carries `${CLAUDE_PLUGIN_ROOT}/`.
|
||||
- **The path check has blind spots** (cleanup R-a). `tests/lib/living-paths.test.mjs` skips a line with an `@<sha>` or `MIT` on it or on the two lines above, so a live path next to a credit is not checked, and it reads a backticked span only when the whole span is a path, so `lib/x.mjs --flag` is not checked unless it carries `${CLAUDE_PLUGIN_ROOT}/`. It reads only `.md` files, so a dead path in code (a script's default input, a comment) is not checked either.
|
||||
- **A removed flag is ignored, not refused** (cleanup R-a). The arg parser puts an unknown flag in `unknown` and the commands do not report it, so `/trekreview --workflow` now runs the standard review without a word.
|
||||
- **`/trekcontinue` Phase 1.5 chooses a prompt file nothing reads** (cleanup R-a, decided in its plan). With the validator gone it prefers the project folder's `NEXT-SESSION-PROMPT.local.md` over a root one and says so in one line; no later phase reads the file, and the producer and age checks are gone.
|
||||
- **A plan's `profile:` field has no writer.** README, `docs/`, `/trekexecute` and `/trekcontinue` say a plan carries or passes on a profile, but no command writes `profile:` to a plan; the plan validator's `MANIFEST_PROFILE_DRIFT` check reads it, and `resolveTrekcontinueProfile` has no caller outside its tests. Left whole for a later hardening part, which chooses between writing the field and taking the promises out.
|
||||
- **Splices and dropped `--x=` options outside fix round 4.5b** (decision 82). The commands' bash blocks still splice `{placeholder}` values into shell lines outside the four places the round closed (the Phase 0 parser lines of `/trekendsession`, `/trekcontinue` and `/trekreview`, and `/trekendsession` Phase 3), for example `"{brief_path}"` in `/trekplan` and the unquoted `{project_dir}` in `/trekendsession` Phase 4; and the release check, the lessons apply (`lib/review/recurrence.mjs`), the next-session validator and the park tool still drop an unknown `--x=` option without a word. One open finding, for fix round 4.5c or 4.5d.
|
||||
- **Splices and dropped `--x=` options outside fix round 4.5b** (decision 82). The commands' bash blocks still splice `{placeholder}` values into shell lines outside the four places the round closed (the Phase 0 parser lines of `/trekendsession`, `/trekcontinue` and `/trekreview`, and `/trekendsession` Phase 3), for example `"{brief_path}"` in `/trekplan` and the unquoted `{project_dir}` in `/trekendsession` Phase 4; and the release check, the lessons apply (`lib/review/recurrence.mjs`) and the park tool still drop an unknown `--x=` option without a word. One open finding, for fix round 4.5c or 4.5d.
|
||||
- **Two 4.5b findings moved to fix round 4.5c.** M12 (`lib/review/reviewed-commit.mjs` makes four git calls per path per commit; at most two paths per commit reach that loop) goes with 4.5c's single `git cat-file --batch` read, and Dc (the trekcontinue `line_ref` row in `tests/fixtures/jsonl-schemas.md`) with 4.5c's change of `line_ref` to a heading anchor.
|
||||
- **Step 9's narrowed unborn-HEAD rule is not recorded as a plan deviation** (MAJOR `c76a0bb8`, `lib/util/park-attempt-files.mjs`; decision 88). The park tool reads HEAD as unborn only when `headIsUnborn` agrees, which narrows plan Step 9's rule; the narrowing was logged as an executor decision, not as a plan amendment.
|
||||
- **The arg parser's stdin-unreadable branch has no test** (MAJOR `b0fd0a04`, `lib/parsers/arg-parser.mjs`; decision 88). No test exercises `ARG_STDIN_UNREADABLE` and its exit 1, the path that makes `/trekcontinue` and `/trekendsession` stop when stdin cannot be read.
|
||||
|
|
|
|||
17
tests/fixtures/bakeoff-rich/runs/run-perfect.json
vendored
Normal file
17
tests/fixtures/bakeoff-rich/runs/run-perfect.json
vendored
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
[
|
||||
{
|
||||
"reviewer": "brief-conformance-reviewer",
|
||||
"findings": [
|
||||
{ "severity": "BLOCKER", "rule_key": "UNIMPLEMENTED_CRITERION", "file": "lib/handlers/login.mjs", "line": 17 },
|
||||
{ "severity": "MAJOR", "rule_key": "PLAN_EXECUTE_DRIFT", "file": "lib/handlers/login.mjs", "line": 13 }
|
||||
]
|
||||
},
|
||||
{
|
||||
"reviewer": "code-correctness-reviewer",
|
||||
"findings": [
|
||||
{ "severity": "BLOCKER", "rule_key": "SECURITY_INJECTION", "file": "lib/auth/jwt.mjs", "line": 19 },
|
||||
{ "severity": "MAJOR", "rule_key": "MISSING_TEST", "file": "lib/auth/refresh.mjs", "line": 0 },
|
||||
{ "severity": "MINOR", "rule_key": "MISSING_ERROR_HANDLING", "file": "lib/auth/refresh.mjs", "line": 10 }
|
||||
]
|
||||
}
|
||||
]
|
||||
2
tests/fixtures/jsonl-schemas.md
vendored
2
tests/fixtures/jsonl-schemas.md
vendored
|
|
@ -26,7 +26,7 @@
|
|||
| trekexecute-stats (autonomy events) | ts, event, known_event, payload | lib/stats/event-emit.mjs `emit()` | event-emit.mjs:78-99 | payload.profile, payload.phase_models, payload.profile_source | none |
|
||||
| trekexecute-stats (PostToolUse Bash) | ts, session_id, command_excerpt, duration_ms, success | hooks/scripts/post-bash-stats.mjs (Bash PostToolUse) | post-bash-stats.mjs:42-54 | none (hook is plugin-level, not profile-aware) | command_excerpt (CWE-212) |
|
||||
| trekreview-stats | ts, slug, verdict, counts (BLOCKER/MAJOR/MINOR/SUGGESTION), reviewed_files_count, mode, duration_ms | lib/stats/stats-append.mjs (trekreview.md Phase 8) | trekreview.md:682-735 | profile, phase_models, profile_source | none |
|
||||
| trekcontinue-stats | ts, project, next_session_label, status | lib/stats/stats-append.mjs (trekcontinue.md Phase 5) | trekcontinue.md:304-308 | profile, profile_source | none |
|
||||
| trekcontinue-stats | ts, project, next_session_label, status | lib/stats/stats-append.mjs (trekcontinue.md Phase 5) | trekcontinue.md:271-275 | profile, profile_source | none |
|
||||
| token-usage | ts, session_id, scope, model, tokens_input, tokens_output, tokens_cache_creation, tokens_cache_read, cost_usd, is_estimate, price_table_version | lib/stats/token-usage.mjs `captureTokenUsage` (via hooks/scripts/otel-export.mjs Stop hook, opt-in `VOYAGE_TOKEN_METER`) | token-usage.mjs:buildRecord | n/a (SKAL-2 schema) | session_id (stripped at export) |
|
||||
|
||||
The `main-merge-declined` event's `payload.reason` (the autonomy-events row) is one of the literal words of
|
||||
|
|
|
|||
49
tests/lib/gold-eval.test.mjs
Normal file
49
tests/lib/gold-eval.test.mjs
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
// tests/lib/gold-eval.test.mjs
|
||||
// SKAL-1·4b — the offline gold-scored output eval (the scoring RUN).
|
||||
//
|
||||
// This is the third test-census category (see lib/util/test-census.mjs):
|
||||
// neither a behavior unit test nor a doc-consistency pin, but a SCORING RUN —
|
||||
// it feeds a committed agent-run fixture through the deterministic coordinator
|
||||
// contract (4a) and scores the result against the golden corpus at
|
||||
// (file, rule_key) granularity. Offline: committed reviewer payloads, no live
|
||||
// agent spawn, no LLM, no network. The all-agree foundation; the
|
||||
// LLM-in-the-loop eval is the separate 4c tier.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { runContract } from '../../lib/review/coordinator-contract.mjs';
|
||||
import { scoreFindings, scoreVerdict } from '../../lib/review/gold-scorer.mjs';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = join(HERE, '..', '..');
|
||||
const gold = JSON.parse(readFileSync(join(ROOT, 'tests/fixtures/bakeoff-rich/gold.json'), 'utf-8'));
|
||||
const runPerfect = JSON.parse(
|
||||
readFileSync(join(ROOT, 'tests/fixtures/bakeoff-rich/runs/run-perfect.json'), 'utf-8'),
|
||||
);
|
||||
|
||||
test('gold-eval — run-perfect reproduces gold at (file,rule_key): precision/recall/f1 = 1.0', () => {
|
||||
const result = runContract(runPerfect);
|
||||
const score = scoreFindings(result.findings, gold.findings);
|
||||
assert.equal(score.precision, 1, `precision ${score.precision}; spurious=${JSON.stringify(score.spurious)}`);
|
||||
assert.equal(score.recall, 1, `recall ${score.recall}; missed=${JSON.stringify(score.missed)}`);
|
||||
assert.equal(score.f1, 1);
|
||||
assert.equal(score.tp, gold.findings.length);
|
||||
});
|
||||
|
||||
test('gold-eval — run-perfect coordinator verdict matches gold expected_verdict (BLOCK)', () => {
|
||||
const result = runContract(runPerfect);
|
||||
assert.equal(result.verdict, gold.expected_verdict);
|
||||
assert.ok(scoreVerdict(result.verdict, gold.expected_verdict));
|
||||
});
|
||||
|
||||
test('gold-eval — run-perfect drops nothing (no suppressed, no schema-skipped payloads)', () => {
|
||||
// A clean run is the regression guard: any future contract change that
|
||||
// silently suppresses or skips one of the 5 seeded findings breaks here.
|
||||
const result = runContract(runPerfect);
|
||||
assert.equal(result.skipped.length, 0, `skipped payloads: ${JSON.stringify(result.skipped)}`);
|
||||
assert.equal(result.suppressed.length, 0, `suppressed findings: ${result.suppressed.length}`);
|
||||
assert.equal(result.findings.length, gold.findings.length);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue