From 0696652d54c40c2ac1142313eecfd58b0568f78c Mon Sep 17 00:00:00 2001 From: kami Date: Sun, 14 Jun 2026 23:52:53 +0400 Subject: [PATCH] =?UTF-8?q?docs(backlog):=20static-first=20reviewer=20ship?= =?UTF-8?q?ped=20+=20QA=20plan;=20supersede=20=C2=A7C=20A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - §B §5 reviewer static-first infra → SHIPPED (pending live-QA) 365eb8a; add §F gate + docs/qa/QA-static-first-reviewer.md. - §C A2 stage-level plan checkpointing → SUPERSEDED (verifyProduces + ManifestContainment + validation already reconcile execution vs the compiled plan); A3 consolidated under §B §6 calibration (build once, two consumers). --- BACKLOG.md | 38 ++++++++++++---- docs/qa/QA-static-first-reviewer.md | 68 +++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 docs/qa/QA-static-first-reviewer.md diff --git a/BACKLOG.md b/BACKLOG.md index 2be2f006..d6c991a5 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -47,9 +47,15 @@ f8fd260) — those are RETRO candidates once live-QA'd. Remaining: §3 analyst keystone (894969d), entity grounding (b050dc4), §2 write-manifest (4e5e4e5), and §5 reviewer prompt/needs wiring (3467826) are shipped — RETRO candidates. Remaining: -- [ ] **§5 reviewer static-first INFRA** — run compiler/detekt/formatters as a pipeline - step and *mechanically exclude their findings* from reviewer context. Needs a - static-findings event representation. (Prompt half is done; infra half is unbuilt.) +- [~] **§5 reviewer static-first INFRA** — **SHIPPED (pending live-QA)** `365eb8a`. A stage declares + `static_analysis = [commands]`; after it produces its artifact a deterministic harness gate + (`runStaticAnalysis`) runs each command (compiler/detekt/formatters) in the workspace root, + records a `StaticAnalysisCompletedEvent` (invariant #9; recorded live, folded on replay), and on + any non-clean command fails the stage retryably with the output fed back verbatim (§2). Only + static-clean output reaches the reviewer, so its context excludes static findings *mechanically* + (resolved upstream) — no output-parsing, no shared-findings schema needed. `ProcessStaticAnalysis- + Runner` is the injected adapter (nullable seam; null → logged no-op). Prompt half was `3467826`. + QA gate in §F → `docs/qa/QA-static-first-reviewer.md`. - [ ] **§6 `CritiqueFinding` schema in code** — only test expectations exist today; no structured type. Shared by plan critic + code reviewer. - [ ] **§6 `CriticCalibrationProjection`** (keyed by modelHash, role) — needs runtime @@ -66,7 +72,8 @@ and §5 reviewer prompt/needs wiring (3467826) are shipped — RETRO candidates. ## C. Plan pipeline addenda — `docs/plans/2026-06-13-plan-pipeline-addenda.md` -A1 is shipped (bcc59d2) — RETRO candidate once live-QA'd. A2/A3 remain. +A1 is shipped (bcc59d2) — RETRO candidate once live-QA'd. A2 is **superseded** (see below); +A3 is the same item as §B §6 calibration (tracked there). - [~] **A1 brief echo-back gate** — **SHIPPED (pending live-QA)** `bcc59d2`. New `brief_echo` stage before the planner in `role_pipeline`: the model restates the analyst brief as a structured @@ -75,11 +82,19 @@ A1 is shipped (bcc59d2) — RETRO candidate once live-QA'd. A2/A3 remain. `BriefEchoMismatchEvent` + retryable stage failure, so a misread brief never reaches plan generation. Pure replay-safe diff (`BriefEchoDiff`); symbols recorded but non-blocking in v1. QA gate in §F → `docs/qa/QA-brief-echo-gate.md`. `freestyle_planning` wiring is a follow-up. -- [ ] **A2 stage-level plan checkpointing** — reconciliation compares produced artifacts vs - the confirmed plan's produces-slots per stage; `StageCheckpointPassedEvent` / - `StageCheckpointFailedEvent` (halt + surface). -- [ ] **A3 critique calibration tracking** — `CritiqueOutcomeCorrelatedEvent` + - `CriticCalibrationProjection` (= role-reliability §6; build once, two consumers). +- 🚫 **A2 stage-level plan checkpointing** — **SUPERSEDED, do not build.** The spec predates the + machinery that now carries this weight. In the current design the "confirmed plan" is the + freestyle `execution_plan` → `ExecutionPlanCompiler` → `WorkflowGraph`, and per-stage + reconciliation against the plan's produces-slots *already happens* in phase-2 execution: + `verifyProduces` checks every `stage.produces` slot (sourced from the plan JSON) against + `ArtifactCreatedEvent`s and fails retryably → surfaces on exhaustion; `ManifestContainmentRule` + blocks out-of-plan writes; the validation pipeline checks content vs kind schema. The only net-new + bits A2 adds are a positive `StageCheckpointPassedEvent` + terminal-vs-retryable halt — and the + sole consumer of the former is A3 calibration, which is itself ordering-blocked. Revisit only if a + real consumer for a positive per-stage checkpoint event appears. (Assessed 2026-06-14.) +- [ ] **A3 critique calibration tracking** — **= §B §6 `CriticCalibrationProjection`** (build once, two + consumers). Tracked under §B §6; needs runtime critique/outcome history first (spec ordering #5). + Not duplicated here. ## D. Research workflow spec — `docs/plans/2026-06-13-research-workflow-spec.md` @@ -143,6 +158,11 @@ These are SHIPPED in code but prompt-/server-/network-dependent and not yet live (schema/prompt/artifacts block). Plan: `docs/qa/QA-brief-echo-gate.md` (faithful echo→planner runs, dropped requirement→`BriefEchoMismatchEvent`+retryable fail, hallucinated file flagged, symbols non-blocking, replay recomputes identically, non-opted pipeline is a no-op). +- [ ] **Static-first reviewer gate live-QA** — `365eb8a`; needs a real implementer-capable model + a + workspace whose configured `static_analysis` commands can be made to pass/fail on demand (and the + gate uncommented in the copied `role_pipeline.toml`). Plan: `docs/qa/QA-static-first-reviewer.md` + (clean→reviewer runs, non-clean→`StaticAnalysisCompleted`+retryable fail w/ output fed back, + gate-OFF absence check, no-runner no-op WARN, replay reads recorded findings without re-running). - [ ] **TUI kernel-steering + AMD gauge fresh live-QA** — approve+note actually revising the *same* stage's output; AMD VRAM/RAM gauge on the real box. - [ ] **Unit-tested-only fixes, not live-verified:** `1a7eb05` (verdict-edge rejection), diff --git a/docs/qa/QA-static-first-reviewer.md b/docs/qa/QA-static-first-reviewer.md new file mode 100644 index 00000000..6fc4553e --- /dev/null +++ b/docs/qa/QA-static-first-reviewer.md @@ -0,0 +1,68 @@ +# QA Plan: static-first reviewer gate — 365eb8a + +Role-reliability §5 (static-first ordering, infra half). Drafted from the diff (commit `365eb8a`) +per the BACKLOG QA rule. The reviewer prompt half shipped earlier in `3467826`. + +**Status:** DRAFT +**Run date / operator:** +**BACKLOG item:** §B "§5 reviewer static-first INFRA". + +--- + +## Preconditions + +- [ ] **server build/branch:** master @ `365eb8a` (or later) — _rebuild only with the server stopped_. +- [ ] **llama-server + model:** a real implementer-capable model (the static gate runs on the + *implementer* stage's output; exercising the retry/feedback path needs a model that actually + writes files and can react to fed-back tool output). Stub providers won't drive the loop. +- [ ] **external deps:** none (no network) — but the **configured commands must be runnable** in the + workspace (e.g. a Kotlin/Gradle repo for `./gradlew compileKotlin`). Use a workspace where the + command both exists and can be made to pass *and* fail on demand. +- [ ] **config synced:** copy the updated `examples/workflows/role_pipeline.toml` into + `~/.config/correx/workflows/` and **uncomment / set** `static_analysis = [...]` on the + `implementer` stage with commands valid for the test workspace. (Commented by default — + a fresh copy runs with the gate OFF.) +- [ ] **fixtures/seed:** a workspace (`StartSession` workingDir) where the implementer will write code, + and where you can arrange a deliberately-failing static check (e.g. a detekt violation / compile error). + +## Acceptance gate (one sentence) + +> The gate is correct **iff** a stage that declares `static_analysis` runs every configured command +> against its produced output, records a `StaticAnalysisCompletedEvent` with each command's exit code, +> lets the run proceed to the reviewer only when all commands are clean, and on any non-clean command +> fails the stage *retryably* (feeding the output back) so the reviewer never runs on broken code — +> while a stage that declares no commands behaves exactly as before. + +## Checks + +| # | Action | Expected observable evidence | Result | +|---|--------|------------------------------|--------| +| 1 | **Serialization registration trap.** `grep StaticAnalysisCompleted core/events/.../serialization/Serialization.kt` | `subclass(StaticAnalysisCompletedEvent::class)` present — without it the event deserializes silently wrong despite passing unit tests | | +| 2 | Run `role_pipeline` (gate ON) where the implementer's output passes all configured commands | `correx events ` shows a `StaticAnalysisCompleted` event after the implementer stage with every finding `exitCode = 0`; flow transitions implementer → reviewer; a `review_report` is produced | | +| 3 | Inspect that `StaticAnalysisCompleted` event | `findings[]` lists each configured command with its exit code and a non-empty `summary` (output tail); the command strings match the TOML `static_analysis` list | | +| 4 | Arrange the implementer's output to FAIL one command (e.g. introduce a detekt/compile error in the written code) | `StaticAnalysisCompleted` with that command `exitCode != 0`; the implementer stage **fails retryably** (a `StageFailed`/retry, NOT a transition to reviewer); the reviewer has **not** produced a `review_report` for this attempt | | +| 5 | Let the retry run with the fed-back output | The next implementer attempt sees the failing command's output verbatim in its context; once it fixes the issue the next `StaticAnalysisCompleted` is all-clean and flow proceeds to the reviewer; retries bounded by `implementer.max_retries`, then surfaces to the operator | | +| 6 | **Absence check — gate OFF.** Run any workflow / a `role_pipeline` copy with `static_analysis` left commented | **No** `StaticAnalysisCompleted` event appears; the implementer→reviewer transition behaves exactly as before | | +| 7 | **No-runner / no-workspace no-op.** (If reproducible) run a stage declaring `static_analysis` but with no workspace root | Server log WARN "stage declares static_analysis but no runner/workspace root is wired — skipping"; the run proceeds (no block), and no `StaticAnalysisCompleted` event | | +| 8 | `correx replay ` on a session that recorded a mismatch (check 4) | The recorded `StaticAnalysisCompleted` findings are read back as-is; replay does **not** re-run the commands (no new process output, deterministic double-read digest) | | + +Evidence sources: `correx events ` (the `StaticAnalysisCompleted` chain + retry/stage events), +server logs (MDC sessionId, the skip WARN), the workspace files for the introduced/fixed defect, and +`correx replay ` for determinism. + +## Out of scope (explicitly NOT covered this pass) + +- Per-command timeout tuning (`ProcessStaticAnalysisRunner` default 5 min) — adjust only if a real + build legitimately exceeds it. +- Quoted/space-containing command arguments — commands are whitespace-split into argv (no shell); + keep configured commands simple. Revisit only if a real command needs shell semantics. +- Wiring the gate onto `freestyle` execution plans / any stage other than the role_pipeline implementer. +- Structured parsing of tool output into per-line findings, and the shared `CritiqueFinding` schema + (§6) — deliberately not built; the gate works on exit codes + raw output tails. + +## Disposition + +- **PASS** → MOVE the §B "§5 reviewer static-first INFRA" entry into `RETRO.md` (cite `365eb8a` + + `3467826` for the prompt half, run date, evidence). Set Status: PASSED. +- **FAIL** → file each failure as a numbered finding back into `BACKLOG.md` (action + exact repro + + the wrong/missing signal), fix, re-run only the failed checks. Set Status: FAILED until green.