Skip to content

fix(behavioral): output-length decay fired on three-call noise - #512

Merged
drewstone merged 1 commit into
mainfrom
fix/decay-minimum-n
Jul 31, 2026
Merged

fix(behavioral): output-length decay fired on three-call noise#512
drewstone merged 1 commit into
mainfrom
fix/decay-minimum-n

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Caught by reading a real report rather than by a test: the deterministic efficiency analyst emitted "output tokens decrease while input tokens increased monotonically" at confidence 1.0 from a 784 → 646 run over three calls. That is an 18% wobble on n=3, not degradation.

Why the old bound could not work

A run of n independent lengths is strictly decreasing by chance with probability 1/n!, so a minimum of 3 fires on roughly one sequence in six.

The paired inputIsMonotonic && inputGrew guard does not offset that, and this is the part worth internalizing: context accumulates by construction in a serial agent loop, so monotonic input growth is very nearly free evidence. The conjunction looked like two independent conditions and was effectively one.

The bound now

  • OUTPUT_DECAY_MINIMUM_CALLS = 5 — chance alone accounts for under 1%, which is the bar a signal reported at full confidence has to clear.
  • OUTPUT_DECAY_MAXIMUM_RETAINED_FRACTION = 0.6 — the last output must fall to at most 60% of the first. Length wanders between turns for reasons that are not degradation, so direction alone is not a finding.

Both constants carry their reasoning in comments, because the next person to loosen them should have to argue with the arithmetic.

Tests

Three fixtures encoded the old three-call shape and were asserting the false positive; they now describe real decay (5 calls, ~33–50% retained). fixture530 (7 calls, 157 → 75 = 48% retained) is unchanged and still fires — the signal keeps its true positive.

Added the regression test that never existed: both rejected shapes are now pinned — the observed 784 → 646 three-call run, and a clean-but-shallow five-call 100 → 80 run.

45 files / 646 tests pass across src/trace-analyst and src/analyst; biome clean; benchmark implementation digest unchanged (this file is outside the sealed set).

The signal shipped a finding at full confidence off a 784 -> 646 run over
three calls. A run of n independent lengths is strictly decreasing by
chance with probability 1/n!, so a minimum of 3 fires on roughly one
sequence in six, and the paired input-growth guard does not offset it:
context accumulates by construction in a serial agent loop, so monotonic
input growth is very nearly free evidence.

Requires 5 serial calls (chance under 1%) and a fall to at most 60% of
the first output, so direction alone is no longer a finding. Three
fixtures that encoded the old three-call shape now describe real decay,
and a regression test pins both rejected shapes — the observed
784 -> 646 run and a clean but shallow 100 -> 80 run.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — b44898e4

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-31T22:48:05Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Concerns 2 (1 low, 1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 63.0s (2 bridge agents)
Total 63.0s

💰 Value — sound

Tightens two thresholds on the output-length-decay detector (min 5 serial calls, ≤60% retained) to kill a real three-call false positive reported at confidence 1.0; minimal, well-reasoned, in-grain.

  • What it does: Adds two named constants to the output-length-decay detector in src/trace-analyst/behavioral-metrics.ts: OUTPUT_DECAY_MINIMUM_CALLS=5 (was hardcoded 3) and OUTPUT_DECAY_MAXIMUM_RETAINED_FRACTION=0.6 (was just last < first). The detector at behavioral-metrics.ts:458-485 now requires both ≥5 serial calls AND the final output ≤ 60% of the first before firing. Tests are retargeted: three fixtures
  • Goals it achieves: Stop reporting normal-length wobble as a high-confidence finding. A deterministic analyst emitted 'output tokens decrease while input tokens increased monotonically' at confidence 1.0 from a 784→646 (18%) run over only 3 calls — pure noise. The goal is to make the signal fire only on genuine decay (late steps that quietly stop doing the work), so human readers trust every finding it emits.
  • Assessment: Good change on its merits. The probabilistic argument is correct and load-bearing: strict monotonic decrease of n independent lengths has probability 1/n!, so n=3 fires on ~1/6 of random sequences, and the paired input-growth guard does NOT offset that because context accumulates by construction in a serial agent loop (input growth is near-free evidence). n=5 drops chance below 1%. The magnitude b
  • Better / existing approach: none — this is the right approach. Searched src/ for any existing decay/trend/statistical-test utility to reuse (rg for decay, retained, monotonic, factorial, 1/n!, minimum-samples). statistics.ts exists but is for benchmark multiple-testing correction (BH FDR), not per-trace deterministic gating; importing it would violate this module's no-model, pure-arithmetic contract. A formal trend test (Spe
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound-with-nits

A well-targeted false-positive fix that tightens two constants in an already-fully-wired detector; no new surface, no duplication, one stale claim-text number to sweep.

  • Integration: Fully integrated. The change does not add surface — it retunes the existing output-length-decay detector in computeTraceMetrics (behavioral-metrics.ts:458-485). That function is called by the behavioralAnalyst analyst (behavioral-analyst.ts:180) which converts the signal into an AnalystFinding via deriveEfficiencyFindings, and by gateTreatmentFromMetrics (treatment-gate.ts:154).
  • Fit with existing patterns: Fits the codebase grain exactly. It is a minimal, in-place retune of one existing detector, consistent with how the neighboring INPUT_GROWTH_FACTOR and MIN_TOOL_CALLS constants already parameterize the other detectors in the same file. The named constants with their reasoning in comments match the repo's documented comment discipline (CLAUDE.md: 'comments describe what the code does and why').
  • Real-world viability: The logic holds under realistic input. Both new bounds are applied as a conjunction with the existing monotonic-input-growth and strict-monotonic-output guards, so edge cases (null tokens, length mismatch, <5 calls) fall through to 'no signal' via the existing guards at behavioral-metrics.ts:460-462. The retained true positive (fixture530: 157→75 over 7 calls) satisfies both new bounds (75 ≤ 1
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added src/trace-analyst/behavioral-metrics.test.ts

  •  { input: 1000, output: 784 },
    

🎯 Usefulness Audit

🟡 Aggregate claim text still says 'at least 3 serial model calls' after the minimum rose to 5 [problem-fit] ``

AGGREGATE_CLAIM['output-length-decay'] at behavioral-analyst.ts:42-43 emits the user-facing string '...across at least 3 serial model calls.' This PR raised OUTPUT_DECAY_MINIMUM_CALLS to 5, so the finding a reader sees now understates the real bound. It is the same surface this change touches, so sweep the '3' to '5' (and optionally note the ≥40% retained-length threshold) in the same commit. Does not gate shipping; the detector logic is correct, only the descriptive text is stale.


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260731T224935Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — b44898e4

Review health 100/100 · Reviewer score 86/100 · Confidence 65/100 · 5 findings (5 low)

glm deepseek deepseek-flash aggregate
Readiness 92 92 86 86
Confidence 65 65 65 65
Correctness 92 92 86 86
Security 92 92 86 86
Testing 92 92 86 86
Architecture 92 92 86 86

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision.

🟡 LOW Missing boundary test at the decay threshold — src/trace-analyst/behavioral-metrics.test.ts

The test suite covers 'too short' (3 calls, below minimum 5) and 'too shallow' (80% retained, above 60% cutoff). No test verifies the precise boundary: 5 calls with outputs [100, 90, 80, 70, 60] (first=100, last=60, ratio=0.60). The condition uses <= on OUTPUT_DECAY_MAXIMUM_RETAINED_FRACTION, so exactly 60% should fire. A boundary smoke test would catch an off-by-one if <= were ever changed to < or if floating point precision ever caused unexpected behavior. Low priority.

🟡 LOW Test comment misstates retained fraction; doc overstates 'most of its length' — src/trace-analyst/behavioral-metrics.test.ts

The shallowRun fixture produces outputs 100→80 (80% retained, 100 - index*5 for index 0..4), but the comment says it 'only reaches 70% retained'. The assertion is correct (80% > 60% so it must not fire), only the comment is wrong. Relatedly, the OUTPUT_DECAY_MAXIMUM_RETAINED_FRACTION doc comment (behavioral-metrics.ts:89) says 'Requiring the response to lose most of its length', but a 0.6 retained bar only requires losing 40% — 'most' is too strong. Both are documentation-only fixes.

🟡 LOW All-zero output run now fires decay with a degenerate 'shrank 0→0' message — src/trace-analyst/behavioral-metrics.ts

decayIsMaterial = last <= first * OUTPUT_DECAY_MAXIMUM_RETAINED_FRACTION evaluates 0 <= 0 as true when every output in a ≥5-call run is 0, so a run with outputs [0,0,0,0,0] and monotonically growing inputs fires 'output-length-decay' with detail 'LLM output tokens shrank 0→0 over 5 serial calls'. The replaced last < first guard returned false for 0 < 0, so this is a new edge-case regression (outputs of exactly 0 are kept by outputTokensOf since 0 is finite). Realistic for degraded/empty generations on some providers. Fix: require first > 0 in addition, e.g. decayIsMaterial = first > 0 && last <= first * OUTPUT_DECAY_MAXIMUM_RETAINED_FRACTION, or keep last < first alongside the fraction check.

🟡 LOW All-zero output run now fires output-length-decay (degenerate false positive at first=0) — src/trace-analyst/behavioral-metrics.ts

The old guard was last < first; the new one is last <= first * OUTPUT_DECAY_MAXIMUM_RETAINED_FRACTION. When first === 0, the outputIsMonotonic check (current <= previous) forces every output to be <= 0, i.e. all zero, so decayIsMaterial = 0 <= 0 * 0.6 = 0 <= 0 evaluates TRUE. Combined with monotonic input growth (e.g. inputs [100,200,300,400,500], outputs [0,0,0,0,0]), a run that produced zero output from the very first call now fires a signal whose name and detail ('shrank 0->0') imply decay FROM something. The old 0 < 0 rejected this. Verified reachable via standalone harness. Practical risk is low (a 5-call all-zero-output run is itself a severe failure worth surfacing, just mislabeled), and real telemetry with 0 output tokens is more likely a missing-usage artifact than a g

🟡 LOW Stale aggregate claim in consumer still says decay needs 'at least 3 serial model calls' — src/trace-analyst/behavioral-metrics.ts

This shot raises the decay call threshold to 5, but the user-visible aggregate claim built from signals — AGGREGATE_CLAIM['output-length-decay'] in src/analyst/behavioral-analyst.ts:43 — still renders '...across at least 3 serial model calls'. The same file's monotonic-input-growth claim (line 41, still 3) remains correct. The aggregate claim will be displayed to users with the wrong threshold; update it to 'at least 5' in the same change (or derive it from the exported constants to prevent drift).


tangletools · 2026-07-31T22:51:09Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 5 non-blocking findings — b44898e4

Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 1/1 planned shots over 2 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-31T22:51:09Z · immutable trace

@drewstone
drewstone merged commit 02f9d6a into main Jul 31, 2026
2 checks passed
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.

2 participants