Skip to content

Commit fd1ba2d

Browse files
committed
fix(pi/reasoning): empty cleared reasoning instead of "[cleared]" (#162 D2, Pi)
Pi's clearOldReasoningPi/replayClearedReasoningPi rewrote aged thinking blocks to the non-empty string "[cleared]" while KEEPING the original thinkingSignature. Pi has no empty-sentinel conversion step (unlike OpenCode), so on canonical Claude/Bedrock that "[cleared]" + original signature reached the wire as a content/signature MISMATCH — a real 400 hazard (the old PARITY note calling this "safe, matches OpenCode" was wrong). Fix (confirmed against Pi serializer source via the Pi peer): empty the thinking text and drop the stale signature, with no per-provider gate — EXCEPT leave `redacted` thinking blocks untouched. Every Pi serializer drops an empty NON-redacted thinking block before the wire (anthropic.ts, openai-completions.ts with reasoning_content="" auto-fill, amazon-bedrock.ts, google-shared.ts, mistral.ts). Redacted blocks are the exception: they serialize `redacted` BEFORE the empty-thinking check (transform-messages.ts, anthropic.ts), so emptying one + dropping its signature would put a MALFORMED redacted block (no data, no sig) on the wire — caught by Oracle review of this diff. Redacted blocks carry no plaintext to save, so we keep them verbatim (safe + byte-stable). Intentionally DIFFERENT from OpenCode's gated [cleared]→sentinel path (OpenCode's non-Anthropic adapters forward empty parts, so it must gate; Pi's drop them, so it needn't) — PARITY.md #10 rewritten. clearOldReasoning only touches OLD assistants; the latest keeps its real reasoning on both harnesses. Replay sets the identical empty+no-sig shape so defer passes stay byte-identical. +1 redacted regression test. Pi 471/0, tsc + biome clean.
1 parent f66ed71 commit fd1ba2d

6 files changed

Lines changed: 66 additions & 10 deletions

File tree

CONFIGURATION.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ Configures the background historian agent that compresses session history into c
311311
| `variant` | `string` | **OpenCode only.** Agent variant — selects a thinking/reasoning preset configured in OpenCode itself. Pi uses `thinking_level` instead. |
312312
| `thinking_level` | `string` | **Pi only.** Explicit reasoning level passed to Pi when spawning the historian subagent (`off`, `low`, `medium`, `high`). Required for GitHub Copilot reasoning models on Pi — without it, Copilot injects `"minimal"` as a default and then rejects it (HTTP 400). The Pi setup wizard prompts for this when you pick a `github-copilot/*` model. |
313313
| `prompt` | `string` | Custom system prompt override. |
314-
| `two_pass` | `boolean` | Default `false`. When `true`, runs a second editor pass after each successful historian output. The editor (a separate hidden `historian-editor` agent using the same fallback chain) re-reads the draft and removes low-signal `U:` lines, redundant paraphrases, and cross-compartment duplicates, producing cleaner narrative-first summaries. Falls back to the draft if the editor call or its validation fails, so it can never regress behavior. Adds one extra historian-scale call per compartment publication. Recommended for non-reasoning models and open-weight local models where the single-pass draft is noisier. For models with extended thinking/reasoning enabled in OpenCode (Claude 4+, GPT-5.x reasoning variants), the single-pass output is usually already clean and `two_pass` can stay `false`. |
314+
| `two_pass` | `boolean` | Default `false`. When `true`, runs a second editor pass after each successful historian output. The editor (a separate hidden `historian-editor` agent using the same model resolution as the historian) re-reads the draft and removes low-signal `U:` lines, redundant paraphrases, and cross-compartment duplicates, producing cleaner narrative-first summaries. Falls back to the draft if the editor call or its validation fails, so it can never regress behavior. Adds one extra historian-scale call per compartment publication. Recommended for non-reasoning models and open-weight local models where the single-pass draft is noisier. For models with extended thinking/reasoning enabled in OpenCode (Claude 4+, GPT-5.x reasoning variants), the single-pass output is usually already clean and `two_pass` can stay `false`. |
315315

316316
---
317317

packages/docs/src/content/docs/concepts/dreamer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ When `dreamer.inject_docs` is true (the default), these files inject into the co
5252

5353
## Cost and model selection
5454

55-
The dreamer uses your configured dreamer model (or fallback chain) and reads your codebase during verification and doc maintenance. Each task spawns a child session with its own context window. Budget accordingly — a full dream run with all five tasks can be several API calls.
55+
The dreamer uses your configured dreamer model (and any `fallback_models` you set, then your session model as a last resort) and reads your codebase during verification and doc maintenance. Each task spawns a child session with its own context window. Budget accordingly — a full dream run with all five tasks can be several API calls.
5656

5757
Because it runs during idle time, the dreamer is a good fit for local models, even slow ones. Configure the model in `magic-context.jsonc` under `dreamer.model`.
5858

packages/docs/src/content/docs/reference/configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ The background agent that condenses old conversation into compact history.
7878
| `historian.fallback_models` | string \\| string[] || Fallback model IDs if primary is unavailable |
7979
| `historian.two_pass` | boolean | `false` | Run a second editor pass over historian output to clean low-signal U: lines and cross-compartment duplicates. Adds ~1 extra API call and ~1.3x cost per historian run. Useful for models without extended thinking support. (default: false) |
8080
| `historian.thinking_level` | `"off"` \\| `"minimal"` \\| `"low"` \\| `"medium"` \\| `"high"` \\| `"xhigh"` || Pi only: explicit thinking level passed as --thinking <level> to Pi historian subagent invocations. Required when using reasoning models (e.g. github-copilot/gpt-5.4) because Pi's default thinking-level resolution can pick a value the provider rejects. OpenCode users set variant instead. Valid: off \| minimal \| low \| medium \| high \| xhigh |
81+
| `historian.disallowed_tools` | `"*"` \\| `"read"` \\| `"aft_outline"` \\| `"aft_zoom"` \\| `"aft_search"`[] | `[]` | OpenCode only. Tools to REMOVE from the historian's default allow-list [read, aft_outline, aft_zoom, aft_search]. Applies to both historian and historian-editor agents. Use ["*"] to strip all tool definitions from the model request — this prevents weak instruction-following models (e.g. mistral-small-latest) from entering tool-calling loops. Individual tool names remove just that tool. Note: a user-supplied historian.permission override can re-allow a tool that disallowed_tools removed — disallowed_tools sets the baseline, permission overrides take precedence. (default: []) |
8182
| `historian_timeout_ms` | number (60000–) | `300000` | Timeout for each historian prompt call in milliseconds (default: 300000) |
8283
| `commit_cluster_trigger` | object || Commit-cluster trigger: fire historian when enough commit clusters accumulate in the unsummarized tail |
8384
| `commit_cluster_trigger.enabled` | boolean | `true` | Enable commit-cluster based historian triggering (default: true) |

packages/pi-plugin/PARITY.md

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,13 +280,19 @@ deliberate and source-justified.
280280
signature) on the wire. (#162 D2.)
281281

282282
- **Pi** (`reasoning-replay-pi.ts`): EMPTIES the thinking text (`thinking = ""`)
283-
and **drops the now-stale `thinkingSignature`**, UNCONDITIONALLY (no
284-
per-provider gate). Every Pi serializer drops an empty thinking block before the
285-
wire — `anthropic.ts` (empty thinking skipped), `openai-completions.ts`
286-
(filtered out of `nonEmptyThinkingBlocks`, with `reasoning_content=""`
287-
auto-filled for providers that require it), `amazon-bedrock.ts` (empty thinking
288-
skipped). So no block and no signature reach ANY provider, which structurally
289-
eliminates the stale-signature mismatch and needs no gate.
283+
and **drops the now-stale `thinkingSignature`**, with NO per-provider gate —
284+
EXCEPT it leaves `redacted` thinking blocks **untouched**. Every Pi serializer
285+
drops an *empty non-redacted* thinking block before the wire — `anthropic.ts`
286+
(empty thinking skipped), `openai-completions.ts` (filtered out of
287+
`nonEmptyThinkingBlocks`, with `reasoning_content=""` auto-filled for providers
288+
that require it), `amazon-bedrock.ts`/`google-shared.ts`/`mistral.ts` (empty
289+
thinking skipped). So no normal block and no signature reach ANY provider, which
290+
structurally eliminates the stale-signature mismatch and needs no gate.
291+
**Redacted blocks are the exception**: they serialize `redacted` BEFORE the
292+
empty-thinking check (`transform-messages.ts`, `anthropic.ts`), so emptying one
293+
+ dropping its signature would put a malformed redacted block (no data, no sig)
294+
on the wire. They carry no plaintext to save, so Pi keeps them verbatim — safe
295+
and byte-stable.
290296

291297
Why the OLD "keep the signature" note was wrong: a `thinkingSignature` is a
292298
cryptographic signature over the ORIGINAL thinking text, so `[cleared]` (or any

packages/pi-plugin/src/reasoning-replay-pi.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,43 @@ describe("clearOldReasoningPi", () => {
148148
expect(result.cleared).toBe(0);
149149
expect(result.newWatermark).toBe(0);
150150
});
151+
152+
it("leaves REDACTED thinking blocks untouched (signature + data preserved)", () => {
153+
// A redacted block bypasses the empty-thinking drop in Pi's serializers
154+
// (transform-messages.ts / anthropic.ts serialize `redacted` before the
155+
// empty check), so emptying it + dropping the signature would leave a
156+
// malformed redacted block on the wire. It must be preserved verbatim.
157+
const messages = [
158+
{
159+
role: "assistant",
160+
timestamp: 1,
161+
content: [
162+
{
163+
type: "thinking",
164+
thinking: "opaque-redacted-payload",
165+
thinkingSignature: "sig-abc",
166+
redacted: true,
167+
},
168+
],
169+
},
170+
];
171+
const id0 = piMessageStableId(messages[0], 0);
172+
if (!id0) throw new Error("piMessageStableId returned undefined");
173+
const messageIdToMaxTag = new Map<string, number>([[id0, 1]]);
174+
const result = clearOldReasoningPi({
175+
messages,
176+
messageIdToMaxTag,
177+
clearReasoningAge: 1,
178+
piMessageStableId,
179+
});
180+
expect(result.cleared).toBe(0);
181+
expect(messages[0].content[0]).toMatchObject({
182+
type: "thinking",
183+
thinking: "opaque-redacted-payload",
184+
thinkingSignature: "sig-abc",
185+
redacted: true,
186+
});
187+
});
151188
});
152189

153190
describe("replayClearedReasoningPi", () => {

packages/pi-plugin/src/reasoning-replay-pi.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* OpenCode's reasoning-clearing replay was added to fix.
1414
*
1515
* Behavior:
16-
* - On execute passes (cache-busting): walk Pi assistant messages
16+
* - On execute passes (cache-busting): walk Pi assistant messages
1717
* whose tag number is older than `clear_reasoning_age` from the
1818
* newest tag, EMPTY each `PiThinkingContent.thinking` (and drop its
1919
* stale signature), persist watermark = max-tag-cleared in
@@ -153,6 +153,14 @@ export function clearOldReasoningPi(args: {
153153
(part as { type?: unknown }).type === "thinking"
154154
) {
155155
const tp = part as PiThinkingContent;
156+
// Leave REDACTED thinking blocks untouched. Unlike normal thinking,
157+
// redacted blocks bypass the empty-drop in Pi's serializers
158+
// (transform-messages.ts and anthropic.ts serialize `redacted`
159+
// before the empty-thinking check), so emptying one + dropping its
160+
// signature would leave a malformed redacted block (no data, no sig)
161+
// on the wire. A redacted block carries no plaintext to save anyway;
162+
// keeping it verbatim is both safe and byte-stable across passes.
163+
if (tp.redacted) continue;
156164
// Empty the thinking AND drop its now-stale signature (a signature
157165
// over the original text would mismatch the emptied content). The
158166
// empty block is dropped by every Pi serializer, so neither reaches
@@ -271,6 +279,10 @@ export function replayClearedReasoningPi(args: {
271279
(part as { type?: unknown }).type === "thinking"
272280
) {
273281
const tp = part as PiThinkingContent;
282+
// Mirror clearOldReasoningPi exactly: redacted blocks are left
283+
// untouched (they bypass the serializers' empty-drop, so emptying
284+
// one would put a malformed redacted block on the wire).
285+
if (tp.redacted) continue;
274286
// Replay the exact clear shape from clearOldReasoningPi: empty
275287
// thinking + dropped signature, so defer passes are byte-identical
276288
// to the cache-busting pass that set the watermark.

0 commit comments

Comments
 (0)