Skip to content

fix: two review defects — dead-end detector silent on alternating calls, recovery metric counted deflection - #1374

Merged
Lexus2016 merged 4 commits into
mainfrom
fix/verification-round-1
Jul 27, 2026
Merged

fix: two review defects — dead-end detector silent on alternating calls, recovery metric counted deflection#1374
Lexus2016 merged 4 commits into
mainfrom
fix/verification-round-1

Conversation

@Lexus2016

Copy link
Copy Markdown
Owner

Two defects found in post-merge review of work merged earlier today. Both are in code I wrote or repaired, both are confirmed by reproduction, and both would have degraded the signal the evolution loop runs on.

Independently flagged by a cross-provider review (Gemini via consilium) and then reproduced locally before fixing.


1. Dead-end detector went silent on alternating calls — agent/loop_guard.py

Repairing #1349 I added a liveness requirement: the repeated signature must be re-issued by the latest tool turn. That part is right — 5x terminal then a read_file, or 8x terminal then a text reply, count as progress everywhere else in that module (TestRunBoundaries).

What was wrong is the order of operations. The code ranked the window with most_common(1) and then required that winner to be live. When the agent alternates between two stuck calls, those two conditions select different signatures:

pytest, ruff, pytest, ruff, pytest, ruff, pytest, ruff   (each x4)
  most_common(1) -> "pytest"   (insertion order breaks the tie)
  live signature -> "ruff check"
  -> no match -> SILENT

Both calls are four-deep in identical re-issues. That is exactly the dead end #1312 exists to catch, and it went unreported.

Fix: look up the live signature first, gate on its own count. Liveness requirement unchanged.

New TestAlternatingCallsAreStillDeadEnds covers: fires at threshold, quiet below it, and names the live call rather than the most-common one.


2. Refusal-recovery counted deflection and explanation — scripts/introspection_extract.py

The pivot clause in _RECOVERY_RE allowed second person. So handing the work back to the human counted as a recovery:

Text Was counted Actually
"I can't access that URL, however you can try visiting it yourself." recovered deflection
"I cannot do that. Instead, please contact support." recovered deflection
"I don't have permission. But you will need to run it manually." recovered deflection
"I can't do that, but I can tell you why it failed." recovered explanation, not an alternative path

This inverts the metric: it improves whenever the agent gets more polite about giving up — the precise behaviour #1327 exists to eliminate.

Fix: pivot clause is first-person only, plus a _NON_RECOVERY_RE veto for deflection (contact support, you can, yourself, manually) and explanation-instead-of-action (I can tell you why). Vetoing beats narrowing the recovery pattern — these shapes are open-ended, and under-counting a real recovery is far cheaper than a rate that rises when the agent gives up nicely. Also adds first-person phrasings the original missed (let me try, I'll write).

Measured impact on the real corpus

Same 4629 sessions, same 143 refusals:

before:  51 recovered   rate 0.3566
after:   31 recovered   rate 0.2168

20 of the 51 were false positives — the headline number was inflated by ~64%. The 21.7% figure is the one Child C (#1327 recovery dispatcher) should be measured against.


107 passing across the four loop-guard suites; 50 in test_introspection_extract.py (7 new). Ruff clean on all four files.

Refs #1312, #1366.

…the top one

Found in post-merge review of #1349. detect_dead_end_loop ranked the window with
`most_common(1)` and then required THAT winner to be the call the latest turn
re-issued. When the agent alternates between two stuck calls the two conditions
select different signatures and the detector goes silent on a textbook dead end.

Reproduced: `pytest` and `ruff check` each re-issued 4 times, alternating. The
counter returns `pytest` (insertion order breaks the tie), the live signature is
`ruff check`, they do not match, no nudge — even though both calls are four-deep
in identical re-issues.

The liveness requirement itself is right and stays: 5x terminal followed by a
read_file, or 8x terminal followed by a text reply, are progress everywhere else
in this module (TestRunBoundaries), and a multi-tool turn is varied work. What
was wrong is ranking first and checking liveness second. Now the live signature
is looked up first and gated on ITS OWN count.

Adds TestAlternatingCallsAreStillDeadEnds: fires at threshold, quiet below it,
and reports the live call rather than the most-common one.

107 passing across test_loop_guard.py, test_loop_guard_readfile_debounce.py,
test_dead_end_loop.py and test_loop_guard_cron_enforcement.py; ruff clean.

Refs #1312
Found in post-merge review of #1366. The pivot clause allowed second person, so
handing the work back to the human counted as a recovery:

  "I can't access that URL, however you can try visiting it yourself."  -> counted
  "I cannot do that. Instead, please contact support."                  -> counted
  "I don't have permission. But you will need to run it manually."      -> counted
  "I can't do that, but I can tell you why it failed."                  -> counted

None of these is the agent taking another path, and the last is an offer to
describe the problem rather than solve it. The metric therefore improved
whenever the agent got more polite about giving up — the inverse of what #1327
is trying to measure.

The pivot clause is now first-person only, and a _NON_RECOVERY_RE veto covers
deflection ("contact support", "you can", "yourself", "manually") and
explanation-instead-of-action ("I can tell you why"). Vetoing beats narrowing
the recovery pattern: these shapes are open-ended, and under-counting a real
recovery is far cheaper than a rate that rises when the agent gives up nicely.
Also adds the first-person phrasings the original missed ("let me try",
"I'll write").

Measured on the same real corpus (4629 sessions, 143 refusals):
  before: 51 recovered, rate 0.3566
  after:  31 recovered, rate 0.2168
20 of the 51 were false positives — the headline number was inflated by ~64%.

50 passing in test_introspection_extract.py (7 new); ruff clean.

Refs #1366
@github-actions github-actions Bot added the fix Bug or fix label Jul 27, 2026
… guidance

Fact-checking my own #1362 docs against the code they describe found three
statements that were wrong. A skill file is executable guidance — an agent
acting on a threshold that does not exist produces a wrong verdict and re-opens
issues that were actually fixed.

1. classify_verdict_by_rate thresholds. The doc claimed "a rate rise >20% is
   regressed, a drop >5% is confirmed, anything between is stable". The function
   has no 5% branch and no stable state: it returns `regressed` when the rate
   rose more than 20% over baseline and `confirmed` in every other case, with
   `no-signal` only when a rate is missing. Three ledger values exist, not four.

2. Shape of failure_rate. Described as "failures / sessions_scanned", implying a
   scalar. It is a dict keyed by tool, exactly like tool_failures — an agent
   reading it as one number would compare a dict against a float.

3. Reason bucket name. The taxonomy emits `quota/billing`; the doc listed
   `quota`, so an exact-match lookup finds nothing.

The record-merge example was verified empirically rather than by reading: the
5th argument after the subcommand does land in `baseline_failure_rate` (checked
by running the documented command and reading the ledger line back). Unchanged.

22 passing in test_evolution_skill_integrity.py; 60 with
test_evolution_realized_impact.py alongside it.

Refs #1362
@Lexus2016
Lexus2016 force-pushed the fix/verification-round-1 branch from 2470509 to 21b98af Compare July 27, 2026 18:31
@Lexus2016

Copy link
Copy Markdown
Owner Author

Third and fourth defects found while fact-checking my own docs from #1362 against the code they describe. Both now in this PR (commit 21b98af99).

3. A threshold that does not exist. The introspection SKILL.md claimed classify_verdict_by_rate() implements "a rate rise >20% is regressed, a drop >5% is confirmed, anything between is stable". Reading the function:

rel_delta = (current_rate - baseline_rate) / baseline_rate
return "regressed" if rel_delta > regression_threshold else "confirmed"

There is no 5% branch and no stable verdict. Three ledger values exist, not four. A skill file is executable guidance — an agent looking for a stable band that isn't there records the wrong verdict, and a wrong regressed re-opens issues that were actually fixed.

4. Two smaller inaccuracies in the same block:

  • failure_rate was described as "failures ÷ sessions_scanned", implying a scalar. It is a dict keyed by tool, like tool_failures.
  • The reason bucket is emitted as quota/billing; the doc listed quota, so an exact-match lookup finds nothing.

Verified good: the record-merge example. Rather than counting positionals by eye I ran the documented command and read the ledger line back — 0.42 lands in baseline_failure_rate. Unchanged.

Running total for this verification round: 4 defects, all in code or docs I merged today, all reproduced before fixing.

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

Labels

fix Bug or fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant