Skip to content

fix(nudge): unify transient-injection budgets into per-turn injection ledger - #228

Closed
ranxianglei wants to merge 2 commits into
masterfrom
2026-08-25_injection-ledger-governance
Closed

fix(nudge): unify transient-injection budgets into per-turn injection ledger#228
ranxianglei wants to merge 2 commits into
masterfrom
2026-08-25_injection-ledger-governance

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Summary

Implements the comprehensive governance plan from the #223 post-mortem audit (ework #18): one injection ledger replaces the whole family of ad-hoc retry/nudge budgets (#6 breaker, and the unmerged #7 / #9 / #194 / #217 branches).

Root cause of the #223 amplification chain

pi rebuilds the outbound array on every LLM call, so any transient injection re-appends per fire unless budgeted. Every patch in the lineage measured a different quantity — Set dedup, distinct failed calls, fire budgets — none measured injections:

mechanism counted escape
retry prompt (removed in #225) distinct failed toolCallIds model never retries → stuck at 1, re-injects forever (#223)
#6 nudge breaker failed/no-op compress calls compress({content: []})"No ranges provided." isError:false → neutral, counter never moves → emergency nudge re-injects forever
throttle kick / delegate notify (none) synthetic user msgs rotate the turn key → reset budgets mid-loop

The fix

  • One ledger (runtime.noteInjection(turnKey, kind, budget)): nudge = 1/genuine-turn, emergency = MAX_EMERGENCY_NUDGES_PER_TURN (3)/turn. Budgets count injections — ignoring, failing, no-op, or neutral compress responses cannot extend them. Only a genuine user-turn switch resets.
  • Genuine turn key: lastUserMessageId (src/tokens.ts) now skips synthetic user messages ([ACP:provider-throttle] kicks, [acp_delegate …] notifications) — they no longer reset budgets.
  • Deleted ~150 LOC of compress-outcome machinery whose only surviving customer was the emergency gate: noteCompressOutcomes, compressOutcomeSeen, compressFailTurnKey/Count, compressRetryCappedFor, collectCompressOutcomes, isCompressSuccessText/isCompressNoopText, and the separate nudgeShownTurns Set. The neutral escape dies with it.
  • One-shot UI notice when the emergency budget exhausts ("nudge paused … emergency truncation still active"); kernel ≥95% truncation remains the mechanical backstop.

Adversarial regression test

20 context fires alternating every response shape the old budgets could not reach (hard failure / no-op panel / neutral "No ranges provided." / plain silence) → exactly 3 emergency injections. Throttle-kick and delegate messages do not re-arm the budget; a genuine user message does. #223 regression (zero retry prompts ever) retained.

Test plan

  • npm run typecheck
  • npm test — 426/426 ✓
  • npm run build ✓ (dist 517.77 KB, −1.95 KB)

Closes the governance gap behind #223/#6; supersedes the budget logic in unmerged #7/#9/#194/#217 (their branches can be closed after this merges).

… ledger

pi rebuilds the outbound array on every LLM call, so any transient
injection re-appends per fire unless budgeted. The #223 amplification
chain happened because every patch measured a different quantity (Set
dedup, failed-call counts, fire budgets) instead of injections.

Replace all of them with one ledger in runtime:
- noteInjection(turnKey, kind, budget): nudge=1/turn,
  emergency=MAX_EMERGENCY_NUDGES_PER_TURN(3)/turn; budgets count
  INJECTIONS, so ignoring/failing/no-op/neutral compress responses
  cannot extend them; only a genuine user-turn switch resets.
- lastUserMessageId skips synthetic user messages (throttle kicks,
  delegate notifications) so they no longer rotate the turn key.
- delete the compress-outcome machinery (noteCompressOutcomes,
  compressOutcomeSeen, compressFailTurnKey/Count, compressRetryCappedFor,
  collectCompressOutcomes, isCompressSuccessText/isCompressNoopText,
  separate nudgeShownTurns) - its only surviving customer was the
  emergency gate, whose neutral escape (compress([]) -> 'No ranges
  provided.' isError:false -> counter never moves) dies with it.
- one-shot UI notice when the emergency budget exhausts; kernel >=95%
  truncation remains the mechanical backstop.

Adversarial regression: 20 fires alternating failure/no-op/neutral/
silence yield exactly MAX_EMERGENCY_NUDGES_PER_TURN injections.

Refs #223, #6, #7, #194, #9, #217
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

📦 Built Extension Artifact

Branch: 2026-08-25_injection-ledger-governance (8f390f2)

Option A — Install from npm PR tag (recommended)

pi install npm:billion-context-pi@pr-228

Each push to this PR publishes a new version under the pr-228 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pi-pr228.tgz
pi install ./package

This comment is automatically updated on each push.

The injection-ledger turn key hardcoded "[ACP:provider-throttle]" and
"[acp_delegate " in lastUserMessageId() while the injection sites built
their own headers independently — two copies of the same magic strings
that can silently drift apart. If they ever diverge, synthetic user
messages stop being skipped, rotate the turn key, and reset the very
per-turn budgets that bound runaway injection loops.

Single source of truth now:
- export DELEGATE_NOTIFY_PREFIX from delegate-tool.ts; injectResult()
  builds its header from it
- SYNTHETIC_USER_PREFIXES = [THROTTLE_KICK_SENTINEL,
  DELEGATE_NOTIFY_PREFIX] in tokens.ts; lastUserMessageId() matches
  against the array
- document the contract: any NEW synthetic sendUserMessage injection
  site MUST register its prefix there
@ranxianglei

Copy link
Copy Markdown
Owner Author

Follow-up (review nit 1 & 4): commit 8f390f2 removes the duplicated magic strings — lastUserMessageId() now matches against SYNTHETIC_USER_PREFIXES = [THROTTLE_KICK_SENTINEL, DELEGATE_NOTIFY_PREFIX], and the delegate header is built from the newly exported DELEGATE_NOTIFY_PREFIX (src/delegate-tool.ts). Single source of truth on both the injector side and the matcher side; the constant documents the contract that any new synthetic sendUserMessage site must register its prefix there. Behavior unchanged (typecheck/build/tests/CI green).

@ranxianglei

Copy link
Copy Markdown
Owner Author

关闭原因:这个修复是错误的——它偏离了本项目的既定设计,不应合入。

此前 review 给出的结论(LGTM、“修复正确且必要”)是错误的,请勿作为参考。作为 review 方,我未能理解到设计层面的意图,只机械地验证了实现本身的行为,是我太愚蠢,没能理解到这一层。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant