Repository navigation
fix: render {{thresholds.X}} in rule text and merge thresholds into extra on the preview path - #361
Mark Warren (ZeSnarf) wants to merge 1 commit into
Conversation
…xtra on the preview path
instruction-bloat, mcp-tool-bloat, excessive-file-context and verbose-prompt-no-compression use
{{thresholds.X}} in their When Triggered text, but template variables only carried count, total,
ratio, pct and extra, so those numbers rendered blank. evaluateRule (the Rule Editor preview) also
did not merge thresholds into extra the way the detector registry does, so {{extra.<threshold>}}
rendered blank there.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@microsoft-github-policy-service agree |
Sanjay Singh (san360)
left a comment
There was a problem hiding this comment.
Thank you Mark Warren (@ZeSnarf) for tracking this down and for the clear write-up! The blank numbers in "larger than bytes" and "≥ chars" were easy to miss, and lining up the Rule Editor preview with the detector path is the right fix. The diff is small and focused, and the new test file reads nicely.
What we verified (current main with this PR merged):
- Dashboard / Anti-Patterns findings: on
main,instruction-bloat,excessive-file-context,verbose-prompt-no-compressionandmcp-tool-bloatrender an empty{{thresholds.X}}("larger than bytes", "attached ≥ files", "of length ≥ chars"). With the PR they show 4000, 30, 800 and 40. With a personal copy ofverbose-prompt-no-compressiontuned tominMessageLength: 200, the finding text follows the tuned value. We checked this on real Claude Code sessions in throwaway repos and on synthetic data. - Rule Editor → Test Rule: on
main, every rule that reads a threshold as{{extra.X}}comes out blank in the preview. That's 8 built-in rules, for examplelazy-prompting("under characters"),mega-sessions,runaway-agent-loops,late-night-codingandverbose-output. With the PR they match the Dashboard text, and so do the four{{thresholds.X}}rules. - No other changes: every other page's text is identical between
mainand the PR on the same data. Markup in a personal rule's text still renders as plain text, and{{thresholds.__proto__}}-style lookups only read values. Nothing gets written toObject.prototype. - Your new tests fail on
main's source and pass with the fix. Typecheck, lint, knip, lockfile-lint, the full unit suite, build and bundle size all pass locally, and so does the Playwright suite. It also merges cleanly with the other open rules PR (#373), and the combined tests pass.
Two small suggestions:
-
A test for the detector path. The one-line change in
detector-registry.tsdrives the Dashboard text most users see, but all three new tests go throughevaluateRule(the preview). There's an inline note on the test file with a small test that covers it. -
A line in the authoring docs. Rule authors will now rely on
{{thresholds.X}}, butdocs/AUTHORING_RULES.md(the When Triggered section) lists only{{count}},{{total}},{{pct}}andextra.<key>. Something like "and the rule's thresholds as{{thresholds.<key>}}" would help, and so would the same words in thedescriptionTemplatecomment insrc/core/types/rule-types.tsand the When Triggered hint insrc/webview/dsl-cheatsheet.ts. One thing worth noting there: the template formats non-integer numbers with one decimal, so a fractional threshold such asmaxRatio: 0.005would render as0.0. None of the shipped rules print a fractional threshold, so this is just good to know.
Neither point needs to block the PR. Since this comes from a fork, a maintainer will approve the CI run. Thanks again for the fix and the tests. We'd love to see more contributions like this!
| expect(result.description).toBe(`limit ${rule.thresholds[key]}`); | ||
| }); | ||
|
|
||
| it('lets the detector extra win over a threshold of the same name', () => { |
There was a problem hiding this comment.
This test passes on main too: there evaluateRule used only emission.extra, so 77 comes out with or without the change, and the test doesn't pin which value wins. The one-line change in detector-registry.ts (the Dashboard / Anti-Patterns text) isn't covered by any test yet.
One idea that covers both, using a shipped rule whose text has a threshold and a measured value with the same name. It fails on main ("larger than bytes") and passes with this PR:
import { detectAntiPatterns } from './detector-registry';
import { createRequest, createSession } from './parser-shared';
it('renders {{thresholds.X}} in a finding, with the measured {{extra.X}} of the same name (instruction-bloat)', () => {
const requests = [createRequest({ messageText: 'Explain the parser', responseText: 'ok', timestamp: Date.UTC(2026, 9, 1) })];
const sessions = [createSession({ sessionId: 's', workspaceId: 'ws', workspaceName: 'ws', harness: 'Local Agent', customInstructionsBytes: 9000, requests })];
const finding = detectAntiPatterns(requests, sessions, false).patterns.find(p => p.id === 'instruction-bloat');
expect(finding?.description).toContain('larger than 4000 bytes (largest: 9000 bytes)');
});
Description
Four built-in rules (
instruction-bloat,mcp-tool-bloat,excessive-file-contextandverbose-prompt-no-compression) use{{thresholds.X}}in their "When Triggered" text. Template variables only carriedcount,total,ratio,pctandextra, so those numbers rendered blank in the finding text.This PR:
thresholdsfield toRuleTemplateVarsand fills it from the rule's thresholds in bothemissionToAntiPattern(detector registry) andevaluateRule(Rule Editor preview);evaluateRulemerge thresholds intoextrathe same way the detector registry already does, so{{extra.<threshold>}}also renders in the preview instead of coming out blank.No detection logic changes; only the values available to the text templates.
Related Issues
None filed; found while customising rule wording in a fork.
Checklist
npm run checkpasses (typecheck + lint + spellcheck + knip + tests)src/core/rule-template-thresholds.test.ts; therule-engine.test.tsexpectation fortemplateVarsnow includesthresholds){{thresholds.X}}is already used by the shipped rules