Skip to content

fix: render {{thresholds.X}} in rule text and merge thresholds into extra on the preview path - #361

Open
Mark Warren (ZeSnarf) wants to merge 1 commit into
microsoft:mainfrom
PharSideLabs:pr/template-thresholds
Open

Mark Warren (ZeSnarf) wants to merge 1 commit into
microsoft:mainfrom
PharSideLabs:pr/template-thresholds

Conversation

@ZeSnarf

@ZeSnarf Mark Warren (ZeSnarf) commented Nov 3, 2026 •

Copy link
Copy Markdown

Description

Four built-in rules (instruction-bloat, mcp-tool-bloat, excessive-file-context and verbose-prompt-no-compression) use {{thresholds.X}} in their "When Triggered" text. Template variables only carried count, total, ratio, pct and extra, so those numbers rendered blank in the finding text.

This PR:

  • adds an optional thresholds field to RuleTemplateVars and fills it from the rule's thresholds in both emissionToAntiPattern (detector registry) and evaluateRule (Rule Editor preview);
  • makes evaluateRule merge thresholds into extra the 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 check passes (typecheck + lint + spellcheck + knip + tests)
  • Changes are covered by tests (src/core/rule-template-thresholds.test.ts; the rule-engine.test.ts expectation for templateVars now includes thresholds)
  • Documentation updated (if applicable): not needed, {{thresholds.X}} is already used by the shipped rules

…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>
@ZeSnarf

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@san360 Sanjay Singh (san360) 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.

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-compression and mcp-tool-bloat render 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 of verbose-prompt-no-compression tuned to minMessageLength: 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 example lazy-prompting ("under characters"), mega-sessions, runaway-agent-loops, late-night-coding and verbose-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 main and 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 to Object.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:

  1. A test for the detector path. The one-line change in detector-registry.ts drives the Dashboard text most users see, but all three new tests go through evaluateRule (the preview). There's an inline note on the test file with a small test that covers it.

  2. A line in the authoring docs. Rule authors will now rely on {{thresholds.X}}, but docs/AUTHORING_RULES.md (the When Triggered section) lists only {{count}}, {{total}}, {{pct}} and extra.<key>. Something like "and the rule's thresholds as {{thresholds.<key>}}" would help, and so would the same words in the descriptionTemplate comment in src/core/types/rule-types.ts and the When Triggered hint in src/webview/dsl-cheatsheet.ts. One thing worth noting there: the template formats non-integer numbers with one decimal, so a fractional threshold such as maxRatio: 0.005 would render as 0.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', () => {

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.

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)');
});

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