fix(cleanup): R-b review round — retired flags in the prose parsers
/code-review high on main..HEAD: the four commands that parse their own arguments in prose (/trekbrief, /trekplan, /trekresearch, /trekexecute) lost their --profile handling, and /trekresearch its --engine, so a typed value became the path or the question. Each now strips a retired flag with its value; the arg parser retires --profile only for the commands that had it (not /trekendsession) and --engine for /trekresearch, seen red first. Stale lines R-b left: architecture's hook row (9 scripts) and bridge agent, the gap row's removed event ledger, README's contrarian marker, and a dangling "also" in /trekexecute Phase 8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
fdb401d0ab
commit
b70c37f0e7
11 changed files with 41 additions and 18 deletions
|
|
@ -60,7 +60,7 @@ Part B makes a WARN open the merge lock and the release check only when the oper
|
|||
- **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.
|
||||
- **One fixed path per command** (cleanup R-b, decision 209). With the phase signals gone, each command takes the heavier of its old paths. `/trekresearch` launches every agent its scope allows, and `contrarian-researcher` runs as wave 2 whenever the scope includes external. `/trekreview` always runs all four coordinator passes: a finding with no citation is dropped, and an unknown `rule_key` goes to `unverified` (the old high path's normalization to `PLAN_EXECUTE_DRIFT` is gone). `/trekplan` no longer writes the `## Adversarial Pass 2 (v5.1.1 high-effort)` section that only said the pass was skipped. The low-effort shortcuts are gone; the `--quick` modes stay. This also ends the earlier entry's "spawn sites follow the profile": a Voyage agent is spawned with no model parameter, so its own `model: opus` applies, and the two spawns of Claude Code's built-in agents (`/trekplan`'s Explore deep-dive, `/trekresearch`'s Phase 5 follow-up) pass `model: "opus"`.
|
||||
- **The brief carries no phase signals** (cleanup R-b, decision 191; Handover 1). `/trekbrief` no longer asks for per-phase effort and model, the template has no `phase_signals` block, and the brief validator's signal codes (`BRIEF_V51_MISSING_SIGNALS`, `BRIEF_SIGNALS_MUTUALLY_EXCLUSIVE`, `BRIEF_INVALID_PHASE_SIGNALS` and the phase, effort and model codes) are gone. An older brief's `phase_signals` key is ignored, so every existing brief stays valid. A consumer that read the signals from a brief gets none from a new one.
|
||||
- **`--engine` is gone and `--profile` is retired** (cleanup R-b). `/trekresearch --engine` no longer exists. `--profile` is still read as a flag with a value, so a typed value is not taken as a path, and nothing uses it (see Known issues).
|
||||
- **`--engine` and `--profile` are retired** (cleanup R-b). Nothing uses them. A typed `--profile` (and `/trekresearch --engine`) is dropped together with the word after it when that word is not a flag, so the value is not taken as a path or as part of the question: the six commands that had the flag say so where they parse their arguments, and the arg parser does the same (see Known issues).
|
||||
- **`intent-approval.mjs --stamp` prints `stamped`, `intent_hash` and `approved_at`** (cleanup R-b). The `record` field and the `brief-approved` stats record are gone; the stamp writes nothing outside the brief.
|
||||
|
||||
### Fixed
|
||||
|
|
@ -227,7 +227,7 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
|
|||
- **Arm B and `--workflow`** (R6, X10): `scripts/trekreview-armB.workflow.mjs` and the `/trekreview --workflow` flag; the standard path is the only review path.
|
||||
- **The fix-round chain** (R7): the round red scripts, regression scripts, mutation files and round prose and script tests. `tests/helpers/red-round.sh`, `red-judge.mjs` and `red-mutate.mjs` stay (K2), as do the 4.5b2 part A corpus and end-to-end script.
|
||||
|
||||
**Cleanup R-b (decisions 175, 189-191, 209).** Voyage's own measurement, the STORM loop and the profiles went, in three commits on one branch; 109 files and about 19 000 lines. `npm test` went from 2930 tests to 2309, 0 failing. `tests/lib/removed-surfaces.test.mjs` fails when a tracked file names a removed surface again (seen red in all three groups before the removal).
|
||||
**Cleanup R-b (decisions 175, 189-191, 209).** Voyage's own measurement, the STORM loop and the profiles went, in three commits on one branch; 109 files and about 19 000 lines. `npm test` went from 2930 tests to 2308, 0 failing. `tests/lib/removed-surfaces.test.mjs` fails when a tracked file names a removed surface again (seen red in all three groups before the removal).
|
||||
|
||||
- **Measurement, statistics and export** (R1, decision 189). `lib/stats/`, `lib/exporters/`, the `post-bash-stats` and `otel-export` hooks, the per-command stats phases (`/trekbrief` Phase 7, `/trekplan` Phase 12, `/trekexecute` Phase 9, `/trekcontinue` Phase 5, and the stats half of `/trekresearch` and `/trekreview` Phase 8), the events `/trekexecute` Phase 8 wrote around the merge (the merge lock never read them), the yardstick, the end-state gate and registry, the comparison gate, `docs/observability.md` and `examples/observability/`. Claude Code's own OpenTelemetry (`CLAUDE_CODE_ENABLE_TELEMETRY`) gives tool durations, tokens and cost per agent and skill; before the removal it was seen working against a local receiver with Claude Code 2.1.319: the command name, Bash durations, and model, effort and tokens per helper (condition 175).
|
||||
- **The STORM loop and the research engine choice** (R2, decision 190). `/trekresearch` Phase 4.5 and the Phase 5 follow-up loop (Phase 5 keeps its single pass, at most two follow-ups, every run), the `--engine` flag with the deep-research adapter and its self-check, the `pre-agent-cap` hook, `lib/util/research-loop-cap.mjs`, `scripts/storm-measure.mjs` and the engine and STORM docs.
|
||||
|
|
@ -239,7 +239,7 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
|
|||
- **`tests/fixtures/bakeoff-rich/` has no reader** (cleanup R-b). It stays for V3 (decision 202); with `gold-eval` gone nothing reads `gold.json`, and its README still describes the removed bakeoff harness as current.
|
||||
- **The query guard has no caller** (cleanup R-b). `lib/validators/query-privacy-gate.mjs` and its tests stay, but its callers were the removed STORM steps, so no research query passes through it; its header and `skills/researching-sources/` say so. Left for the design (187).
|
||||
- **Eleven agents take the session's effort** (cleanup R-b). Nine agents pin `effort: high`; the other eleven inherit the session level, so a session started below high runs them below high. Left for the design (187).
|
||||
- **A typed `--profile` is ignored without a word** (cleanup R-b). The parser keeps `--profile` as a retired valued flag, so `--profile x` consumes `x` instead of reading it as a path, and no command says the flag does nothing.
|
||||
- **A retired flag is dropped without a word** (cleanup R-b). A typed `--profile x` (or `/trekresearch --engine x`) is stripped with its value, and no command says the flag does nothing.
|
||||
- **The plan gate still leaves out a Pass 2 heading** (cleanup R-b). `lib/validators/plan-gate.mjs` keeps its exclusion of the "Adversarial Pass 2 (v5.1.1 high-effort)" heading that `/trekplan`'s high-effort path wrote, so older plans hash the same; no command writes the marker now.
|
||||
- **The lessons apply still skips `original_rule_key`** (cleanup R-b). `lib/review/recurrence.mjs` skips a finding's `original_rule_key`, which only the removed high-effort normalization wrote; harmless, left.
|
||||
- **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).
|
||||
|
|
|
|||
|
|
@ -770,7 +770,7 @@ flowchart TB
|
|||
end
|
||||
subgraph RES["/trekresearch · Phase 4 — parallel"]
|
||||
RL["LOCAL: architecture-mapper · dependency-tracer<br/>task-finder · git-historian · convention-scanner"]
|
||||
RE["EXTERNAL: docs-researcher · community-researcher<br/>security-researcher · contrarian-researcher*"]
|
||||
RE["EXTERNAL: docs-researcher · community-researcher<br/>security-researcher · contrarian-researcher"]
|
||||
end
|
||||
subgraph PL["/trekplan"]
|
||||
PLG["Phase 4b · brief-reviewer (gate)"]
|
||||
|
|
@ -789,7 +789,7 @@ flowchart TB
|
|||
BR --> RES --> PL --> EX --> RV
|
||||
```
|
||||
|
||||
`* = conditional`: convention-scanner / test-strategist on medium+ codebases (50+ files); research-scout for unknown external tech; contrarian-researcher runs in wave 2 of every research run, after wave 1 returns, with the leading answer of wave 1's external findings to attack, always at `effort=high`; brief-conformance-reviewer skipped under `--quick`.
|
||||
`* = conditional`: convention-scanner on medium+ codebases (50+ files); research-scout for unknown external tech; brief-conformance-reviewer skipped under `--quick`. contrarian-researcher runs in wave 2 of every research run whose scope includes external, after wave 1 returns, with the leading answer of wave 1's external findings to attack, always at `effort=high`.
|
||||
|
||||
> **Which Claude Code primitive each phase uses — and the alternatives considered (Workflow substrate, delegated orchestrator)** → see [docs/architecture.md §Primitives per step](docs/architecture.md#primitives-per-step-decision-matrix).
|
||||
|
||||
|
|
|
|||
|
|
@ -30,7 +30,8 @@ foreground if the user opts in.
|
|||
|
||||
## Phase 1 — Parse mode and validate input
|
||||
|
||||
Parse `$ARGUMENTS`:
|
||||
Parse `$ARGUMENTS`. A retired flag is stripped first and ignored, together with the word after it
|
||||
when that word is not a flag: `--profile`.
|
||||
|
||||
1. If arguments start with `--quick`: set **mode = quick**. The interview
|
||||
proposes `small` as the starting size class (Phase 3 still says the class
|
||||
|
|
|
|||
|
|
@ -32,7 +32,8 @@ never start helpers.
|
|||
|
||||
## Phase 1 — Parse mode and validate input
|
||||
|
||||
Parse `$ARGUMENTS` for mode flags:
|
||||
Parse `$ARGUMENTS` for mode flags. A retired flag is stripped first and ignored, together with the word after it
|
||||
when that word is not a flag: `--profile`.
|
||||
|
||||
1. If arguments contain `--project <dir>`: extract the directory path.
|
||||
- Resolve `{dir}` (trim trailing slash).
|
||||
|
|
@ -2705,7 +2706,7 @@ On confirm:
|
|||
(the merge ran, or there was nothing to merge)
|
||||
|
||||
On the multi-session wave path (Phase 2.6), and only when `next-wave` said
|
||||
`none` (every wave merged) and Step 4 passed, also clear the plan's orders
|
||||
`none` (every wave merged) and Step 4 passed, clear the plan's orders
|
||||
directory — orders, archived progress copies and the merged-waves record — so a
|
||||
later plan with the same `{slug}` starts clean. This is the one place it is
|
||||
cleared:
|
||||
|
|
|
|||
|
|
@ -27,7 +27,8 @@ Pipeline position:
|
|||
|
||||
## Phase 1 — Parse mode and validate input
|
||||
|
||||
Parse `$ARGUMENTS` for mode flags. Order of precedence:
|
||||
Parse `$ARGUMENTS` for mode flags. A retired flag is stripped first and ignored, together with the word after it
|
||||
when that word is not a flag: `--profile`. Order of precedence:
|
||||
|
||||
1. **`--export headless <plan-path>`** — extract `{format}` (first token after
|
||||
`--export`) and `{plan-path}` (remainder). `headless` is the only export
|
||||
|
|
|
|||
|
|
@ -25,7 +25,8 @@ specialized agents, each seeing only what they need. The value is in triangulati
|
|||
|
||||
Parse `$ARGUMENTS` for mode flags. Flags can appear in any order before the
|
||||
research question. Collect all flags first, then treat the remainder as the
|
||||
research question.
|
||||
research question. A retired flag is stripped first and ignored, together with the word after it
|
||||
when that word is not a flag: `--profile` or `--engine`.
|
||||
|
||||
Supported flags:
|
||||
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ Doc-consistency test at `tests/lib/doc-consistency.test.mjs` pins agent-table co
|
|||
|
||||
**Brief:** 7-phase workflow: Parse mode → Create project dir → Phase 3 completeness loop (section-driven, no question cap) → Phase 4 draft/review/revise with `brief-reviewer` as stop-gate (max 3 iterations; gate = all dimensions ≥ 4 and research plan = 5) → Finalize (`brief.md` on pass, or `brief_quality: partial` on cap/force-stop) → Manual/auto opt-in. Always interactive. Auto mode runs research + plan inline in the main context (v2.4.0).
|
||||
|
||||
**Research:** Foreground workflow (v2.4.0): Parse mode → Interview → Parallel research swarm (5 local + 4 external + 1 bridge, spawned from main context) → Follow-ups → Triangulation → Synthesis + brief. With `--project`, writes to `{dir}/research/NN-slug.md`.
|
||||
**Research:** Foreground workflow (v2.4.0): Parse mode → Interview → Parallel research swarm (5 local + 4 external, spawned from main context) → Follow-ups → Triangulation → Synthesis + brief. With `--project`, writes to `{dir}/research/NN-slug.md`.
|
||||
|
||||
**Plan:** Foreground workflow (v2.4.0): Parse mode (validate brief input) → Codebase sizing → Brief review (`brief-reviewer`) → Parallel exploration (6-8 agents, spawned from main context) → Deep-dives → Synthesis (with architecture-note cross-reference if present) → Planning → Adversarial review (`plan-critic` + `scope-guardian`) → Present/refine → Handoff. With `--project`, writes to `{dir}/plan.md` and auto-detects `{dir}/architecture/overview.md` (produced by an opt-in upstream architect plugin if installed; not bundled).
|
||||
|
||||
|
|
@ -89,7 +89,7 @@ Which native Claude Code primitive each pipeline step runs on today, and the alt
|
|||
| **execute** | Inline step loop; multi-session via `git worktree` + session orders the user runs, wave by wave; deterministic manifest audit | CC `TaskCreate`/`TodoWrite` for progress/resume (insufficient — carries no step status / attempts / SHA / drift → own typed `progress.json` contract) |
|
||||
| **review** | Inline parallel reviewers (no cross-feed) → `review-coordinator` Judge | **Workflow** substrate for Phase 5–6 (bake-off POSITIVE: +4.4 % tokens / +54 % wall-time → shipped opt-in, since removed; wholesale substrate swap declined) |
|
||||
| **continue** | Inline reads `.session-state.local.json` → zero-confirm resume | CC `--resume` (transcript replay, not typed work-state → insufficient) |
|
||||
| **cross-cutting** | 5 hook scripts: `pre-bash` + `pre-write` guards, `session-title`, `pre-`/`post-compact` flush | — |
|
||||
| **cross-cutting** | 9 hook scripts: `pre-bash` + `pre-write` guards, `pre-plan-gate`, `session-title`, `post-approval-log`, `post-edit-check`, `pre-`/`post-compact` flush, `stop-test-lock` | — |
|
||||
|
||||
¹ MCP per research agent: `docs-researcher` → Microsoft Learn + Tavily · `community-`/`security-`/`contrarian-researcher` → Tavily (+ WebSearch/WebFetch). Graceful degradation when an MCP server is absent.
|
||||
|
||||
|
|
|
|||
|
|
@ -63,7 +63,7 @@ goes to it under MIT, and to the Anthropic article it is itself derived from.
|
|||
|
||||
| Candidate | Does Voyage already have it | Verdict |
|
||||
|---|---|---|
|
||||
| Hash-chained, tamper-evident approval ledger | No. No hash chain exists anywhere | **Partly.** The idea answers "did we build what we agreed", but a new ledger engine is out of scope; the cheap form is a `prev_hash` field on the event ledger that already exists |
|
||||
| Hash-chained, tamper-evident approval ledger | No. No hash chain exists anywhere | **Partly.** The idea answers "did we build what we agreed", but a new ledger engine is out of scope |
|
||||
| Plan as a machine-readable file list with a lock | Yes — `plan_version 1.7` manifest, `plan-validator.mjs --strict`, and the independent Phase 7.5 audit | **Borrow the boundary only:** their named process-file exclusion list, so a state or docs update does not need a manifest entry |
|
||||
| Short intent and spec templates | Partly — the brief already requires `## Intent`, `## Goal` and `## Success Criteria` | **No** to a separate spec artifact (the operator is both orderer and builder). **Yes** to their "Gotchas" section: one fixed place for where two constraints collided and who decided |
|
||||
| A rule → enforcement table ("advisory or enforced, and where") | No such table; the doc-pins are that idea applied narrowly | **Yes, cheap.** It makes visible which of Voyage's own rules are only prose |
|
||||
|
|
|
|||
|
|
@ -13,9 +13,15 @@
|
|||
import { readFileSync } from 'node:fs';
|
||||
import { isMain } from '../util/is-main.mjs';
|
||||
|
||||
// Flags retired with a value (cleanup R-b): the flag and the word after it,
|
||||
// when that word is not a flag, are consumed and dropped, never an error.
|
||||
const RETIRED_VALUED = ['--profile'];
|
||||
// Flags retired with a value (cleanup R-b), per command that had them: the
|
||||
// flag and the word after it, when that word is not a flag, are consumed and
|
||||
// dropped, never an error.
|
||||
const RETIRED_PROFILE = ['--profile'];
|
||||
const RETIRED_VALUED = {
|
||||
trekbrief: RETIRED_PROFILE, trekplan: RETIRED_PROFILE, trekexecute: RETIRED_PROFILE,
|
||||
trekreview: RETIRED_PROFILE, trekcontinue: RETIRED_PROFILE,
|
||||
trekresearch: [...RETIRED_PROFILE, '--engine'],
|
||||
};
|
||||
|
||||
const FLAG_SCHEMA = {
|
||||
trekbrief: {
|
||||
|
|
@ -115,7 +121,7 @@ export function parseArgs(argString, command) {
|
|||
continue;
|
||||
}
|
||||
|
||||
if (RETIRED_VALUED.includes(tok)) {
|
||||
if ((RETIRED_VALUED[command] || []).includes(tok)) {
|
||||
if (i + 1 < tokens.length && !tokens[i + 1].startsWith('--')) i++;
|
||||
continue;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -289,4 +289,8 @@ test('retired --profile swallows its value: --profile economy leaves positional
|
|||
const bare = parseArgs('--project x --since HEAD~1 --profile', 'trekreview');
|
||||
assert.deepEqual(bare.errors, [], `a bare retired flag is no error: ${JSON.stringify(bare)}`);
|
||||
assert.equal(parseArgs('--profile --project x', 'trekreview').flags['--project'], 'x', 'the retired flag never takes a flag as its value');
|
||||
const engine = parseArgs('--engine deep-research What is best?', 'trekresearch');
|
||||
assert.deepEqual([engine.positional, engine.unknown], [['What', 'is', 'best?'], []], `trekresearch retires --engine too: ${JSON.stringify(engine)}`);
|
||||
const end = parseArgs('--profile next-brief.md', 'trekendsession');
|
||||
assert.deepEqual([end.positional, end.unknown], [['next-brief.md'], ['--profile']], `a command that never had the flag keeps reporting it: ${JSON.stringify(end)}`);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -52,11 +52,20 @@ const EXEMPT = new Map([
|
|||
['docs/proevesett.md:139', 'dated v1/v2 measurement record of the fixed test set, kept unedited'],
|
||||
['docs/proevesett.md:143', 'dated v1/v2 measurement record of the fixed test set, kept unedited'],
|
||||
['docs/proevesett.md:146', 'dated v1/v2 measurement record of the fixed test set, kept unedited'],
|
||||
['lib/parsers/arg-parser.mjs:18', 'retired flag swallows its value'],
|
||||
['commands/trekbrief.md:34', 'a command strips the retired flag where it parses its arguments'],
|
||||
['commands/trekexecute.md:36', 'a command strips the retired flag where it parses its arguments'],
|
||||
['commands/trekplan.md:31', 'a command strips the retired flag where it parses its arguments'],
|
||||
['commands/trekresearch.md:29', 'a command strips the retired flag where it parses its arguments'],
|
||||
['lib/parsers/arg-parser.mjs:19', 'retired flag swallows its value'],
|
||||
['lib/parsers/arg-parser.mjs:23', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:285', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:286', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:289', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:291', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:292', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:293', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:294', 'retired flag swallows its value'],
|
||||
['tests/lib/arg-parser.test.mjs:295', 'retired flag swallows its value'],
|
||||
['tests/validators/brief-validator.test.mjs:200', 'older brief stays valid (decision 191)'],
|
||||
['tests/validators/brief-validator.test.mjs:203', 'older brief stays valid (decision 191)'],
|
||||
['tests/validators/brief-validator.test.mjs:205', 'older brief stays valid (decision 191)'],
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue