From 514aeae75fc02112c48eb8b72455d6feb3ac93ed Mon Sep 17 00:00:00 2001 From: kami Date: Sun, 26 Jul 2026 21:00:36 +0400 Subject: [PATCH] docs(audit): context entry role/layer placement sweep (#312) Audits every ContextEntry producer against PromptRenderer's three real render destinations (system fold / inline / trailing user slot). Finds four escalation-grade entries roled SYSTEM and therefore buried in the leading system message: recoveryTicket, remainingDelta, groundingFeedback, rejectionFeedback. remainingDelta additionally mutates the cached system prefix every turn. Report only, no code changes. Re-role work filed as #313. Co-Authored-By: Claude Opus 5 --- docs/audits/2026-07-26-context-role-audit.md | 73 ++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/audits/2026-07-26-context-role-audit.md diff --git a/docs/audits/2026-07-26-context-role-audit.md b/docs/audits/2026-07-26-context-role-audit.md new file mode 100644 index 00000000..2cc3bb24 --- /dev/null +++ b/docs/audits/2026-07-26-context-role-audit.md @@ -0,0 +1,73 @@ +# Audit — ContextEntry role/layer placement (Vikunja #312) + +**Date:** 2026-07-26 · **Scope:** every `ContextEntry` producer on the orchestrator stage path. +**Output:** table + recommendations. No code changes. + +## How placement actually resolves + +`PromptRenderer.render` (core/inference/.../PromptRenderer.kt): + +1. **Any** entry with `layer == L0` **or** `role == SYSTEM` → folded into the single leading + system message, ordered by `(layer.ordinal, entry.ordinal)`. Role is irrelevant at L0. +2. Everything else renders inline as its own message, ordered by `entry.ordinal` + (layer priority is a tiebreak only when all ordinals are 0 — router chat). +3. `sourceType == "steeringNote"` **and** SYSTEM-folded → additionally re-emitted as a trailing + user "Reminder" turn. +4. `sourceType ∈ repairMandateSourceTypes` (currently `{"retryFeedback"}`) → pulled out of the + inline flow and emitted as the **final** message, role user. + +So there are three destinations, not four: **leading system fold**, **inline transcript**, +**trailing user slot**. `EntryRole.SYSTEM` is not "high salience" — it is "buried at the top". + +## Inventory + +| Entry (sourceType) | Producer | Current layer/role → renders as | Recommended | Rationale | +|---|---|---|---|---| +| `retryFeedback` | ContextFeedback.kt:34 | L1/USER → **trailing** | keep | Reference implementation (#293). | +| `recoveryTicket` | ContextFeedback.kt:118 | L1/SYSTEM → **system fold** | **→ USER + trailing** | ⚠️ Highest-value defect. The recovery stage exists *only* because of this ticket, and its mandate ends up above the whole transcript. Same shape as `retryFeedback`; got the opposite treatment. | +| `remainingDelta` | ContextFeedback.kt:186 | L1/SYSTEM → **system fold** | **→ USER + trailing** | ⚠️ Loop-state, recomputed every turn a write lands. It is the stage's completion signal and it is buried, *and* mutating it invalidates the cached system prefix each turn — the one cache anti-pattern #312 asked to flag. Trailing is both more salient and cache-safe. | +| `groundingFeedback` | ContextFeedback.kt:89 | L1/SYSTEM → **system fold** | **→ USER + trailing** | Gate verdict on a returned plan; "emit a corrected plan" is a repair mandate by any reading. | +| `rejectionFeedback` | SessionOrchestratorContext.kt:117 | L2/SYSTEM → **system fold** | **→ USER + trailing** | Literally operator voice ("the operator declined"). Escalation-grade; the user channel is where it belongs. | +| `steeringNote` (locked) | Context.kt:80 | L0/SYSTEM → system fold **+ trailing anchor** | keep | Already double-anchored. | +| `steeringNote` (unlocked) | Context.kt:94, ToolExec.kt:528 | L2/USER → **inline** | keep, note inconsistency | The renderer's anchor only catches the SYSTEM variant, so the two steering paths get different salience. Cosmetic, not load-bearing. | +| `artifactRepair` | Artifacts.kt:132 | L2/USER → inline, but **sole entry** in its pack | keep | Isolated tools-less pack; already the last (only) message. | +| `criticFeedback` / `neededArtifact` | Context.kt:444 | L1/USER → **inline** | keep | Stage *input*, not a mid-loop correction. Trailing slot should not carry inputs. | +| `unconfirmedFix` | Concepts.kt:107 | L1/USER → inline | keep | Advisory, not a mandate. | +| `toolResult` (incl. failures) | Execution.kt:351/564, ToolExec.kt:215… | L2/TOOL → inline tool turns | **keep** | Routine self-correctable — "target not found", READ_BEFORE_WRITE, patch misses. Lifting these floods the user channel and destroys the effect. Escalation happens on *repetition*, and that path already exists (`STAGE_LOOP_BREAK_GATE` → `recoveryTicket`); #309's loop-breaker is the right place, not the per-failure site. | +| `assistantToolCall` | ToolExec.kt:205… | L2/ASSISTANT → inline | keep | Correct. | +| `initialIntent` | Context.kt:162 | L1/USER, but **L0 ⇒ system fold**… no: L1/USER → inline | keep | Doc comment at Context.kt:148 claims "pinned L0 SYSTEM"; code says L1/USER. Comment is stale — **fix the comment**, not the code (L1/USER is right). | +| `clarificationAnswer` | Context.kt:205 | L0/SYSTEM → system fold | keep, flag cache | Mutable mid-run (grows per clarification) inside the cached prefix. Low frequency; acceptable. | +| `schemaInstruction`, `systemPrompt`, `operatingGuidance`, `projectProfile`, `agentInstructions`, `operatorProfile`, `claimedTask`, `promotedConcept`, `verifiedBaseline`, `successfulPlanShape` | various | L0/SYSTEM → system fold | keep | Stable per stage. Correct and cache-friendly. | +| `agentPrompt` | Execution.kt:120/147 | L1/USER → inline | keep | The stage task. | +| `repoMap`, `docsCatalog`, `relevantFiles`, `decisionJournal` | Context.kt:289/376, ContextFeedback.kt:245, Execution.kt:189 | L3/USER → inline | keep | Reference material; #290 already moved these out of the system fold. | + +## Recommendations, in order + +1. **Re-role the four escalation entries** (`recoveryTicket`, `remainingDelta`, + `groundingFeedback`, `rejectionFeedback`) to `EntryRole.USER` and add their sourceTypes to + `PromptRenderer.repairMandateSourceTypes`. One-line change each plus the set. + +2. **Guard the scarcity invariant first.** `repairMandateSourceTypes` currently joins all matches + with `\n\n`. With one member that is fine; with five, a recovery stage on a retry with an unmet + delta emits three stacked "mandates" and the channel stops being authoritative. Before (1) lands, + the trailing slot needs either a priority order emitting the single highest-precedence entry, or + one consolidated block under a single header. Suggested precedence: + `recoveryTicket > retryFeedback > groundingFeedback > rejectionFeedback > remainingDelta`. + `remainingDelta` is the exception worth appending unconditionally — it is the completion signal, + not a competing mandate. + +3. **Cache note.** All four moves are *out of* the system prefix and are therefore cache-positive. + `remainingDelta` is the biggest win: per-turn mutation currently sits inside the cached prefix. + No move *into* system is recommended anywhere. + +4. **Do not touch tool-role failures.** Escalation belongs at the repeat detector (#309), not at + each failure site. + +Everything above stays event-derived (invariant #9); the trailing block is synthetic and should keep +its `## `-headed, non-conversational framing so it never reads as a real operator turn. + +## Note on sequencing + +The sprint deferred #312 behind #307 (manifest event) "so the audit has ground truth". Not needed — +placement is statically determined by `PromptRenderer` + the producer's role/layer, both read +directly. #307 remains useful for verifying the *result* of recommendation (1) on a live run.