Repository navigation
fix(loop-context): refuse a similarity threshold that switches stagnation off - #643
Open
THRISHAL12345 wants to merge 1 commit into
Open
THRISHAL12345 wants to merge 1 commit into
THRISHAL12345 wants to merge 1 commit into
Conversation
`--similarity-threshold 95` (meant as 95%) was accepted. Similarity is at
most 1.0, so above 1 no two errors ever match: stagnation and the repeated-
action rule never fire, and pruning stops collapsing repeats. Four identical
failures came back CONTINUE, exit 0, where the default escalates. 1.5 and
Infinity did the same. Only no-progress and the iteration cap were left as
backstops.
The CLI now takes a fraction in (0, 1] and says what to use when the value
looks like a percentage ("If you meant 95%, use 0.95"). The library
validates too: checkCircuitBreaker, pruneLedger, summarizeAttempts and
buildContextInjection throw on a similarity threshold outside (0, 1], or on
a non-positive or fractional count, instead of quietly never escalating.
validateBreakerConfig / validatePruneConfig are exported.
loop-drill reports a config loop-context rejects as a failed breaker.config
drill rather than crashing. Its trigger-attribution tests used 95 and 0 to
fake a breaker that fires the wrong rule; they now pass a stand-in breaker.
Contributor
|
This PR changes paths that must run the real Fork PRs from first-time contributors start with those workflows waiting for approval. A maintainer needs to open the Checks tab and click Approve and run workflows. Until that happens, branch protection will show the PR as blocked even after a review. Content-only PRs ( — loop-engineering fork-pr-gate |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
loop-context --similarity-threshold 95is accepted. Someone writing95almost certainly means 95%, but the value is a fraction, andcalculateSimilarity()never returns more than 1.0. Above 1, no two errors can ever count as "the same error":At those values:
--prunestops collapsing repeated errors, so the ledger fed back to the agent grows instead.Only the no-progress rule (5 consecutive failures by default) and the iteration cap are left to stop it.
The CLI already refuses integer flags that would "silently disable the breaker", as
parsePositiveIntFlag's comment puts it. The float check only rejected NaN and values ≤ 0, so the most natural typo got through. The library took any number too:checkCircuitBreaker(ledger, { ...DEFAULT_BREAKER, similarityThreshold: 95 })failed the same way.What this changes
CLI:
--similarity-thresholdtakes a fraction in (0, 1]. Anything else exits1with the reason, and a whole-number percentage gets a direct suggestion:The help text and the README's flag table say so too. The README was missing this flag entirely.
Library:
checkCircuitBreaker,pruneLedger,summarizeAttemptsandbuildContextInjectionnow throw on a config that would switch a rule off:maxIterations, the three thresholds,tokenBudget,window,maxTraceLines.A breaker that refuses to start is loud. One that silently never breaks is the failure this package exists to prevent.
validateBreakerConfig/validatePruneConfigare exported, so callers can check a config up front.loop-drill: a config thatloop-contextrejects is now a failedbreaker.configdrill with the library's message, not a crash. It's still exit 2, so CI still fails. Four of its tests had used95and0as a trick to simulate a breaker that fires the wrong rule. Valid configs can't produce that any more, which is the point of this fix. SorunBreakerDrillstakes an optional breaker function, and those tests pass a stand-in that always answers with one trigger. That keeps the "credit only the rule under test" logic covered without relying on a config the library now refuses.Why exit 1
Exit 1 is
loop-context's documented "error" code (0continue ·2escalate ·1error). The README's control-flow example,loop-context --check … || { …; exit 2; }, stops on any non-zero exit. The existing integer-flag rejections already use 1.Verification
loop-contextgoes from 53 to 62: 4 new CLI tests and 5 new library tests. They cover 95, 100, 1.5 and Infinity being rejected with no decision printed, 0, −0.5 andabcbeing rejected, the boundaries (1, 0.95, 0.5, 0.01 andNumber.MIN_VALUEall still trip stagnation),--prunevalidation, every config-taking function, and when the percentage hint appears.loop-drillis at 58.mcp-serverpasses 28/28. Itsloop_check_breakerusesDEFAULT_BREAKER, so it isn't affected.loop-context(10 guards): validation incheckCircuitBreaker,pruneLedgerandsummarizeAttempts, the upper bound, the lower bound, integers only, thetokenBudgetcheck, thefrustrationThresholdcheck, the CLI flag check, and the hint only for whole-number percentages.loop-drill(2 guards): the rejected-config catch and the stand-in breaker being used.isFinitecheck. The range comparison already rejects NaN and Infinity, so I removed it rather than keep dead code.ci-validate-gates.shexits 0 with 340 tests passing and none failing, including theloop-drilldogfood run against this repo's breaker.ci-audit-gates.shpasses with a reference score of 100. On Windows I applied fix: guard loops against prompt injection from untrusted input #641's one-linegithub-triage.test.mjspath fix for the local run only; it isn't in this PR.Notes
tools/. I haven't bumpedloop-context's version, so you can pick patch or minor.loop-drill, but incli.tsand new files. This PR only changesrunBreakerDrillsindrill.ts, andgit merge-treemerges this branch cleanly with both.