Files
correx/qa/audit-report-2026-06-08.md
T
kami 89487db72a fix(kernel,router): rehydrate by CAS content hash (F-021) + narration budget
rehydrate() now scans ArtifactContentStoredEvent (the durable slot->CAS-hash
bridge) and fetches stored bytes by content hash, keying the cache by the
logical slot name. The slot name is never passed to the hash-keyed CAS, so the
odd-length-hex crash is gone and artifact content is restored on cold-start
resume. RehydrateTest reworked to use a content-addressed fake store (slot name
!= content hash) so it actually exercises the bug. Replay (#8) is unaffected —
rehydrate is live-resume-only.

Also bump router narration maxTokens 1024 -> 4096: reasoning models were
spending the whole budget thinking and emitting empty content (finishReason=
length), so narrations never reached the TUI.

Removes the stale TUI-refactor progress doc.
2026-06-09 10:14:04 +04:00

476 lines
34 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# correx QA / Audit Report — 2026-06-08
Live operator session (Phase 3). Provider: local llama-server via env-var fallback
(`llama-cpp:default` @ 127.0.0.1:10000). Target: `qa/sample-repo/` (tag `clean`).
## Run context / repro prompt
Workflow: `role_pipeline` (analyst→architect→planner→implementer⇄reviewer).
Target workspace (TUI `Hello` workingDir): `/home/kami/Programs/correx/qa/sample-repo`
(reset between runs with `git -C qa/sample-repo reset --hard clean`).
Provider: env-var fallback `llama-cpp:default` @ 127.0.0.1:10000 (start llama-server first).
Operator prompt used for every run so far:
```
Fix the failing test in this repository. Running `python3 -m pytest` shows
tests/test_calc.py::test_average_empty failing: calc.average([]) raises
ZeroDivisionError, but it should return 0.0 for an empty list. Diagnose the
cause in src/sampleapp/calc.py, implement the fix, and verify all tests pass.
```
Patches applied this session (rebuild + restart server to load):
- F-001: `LlamaCppInferenceProvider.kt` — drop grammar when tools present.
- F-005: `FileReadConfig`/`Main` workspace builder/`FileReadTool` — workingDir-relative
path resolution.
- F-007: validator content-map (drops CAS dep) + `ArtifactContentStoredEvent` slot→hash
recording. No server restart needed for validation correctness.
- F-008: `TomlWorkflowLoader` — propagate stage `token_budget``generationConfig.maxTokens`.
- F-009: `FileReadTool` file-not-found → `recoverable=true`; `SessionOrchestrator` feeds
recoverable tool errors back into the loop instead of failing the stage.
- F-010: `SessionOrchestrator` — reject premature `stage_complete` when an LLM-emitted slot
is unfilled; nudge the model to emit the artifact.
- F-011: artifact schemas (`analysis`/`design`/`impl_plan`) — list fields `string``array`.
Live config under `~/.config/correx/schemas/` + repo `docs/schemas/` synced.
- F-012: `SessionOrchestrator.stripCodeFence` — strip outer ```` ``` ```` fence from
LLM-emitted artifacts before hash/store/validate.
- F-013: `ValidationPipeline` — payload-validation failures retryable, structural ones not.
- F-014 + diff: `FileEditTool` — schema matches real params (`target`/`replacement`/`patch`);
successful edits emit a unified diff.
- F-015: `SessionOrchestrator` — record tool-execution events + materialise a real
`file_written` artifact from the on-disk write; gate validation so a no-write stage cannot
be rubber-stamped.
- F-016: `Main.kt``stageTimeoutMs` 60s → 180s (still a hardcode; should be config-wired).
- F-018: `SessionOrchestrator` — write-nudge: when a stage owes a `file_written` artifact but
the model ended a turn with content (or called `stage_complete`) without writing, push back a
corrective tool-result instructing it to invoke `file_edit`/`file_write` (bounded by
`MAX_TOOL_ROUNDS`). Extends the F-010 premature-completion guard to file-write stages.
Regression fix: `FileReadToolTest` updated for the F-009 `recoverable=true` change.
- F-019: `FileWrittenArtifact`/`FileWrittenKind` gain a `diff` field; `file_edit`+`file_write`
emit a unified diff (shared `DiffUtil.unifiedDiff`); `materializeFileWritten` threads it into the
artifact the reviewer reads, so the reviewer can verify the change.
- F-020: `FileEditTool` — model-correctable failures (bad patch, target-not-found, bad params,
unknown op) now `recoverable=true` (feed back, don't abort); tool/`operation` descriptions +
implementer prompt steer toward `replace`/`file_write` over the fragile `patch` op.
- F-021: FIXED — resume `rehydrate()` now consumes `ArtifactContentStoredEvent` (slot→hash)
and fetches CAS bytes by content hash instead of passing the slot name to the hash-keyed store.
**Key reproducibility note:** the live config the server reads is `~/.config/correx/`
(`configDir = ConfigLoader.configPath().parent`), NOT the repo. Code fixes load via repo
build jars on the classpath; all config/prompts/schemas come from `~/.config/correx/`. The
repo `docs/schemas/` were stale relative to live — a clean-checkout reproduction will differ.
## Findings
### F-001 — grammar + tools sent together → llama.cpp 400, every tool+artifact stage dies
- **invariant/contract:** functional (workflows run end-to-end) / #7 path (artifact constraint)
- **subsystem:** infrastructure/inference (llama_cpp)
- **severity:** blocker
- **evidence:** `logs/current.log` analyst stage: `llama-server returned 400 Bad Request:
"Cannot use custom grammar constraints with tools."`. Request body carries both
`"grammar": "root ::= ..."` and `"tools":[file_read, stage_complete]`.
- **repro:** `correx run role_pipeline` (any prompt) against sample-repo. Analyst declares
`allowed_tools=[file_read, ShellTool]` AND `produces=[analysis]`. Fails immediately.
- **root cause:** `LlamaCppInferenceProvider.kt:103-104` sets `grammar` and `tools`
unconditionally. llama.cpp's server forbids the combination. ANY stage that both has
tools and a JSON `responseFormat` is unrunnable on llama.cpp. role_pipeline analyst +
implementer both do → pipeline cannot start.
- **fix direction:** when tools are present, drop the GBNF grammar and rely on
post-hoc schema validation + retry (invariant #7 still holds — output is validated);
OR gate the artifact emission into a separate tool-free final inference turn.
- **status:** PATCHED (workaround) — `LlamaCppInferenceProvider.kt`: grammar suppressed
when tools are present; artifact now constrained by validation+retry only. Requires
server restart to take effect. Proper fix tracked below.
- **follow-up TODO (proper fix):** add a first-class artifact-emission tool — the LLM
calls `emit_artifact(path, content, description)` like it calls file_read/file_write,
making artifact production an explicit schema-checked tool call instead of a grammar
constraint that can't coexist with tools. TODO left in code at the patch site.
### F-002 — non-retryable 400 is retried to exhaustion
- **invariant/contract:** functional (retry accounting)
- **subsystem:** core/kernel (orchestration retry policy)
- **severity:** minor
- **evidence:** orchestrator marks the 400 `retryable=true`; retries 1/3, 2/3, 3/3 all
identical 400s within ~80ms, then `WorkflowFailedEvent retryExhausted=true`.
- **suspected cause:** retry classifier treats all provider failures as retryable. A 400
`invalid_request_error` is deterministic request-construction failure → should be terminal.
- **status:** confirmed
### F-003 — `~` not expanded in default_system prompt path
- **subsystem:** core/kernel (prompt loading) / core/config
- **severity:** minor
- **evidence:** `failed to load prompt '~/.config/correx/prompts/default_system.md':
Prompt not found (searched: .../prompts/~/.config/correx/prompts/default_system.md)` —
tilde concatenated literally onto the config dir. Logged ERROR on every attempt.
Non-fatal (run proceeds) but noisy + the default system prompt is silently absent.
- **status:** confirmed
### F-005 — file_read jails relative paths against process cwd, not the workspace
- **invariant/contract:** functional (tool jailing) / two-plane consistency (#9)
- **subsystem:** infrastructure/tools/filesystem (FileReadTool) + apps/server wiring
- **severity:** major (blocks any file_read of a relative path unless server launched
from inside the workspace)
- **evidence:** `logs/current.log` session c0eab7ba: analyst emitted
`file_read(path=tests/test_calc.py)`. Plane-2 PATH_CONTAINMENT PROCEEDed
(`resolved=.../qa/sample-repo/tests/test_calc.py, inWorkspace=true`), but the tool's own
`validateRequest` rejected it: `Path 'tests/test_calc.py' is not in the allowed list`
→ stage produced no artifact → surfaced as the misleading `no transition condition
matched from stage analyst`.
- **root cause:** `FileReadTool` resolved the relative path via `Paths.get(p).toAbsolutePath()`
= JVM process cwd (the correx repo root, where the server was launched), not the bound
workspace. `FileReadConfig` had no `workingDir` field at all, unlike FileWrite/FileEdit
which carry `workingDir` and call `resolvePath()`. So the two file-access planes anchored
relative paths to different roots.
- **status:** PATCHED — added `workingDir` to `FileReadConfig`, wired
`workspace.workingDir` in Main's per-workspace tool builder, and gave `FileReadTool` a
`resolvePath()` mirroring FileWriteTool. Requires server restart.
- **note:** the operator-facing failure reason (`no transition condition matched`) masked
the real cause (tool denial). See F-004 family — failure-reason surfacing is lossy.
### F-007 — ArtifactPayloadValidator conflates slot name with CAS content-hash → crash
- **invariant/contract:** functional (artifacts validate) / #7 (LLM output untrusted
until validated — the validation step itself is broken)
- **subsystem:** core/validation + core/artifacts-store contract vs infrastructure CAS
- **severity:** BLOCKER (every workflow with a typed `produces` slot crashes at the first
post-stage validation against the real CAS store)
- **evidence:** `logs/current.log` session 28c818c1: analyst COMPLETED
(responseArtifactId=82da21d5…, a real blake3 hash), then
`runSession ... failed: hex string must have even length` —
`CasArtifactStore.hexToBytes` at `CasArtifactStore.kt:102`, called from
`ArtifactPayloadValidator.validateSlot:36`.
- **root cause:** `ArtifactPayloadValidator.validateSlot` calls
`artifactStore.get(slot.name)`. `slot.name` is the LOGICAL slot id ("analysis"), but
`CasArtifactStore.get` requires `ArtifactId.value` to be a hex content-hash
(`id.value.hexToBytes()`). "analysis" → odd-length → `require` throws. The validator has
no slot-name→content-hash mapping: `ValidationContext` carries only the static graph +
`SessionState`, and `SessionState` holds NO produced-artifact map. So the validator
cannot locate the bytes it is supposed to validate.
- **deeper implication:** this path has apparently never been exercised end-to-end against
the real content-addressed CAS — it only "works" if the ArtifactStore is keyed by literal
slot name (an in-memory test double). The `ArtifactStore.get(ArtifactId)` contract is
ambiguous: the validator treats ArtifactId as a logical name, the CAS treats it as a hash.
- **fix direction (NOT a one-liner):** the run's produced-artifact mapping
(slot name → ArtifactId(hash)) must be derived from the event log (an ArtifactProduced-type
event projection per invariant #1) and threaded into `ValidationContext`, so the validator
resolves slot.name → stored hash before `get`. Cross-module change
(events → projection → context builder → validator). Audit-only: NOT patched.
- **status:** FIXED.
- **fix applied:** two changes (validation crash + durable mapping).
1. *Validator no longer touches the CAS.* `ValidationContext` gained
`producedArtifactContent: Map<String,String>` (slot.name.value → payload). The
orchestrator populates it from the in-memory `artifactContentCache` for every produced
slot across `graph.stages` (scoped to the session). `ArtifactPayloadValidator` reads
from that map and **dropped its `ArtifactStore` dependency entirely** — the slot-name
vs content-hash conflation is gone. `Main.kt` wiring updated. Validation stays live-only
(never runs during replay) so invariant #8 is unaffected.
2. *Slot→CAS-hash mapping now recorded as an event* (invariant #9 — env observation
recorded at the moment it runs). New `ArtifactContentStoredEvent(artifactId=logical
slot, contentHash=CAS blake3 hash, sessionId, stageId)`, registered in
`Serialization.kt`, emitted at both CAS write sites (LLM-emitted slot path + tool/
process_result path; the tool path previously discarded the `put()` return value). This
is the durable record that lets content be recovered from CAS by hash after a cold start.
- **remaining follow-up (not in this fix):** the validator/`needs`-rehydration still read the
*in-memory* `artifactContentCache`, which is lost on restart (the F-001 rehydration-gap
suspect). Closing that means resolving slot→hash→CAS from the new
`ArtifactContentStoredEvent` on cold start / re-validation. Foundation is now in place.
### F-006 — TUI input pane render glitch
- **subsystem:** apps/tui-go
- **severity:** minor (cosmetic)
- **evidence:** operator screenshot 16:07 — a garbled/smeared rectangular artifact in the
bottom-right of the input pane (uncleared region / stale cell buffer) while the prompt
text rendered normally on the left.
- **repro:** operator confirms it appears **only on multi-line paste** into the input box;
single-line / typed input renders clean. Likely the input widget doesn't repaint the full
region when a pasted block changes line count.
- **status:** confirmed, repro narrowed (multi-line paste only)
### F-004 — InferenceFailedEvent / RetryAttemptedEvent unmapped to TUI bridge
- **invariant/contract:** functional (operator observability)
- **subsystem:** apps/server (DomainEventMapper)
- **severity:** minor
- **evidence:** `DomainEventMapper: unmapped payload type=InferenceFailedEvent` and
`=RetryAttemptedEvent` repeated. Operator sees failure only via router narration, not a
structured failure/retry surface in the TUI.
- **status:** confirmed
- **note:** `ArtifactContentStoredEvent` is also unmapped (`DomainEventMapper: unmapped
payload type=ArtifactContentStoredEvent`) — same class of gap, new event added this session.
### F-008 — stage `token_budget` not propagated to provider `max_tokens` → truncation
- **invariant/contract:** functional (workflows run end-to-end) / #7 (validatable output)
- **subsystem:** infrastructure/workflow (TomlWorkflowLoader) + core/transitions (StageConfig)
- **severity:** major
- **evidence:** session `f5046936`: `role_pipeline.toml` analyst declares `token_budget=16384`,
but every request carried `"max_tokens":2048` (the StageConfig default). The analyst's 2nd
inference hit `completionTokens=2048 finishReason=length` → JSON truncated mid-string →
artifact validation failed → `no transition condition matched`.
- **root cause:** `TomlWorkflowLoader.toWorkflowGraph` set `StageConfig.tokenBudget` from the
TOML but never populated `generationConfig`, so the 2048 default `maxTokens` reached the
provider. `token_budget` fed context budgeting only, never the completion cap.
- **status:** FIXED — loader sets `generationConfig.maxTokens = s.tokenBudget`. Confirmed live
(session `1876d22a`: 337 completion tokens, no truncation).
### F-009 — tool-execution errors abort the stage instead of feeding back to the model
- **invariant/contract:** functional / #3 (tools suggest; core records and continues)
- **subsystem:** infrastructure/tools/filesystem (FileReadTool) + core/kernel (orchestrator loop)
- **severity:** major
- **evidence:** session `1876d22a`: analyst speculatively read `tests/__init__.py` (doesn't
exist); `FileReadTool` returned `recoverable=false` → orchestrator treated it as FATAL →
`stage failed reason=File not found: tests/__init__.py retryable=false`. Both FATAL and
recoverable ERROR tool results aborted the stage (the `ERROR:` feedback entry was built then
discarded) — the model never got to adapt.
- **root cause:** `FileReadTool` marked missing-file `recoverable=false`; `SessionOrchestrator`
loop returned `StageExecutionResult.Failure` for any ERROR/FATAL tool entry instead of
appending recoverable errors to context and continuing.
- **status:** FIXED — missing file → `recoverable=true`; recoverable errors flow back into the
tool loop (bounded by `MAX_TOOL_ROUNDS`); only FATAL aborts. Confirmed live.
### F-010 — premature `stage_complete` stores an empty artifact → validation fails
- **invariant/contract:** functional / #7 (the artifact-emission contract)
- **subsystem:** core/kernel (SessionOrchestrator)
- **severity:** major (direct consequence of the F-001 grammar drop)
- **evidence:** session `ea968b72`: after reading files the analyst called `stage_complete`
with empty content, never emitting the `analysis` JSON. The orchestrator captured
`response.text` (empty, hash `af1349b9…`) as the artifact → `validation failed`.
- **root cause:** once the GBNF grammar is dropped for tool-bearing stages (F-001), nothing
forces artifact emission; the model can end the stage via `stage_complete` without producing
its declared slot.
- **status:** FIXED — `stage_complete` is rejected while an LLM-emitted slot is unfilled; a
corrective tool-result nudges the model to emit the artifact (bounded by `MAX_TOOL_ROUNDS`).
Confirmed live (session `c02cf17e`: nudge worked, real artifact emitted on the 3rd turn).
- **follow-up (proper fix):** the first-class `emit_artifact` tool from the F-001 TODO would
make artifact production an explicit schema-checked tool call.
### F-011 — artifact schemas over-constrain list fields as `string` → validation rejects arrays
- **invariant/contract:** functional (artifacts validate)
- **subsystem:** config schemas (`~/.config/correx/schemas/`) + core/validation
- **severity:** major (latent — masked by F-007/F-008/F-010 until artifacts validated cleanly)
- **evidence:** session `c02cf17e`: analyst emitted a complete `analysis` with `requirements`
as a JSON **array**; schema declared `requirements`/`affected_areas` as `"type":"string"`
(newline-separated). `JsonSchemaValidator` (single-type, no unions) rejected the array.
The model emits arrays every run.
- **root cause:** schema design mismatch with natural model output; the minimal validator
cannot express a string|array union.
- **status:** FIXED — `requirements`/`affected_areas` (analysis), `components`/`risks` (design),
`steps`/`verification` (impl_plan) changed `string`→`array`. Live config + repo docs synced.
`review_report.verdict` left `string` (the `verdict == approved` transition depends on it).
- **note:** validation strictness itself is correct (#7 held — it rejected mismatched output).
### F-012 — LLM wraps artifact JSON in a markdown code fence → unparseable
- **invariant/contract:** functional (artifacts validate)
- **subsystem:** core/kernel (SessionOrchestrator)
- **severity:** major (model-nondeterministic — any stage, any run)
- **evidence:** session `231179c5`: planner emitted ```` ```json … ``` ```` for `impl_plan`;
the validator parses raw JSON → parse failure → `validation failed`. analyst/architect that
run emitted bare JSON and passed — the difference is pure model nondeterminism.
- **status:** FIXED — `stripCodeFence` in `runInference` removes an outer fence wrapper before
the artifact is hashed/stored/validated (CAS, cache, validation stay consistent). Conservative:
only fires when the whole response is a fenced block.
### F-013 — validation failures never retried → one bad LLM output kills the workflow
- **invariant/contract:** #7 ("Rejected + retried (max_retries)") — the plan's stated contract
- **subsystem:** core/validation (ValidationPipeline)
- **severity:** BLOCKER (robustness — the dominant failure mode with a nondeterministic model)
- **evidence:** session `67776482`: analyst emitted a malformed artifact (a tool-call shape as
JSON content); validation rejected it `retryable=false` and the workflow died, despite
`max_retries=2`. The previous run's analyst was flawless — pure nondeterminism with zero
recovery.
- **root cause:** `ValidationPipeline` marked **all** ERROR-severity validation outcomes
`retryable=false` ("structural errors must be fixed"), conflating graph misconfiguration with
transient bad LLM output.
- **status:** FIXED — payload-validation failures (`artifact_payload` section) are now retryable
(stage re-runs up to `max_retries`); structural/graph errors stay terminal. This wired the
invariant-#7 contract that was never actually implemented. Stabilised the pipeline end-to-end
(session `c6212fd0` completed 9 stages after this).
### F-014 — `file_edit` schema misdescribes its own parameters → edits never apply
- **invariant/contract:** functional (tool side effects) / #5
- **subsystem:** infrastructure/tools/filesystem (FileEditTool)
- **severity:** BLOCKER
- **evidence:** session `c6212fd0`: implementer called `file_edit operation=patch` with
`old_str`/`new_str` (exactly what the schema advertised) → `Missing 'patch' parameter for
'patch' operation`. Across all 4 implementer rounds, not one edit applied; the file stayed
unchanged.
- **root cause:** schema documented `content`/`old_str`/`new_str`; the code reads `content`
(append), `target`/`replacement` (replace), `patch` (unified diff, via `patch -p1`).
`old_str`/`new_str` are read by nothing; `target`/`replacement`/`patch` are documented
nowhere. A schema-following model can never `replace` or `patch`.
- **status:** FIXED — schema now documents the real params (`target`/`replacement`/`patch`),
drops the dead `old_str`/`new_str`. Plus: successful edits emit a `diff -u` unified diff
(operator-requested), surfaced in `ToolReceipt.diff` and the `file_written` artifact.
### F-015 — tool-output artifacts auto-validated without verifying the side effect → false success
- **invariant/contract:** #5 (all side effects captured in events) / functional (real output)
- **subsystem:** core/kernel (SessionOrchestrator emitToolArtifacts + dispatch)
- **severity:** BLOCKER (a green `done` run that changed nothing — worst-case for a kernel)
- **evidence:** session `c6212fd0`: workflow completed `terminalStage=done, verdict=approved,
9 stages`, but `git status` clean and `calc.average` bug intact. `emitToolArtifacts`
unconditionally emitted `ArtifactCreated→Validating→Validated` for the `patch` slot every
implementer round regardless of any write; `validateSlotAsFileWritten` early-returns when the
slot has no content (it never had any) → rubber-stamped. The reviewer then approved a no-op.
- **root cause:** file-mutating tools never produced a `file_written` artifact (unlike shell
tools → `ProcessResultArtifact`); tool executions emitted no completion/file-write events at
all, so nothing tied the `patch` slot to a real write.
- **status:** FIXED — dispatch now emits `ToolExecutionCompleted`/`Failed` + `FileWrittenEvent`,
and **materialises a real `file_written` artifact** (path, content hash, size, diff) from the
on-disk result. `emitToolArtifacts` gates the `file_written` slot on that content: no write →
no artifact → `verifyProduces` fails the stage → retry → honest failure (confirmed live,
session `57d5795d`: `did not produce declared artifacts: patch retryExhausted=true`).
### F-016 — `stageTimeoutMs` hardcoded at 60s, too tight for local models + not configurable
- **invariant/contract:** functional (operability)
- **subsystem:** core/kernel (OrchestrationConfig) + apps/server (Main)
- **severity:** major
- **evidence:** session `57d5795d`: ~15 `TIMEOUT: Timed out waiting for 60000 ms` across the
run (78 on implementer alone). Large implementer/reviewer turns (plan + design + file
contents) were cut off mid-generation, burning all retries → `did not produce patch`.
- **root cause:** `OrchestrationConfig.stageTimeoutMs` defaults to `60_000L` and is hardcoded
in `Main` (`defaultOrchestrationConfig`) with no config wiring.
- **status:** PARTIAL — raised to `180_000L` in `Main`. Should be surfaced as a config knob
(TODO left at the patch site).
### F-017 (observation, not a correx defect) — local model does not reliably issue write tools
- **invariant/contract:** n/a (model capability)
- **subsystem:** n/a (provider model `llama-cpp:default`)
- **severity:** informational
- **evidence:** session `57d5795d` implementer: 6 inference attempts, **only `file_read` calls**
— zero `file_edit`/`file_write`, zero `stage_complete`. Even on turns that completed within
timeout, the model read repeatedly and never attempted the edit it was instructed to make.
- **note:** correx offers `file_write`/`file_edit` correctly and the implementer prompt
explicitly instructs tool-based writing. This is a model-selection conclusion: role_pipeline's
correctness is sound, but this local model is too weak for the implementer stage. Re-test the
implementer with a stronger model before drawing functional conclusions about that stage.
---
### F-018 — implementer emits the change as prose instead of invoking the write tool
- **invariant/contract:** functional (workflows write expected files) — mitigation for F-017
- **subsystem:** core/kernel (SessionOrchestrator tool loop)
- **severity:** major (robustness; the stage silently failed instead of coaxing a write)
- **evidence:** sessions `57d5795d`, `fef41879`: implementer dispatched only `file_read`
(23 calls), then ended turns with `finishReason=stop`/`length` — emitting the corrected code
as message content, never calling `file_edit`/`file_write`. The write tools WERE offered in the
request (confirmed: `file_edit`/`file_write` present in the implementer's tools array). The
stage then failed `did not produce declared artifacts: patch` with no attempt to redirect the
model.
- **status:** FIXED (staged) — the tool loop now nudges: a content turn (or a `stage_complete`)
while a `file_written` slot is unfilled triggers a corrective tool-result ("you MUST call
file_edit/file_write …") and a re-run, bounded by `MAX_TOOL_ROUNDS`. If the model never
complies it fails honestly at the round cap — a clean signal that the model (not correx) is the
wall. Unit tests green; awaiting a live run to confirm it tips this model into tool-calling.
### F-019 — `file_written` artifact carries only metadata → reviewer can't see the change, loops forever
- **invariant/contract:** functional (refinement loop / reviewer can verify work)
- **subsystem:** core/artifacts (FileWrittenArtifact) + core/kernel (needs-injection) + the diff path
- **severity:** major
- **evidence:** session `265f5fe6` — the implementer **wrote correctly** (`file_write`, real diff;
`calc.average` fixed, all 7 pytest pass). But the reviewer (`needs=[patch, impl_plan]`) saw only
the `FileWrittenArtifact` metadata and returned `changes_requested`: *"Cannot verify the patch
content — only metadata (hash, size, mode) was provided, not the actual code changes."* The
refinement loop hit its 2-iteration cap → `WorkflowFailed: refinement loop exceeded 2 iterations`.
- **root cause:** `FileWrittenArtifact` is `(path, contentHash, size, mode)` — no diff, no content.
The diff added in F-014 lives on `ToolReceipt.diff` / the tool output, but is NOT threaded into
the artifact the reviewer's `needs` injection reads. (Also: the implementer used `file_write`,
which produces no diff at all.) So the reviewer is structurally unable to review the change — the
flip side of the operator's original "patch tool should provide a diff" point.
- **status:** FIXED (staged) — `FileWrittenArtifact` gained a `diff` field; both `file_edit` and
`file_write` now compute a `diff -u` unified diff (shared `unifiedDiff` helper in
`DiffUtil.kt`) and surface it via `ToolResult.metadata["diff"]`; `materializeFileWritten`
threads it into the artifact the reviewer's `needs`-injection reads. Unit tests green (artifacts
6, filesystem 38, kernel, validation). Awaiting a live run to confirm the reviewer can now verify
the change and the loop converges.
- **secondary observation:** reviewer verdicts were nondeterministic — one turn emitted
`verdict=approved` ("approved at tier T3…"), later turns `changes_requested`. With no diff to
anchor on, the verdict is essentially ungrounded.
### F-020 — `file_edit` failures are fatal + model defaults to the fragile `patch` op
- **invariant/contract:** functional (tool robustness) / #3 (tools suggest, core records & continues)
- **subsystem:** infrastructure/tools/filesystem (FileEditTool) + implementer prompt
- **severity:** major
- **evidence:** session `6ebe0c50` — the implementer correctly called `file_edit` (F-014 works)
but chose `operation=patch` and produced a non-applying unified diff → `patch -p1` exit 2 →
`Patch failed with exit code 2`. The failure was `recoverable=false` (FATAL) → the stage aborted
with no chance for the model to fall back to `replace`/`file_write`. File unchanged.
(Also observed: a client-side `Request timeout has expired` independent of `stageTimeoutMs`.)
- **root cause:** (a) model-correctable `file_edit` failures (bad patch, target-not-found, bad
params, unknown op) were all marked `recoverable=false`, so they aborted instead of feeding the
error back (same class as F-009); (b) nothing steered the finicky local model away from the
fragile `patch` op toward the reliable `replace`/`file_write`.
- **status:** FIXED (staged) — (a) those failures are now `recoverable=true` → fed back into the
loop so the model can self-correct or fall back (and F-018's write-nudge re-engages); the patch
failure message now explicitly suggests `replace`/file_write. (b) Tool description + `operation`
description recommend `replace`/file_write over `patch`; implementer prompt rewritten to call a
write tool (prefer file_write/replace, avoid patch) and not `stage_complete` before writing.
Filesystem tests green (38). Awaiting a live run.
- **note (model-swap groundwork):** correx already has per-stage model selection in the data model
(`StageConfig.modelId`, capability routing, multi-provider config) but it is **unwired** — the
TOML loader parses neither a per-stage `model` nor `capabilities`, and `DefaultInferenceRouter`'s
`modelId` overload falls back to capability routing. Enabling the llm_pipeline-style per-stage
model swap (e.g. a tool-strong model for implementer/reviewer) needs only targeted wiring, not
new architecture. Deferred pending the robustness-first decision.
### F-021 — resume-after-restart `rehydrate()` reuses the F-007 slot-name-vs-CAS-hash conflation
- **invariant/contract:** #1 (event log is the only source of truth) / #8 (replay/restore is
environment-independent) — resume-after-restart
- **subsystem:** core/kernel (DefaultSessionOrchestrator.rehydrate)
- **severity:** major
- **evidence (static, not yet probed live):** `DefaultSessionOrchestrator.rehydrate` (line 171)
walks `ArtifactValidatedEvent`s and calls `artifactStore.get(p.artifactId)` to repopulate
`artifactContentCache`. `p.artifactId` is the **logical slot id** ("analysis", "patch"), but
`CasArtifactStore.get` requires a content **hash** (`id.value.hexToBytes()`). `get("analysis")`
→ non-hex → throws → `rehydrate` throws → `resumeSession (rehydrate)` logs failure. So a
mid-session resume cannot restore produced-artifact content; downstream `needs`-injection and
artifact validation (which read `artifactContentCache`) would then be empty/broken.
- **root cause:** identical to F-007 — slot name vs CAS content-hash. The F-007 fix added
`ArtifactContentStoredEvent(artifactId=slot, contentHash=hash)` as the durable bridge, but
`rehydrate` does not consult it; it still passes the slot id straight to the hash-keyed CAS.
- **fix direction:** rehydrate from `ArtifactContentStoredEvent` — `artifactStore.get(contentHash)`
then `artifactContentCache["sessionId:slot"] = content`. The F-007/F-015 events already record
the needed slot→hash mapping; this just consumes it on cold start.
- **status:** FIXED — `rehydrate` now scans `ArtifactContentStoredEvent` (the durable slot→CAS-hash
bridge added in F-007) and calls `artifactStore.get(p.contentHash)`, keying the cache by the
logical slot name (`p.artifactId`). The slot name is never passed to the hash-keyed CAS, so the
odd-length-hex crash is gone and content is restored on cold start. `RehydrateTest` reworked to
use a content-addressed fake store (slot name ≠ content hash) so it actually exercises the bug;
both cases green. Replay (#8) unaffected — rehydrate is live-resume-only. Still wants a live
#1/#8 resume probe to confirm end-to-end restoration.
### Steering (functional contract) — partially exercised live
- During the live operator sessions, approval-with-steering was informally validated: the operator
steered toward a specific indentation; the initial attempt was made without that guidance and the
steered re-attempt incorporated it. So the steering note entered the journal and affected the
run. Not yet a controlled probe (no event-level assertion captured), but not untouched either.
## Audit outcome summary
role_pipeline is now **fully green, end-to-end**. Session `4679ed1e` (the final run) completed
`WorkflowCompleted, terminalStage=done, 5 stages` in a single clean pass — analyst → architect →
planner → implementer → reviewer → done, **no review-loop bounce**:
- the implementer followed the F-020 steering and used `file_write` (4 calls, 3 `FileWrittenEvent`s,
no `patch` botching);
- `calc.average` was correctly fixed (`if len(values) == 0: return 0.0`), real git diff, **all 7
pytest pass**;
- the `file_written` artifact carried the **diff** (F-019), the reviewer's `needs`-injection showed
`Input artifact: patch` *with the diff*, so the reviewer could actually verify the change and
**approved on the first pass**.
Every correctness defect found (F-001, F-005, F-007 through F-016, F-018, F-019, F-020) is fixed
and now confirmed live. The earlier failures (`265f5fe6` reviewer loop, `6ebe0c50` bad patch) were
the last two blockers; both are closed.
**Still to PROBE** (implementations mostly exist; the audit hasn't run the adversarial checks —
"outstanding" here means un-probed, not unimplemented):
- **#8 offline replay** — *implemented and unit-tested* (`ReplayOrchestrator` + `testing/replay/`
suite). Needs the adversarial probe (export log, replay with network/LLM off) to confirm.
- **#1 resume-after-restart** — *wiring implemented* (`POST /sessions/{id}/resume`, `resume()`,
`rehydrate()`) but **F-021 shows `rehydrate()` is broken** (slot-name-vs-CAS-hash, same as
F-007). Strongly expected to fail a live probe until F-021 is fixed.
- **#2** projection isolation, **#4** policy-vs-approval, **#6** compression non-authority,
**#9** env-observation replay — not yet probed this audit.
- **undo / cancel / approval-gate** — functional contracts not yet exercised. **Steering** was
*partially* exercised live (operator steered indentation; re-attempt incorporated it).
There is now a real recorded, validated file change (`4679ed1e`) to point the undo/replay/resume
probes at — though resume needs F-021 first. **Optional groundwork:** wire per-stage model
selection (F-020 note); the model remained the flaky element even on this passing run.