mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-11 21:57:14 +03:00
Fix two Bugbot findings: Cursor prefix budget and footer-cutting tail slice
Both flagged by Cursor Bugbot on PR #508 after the main merge; both real. 1. cursorBlockMessage computed min(maxChars, 4000 - prefix), so a configured maxChars at or below the Cursor ceiling never charged the BLOCK_PREFIX against the budget: the final deny text could exceed maxChars by the prefix length, and appendDesignSystemNoteOnce's size check lost exactly the room designNoteReserve had held back. The prefix now comes off whichever limit binds. Default-config behavior is unchanged (min(8000, 4000) - 60 equals the old 4000 - 60). 2. The note reservation is subtracted after renderTemplate's 500-char floor, so the clamp can run below the budget clampLastLine assumed safe, and its last-resort path tail-sliced the rendered text, cutting the policy footer (the failure mode this PR exists to eliminate) when a deep file path met a pending DESIGN.md note. The reservation order stays (the staleness-note delivery guarantee at floor budgets depends on it); the last resort now drops the finding line and clips the head instead, so the footer survives every path. New regression test pins it: 6 findings, 100-char path, maxChars 500, reserveChars 134. Prepared with AI assistance (Claude Code). Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -358,10 +358,16 @@ const BLOCK_PREFIX = 'Impeccable design hook blocked this write before it landed
|
||||
|
||||
function cursorBlockMessage(findings, filePath, config, cwd, footerMode, reserveChars) {
|
||||
const limits = config?.limits || DEFAULT_CONFIG.limits;
|
||||
// Subtract the prefix from whichever limit binds, not just the Cursor
|
||||
// ceiling: with a configured maxChars at or below the ceiling, the old
|
||||
// `min(maxChars, ceiling - prefix)` never charged the prefix against the
|
||||
// budget, so the final deny text could exceed maxChars by the prefix
|
||||
// length and appendDesignSystemNoteOnce's size check lost exactly the
|
||||
// room designNoteReserve had held back (Bugbot on PR #508).
|
||||
const budget = Math.min(
|
||||
limits.maxChars || DEFAULT_CONFIG.limits.maxChars,
|
||||
CURSOR_DENY_LIMIT - BLOCK_PREFIX.length,
|
||||
);
|
||||
CURSOR_DENY_LIMIT,
|
||||
) - BLOCK_PREFIX.length;
|
||||
const rendered = renderTemplate(findings, filePath,
|
||||
{ ...config, limits: { ...limits, maxChars: budget } },
|
||||
{ cwd, footer: footerMode, reserveChars });
|
||||
|
||||
@@ -975,7 +975,10 @@ export function renderTemplate(findings, filePath, config, opts = {}) {
|
||||
const cap = Math.max(1, limits.maxFindings || DEFAULT_CONFIG.limits.maxFindings);
|
||||
// reserveChars holds back room for a note the caller appends after render
|
||||
// (the DESIGN.md staleness note), so the final payload stays inside the
|
||||
// configured budget.
|
||||
// configured budget. It comes off after the 500-char floor, so at floor
|
||||
// configs the note keeps guaranteed delivery room; the clamp budget can
|
||||
// therefore sit below 500, which clampLastLine's footer-preserving
|
||||
// fallback handles (Bugbot on PR #508).
|
||||
const maxChars = Math.max(500, limits.maxChars || DEFAULT_CONFIG.limits.maxChars) - (opts.reserveChars || 0);
|
||||
|
||||
const cwd = opts.cwd || process.cwd();
|
||||
@@ -1115,15 +1118,22 @@ function clampToBudget(header, lines, more, footer, maxChars) {
|
||||
// whatever happened to be last, which was always the footer.
|
||||
function clampLastLine(build, line, maxChars) {
|
||||
const footerText = directiveFooter({ mode: 'short' });
|
||||
const bare = build([], footerText);
|
||||
// +1 for the newline the line itself brings when it joins the blocks.
|
||||
const room = maxChars - build([], footerText).length - 1;
|
||||
const room = maxChars - bare.length - 1;
|
||||
if (room >= 24) {
|
||||
const clipped = line.length > room ? `${line.slice(0, room - 1)}…` : line;
|
||||
return build([clipped], footerText);
|
||||
}
|
||||
// maxChars is floored at 500 and header + short footer fit well inside
|
||||
// that, so this is unreachable; keep the hard slice as the safety net.
|
||||
return `${build([line], footerText).slice(0, maxChars - 1)}…`;
|
||||
// No room for even a clipped finding line: the note reservation can pull
|
||||
// the budget below the 500-char floor, and a deep file path can push the
|
||||
// header past what remains beside the short policy (Bugbot on PR #508).
|
||||
// Drop the line, and if the bare header + policy still overflow, clip the
|
||||
// head. Never tail-slice: the footer sits at the end, so a tail slice is
|
||||
// exactly the footer cut this renderer exists to prevent.
|
||||
if (bare.length <= maxChars) return bare;
|
||||
const head = bare.slice(0, Math.max(0, maxChars - footerText.length - 4));
|
||||
return `${head}…\n\n${footerText}`;
|
||||
}
|
||||
|
||||
// `compact` drops the registry description: within one emission the first
|
||||
|
||||
@@ -1224,6 +1224,24 @@ describe('renderTemplate()', () => {
|
||||
assert.match(renderFor('win32'), /ignore-value overused-font "Space Grotesk Var"/);
|
||||
});
|
||||
|
||||
it('keeps the policy footer when reserveChars presses against the 500-char floor', () => {
|
||||
// Bugbot on PR #508: the note reservation used to be subtracted after
|
||||
// the 500-char floor, so the clamp could run at ~366 chars, below the
|
||||
// budget clampLastLine assumes safe, and the hard tail slice cut the
|
||||
// policy footer. The reserve now comes off before the floor; when the
|
||||
// floor wins, the note defers instead.
|
||||
const config = { ...DEFAULT_CONFIG, limits: { ...DEFAULT_CONFIG.limits, maxChars: 500 } };
|
||||
const longPath = `/x/${'deeply-nested/'.repeat(6)}Component.tsx`;
|
||||
const text = renderTemplate(
|
||||
Array.from({ length: 6 }, (_, i) =>
|
||||
finding('side-tab', i + 1, { name: 'Side tab', description: 'Colored side border.' })),
|
||||
longPath, config, { cwd: '/x', reserveChars: 134 }
|
||||
);
|
||||
assert.ok(text.length <= 500, `stays inside the floored budget (got ${text.length})`);
|
||||
assert.match(text, /unsure, ask in one line\.$/);
|
||||
assert.doesNotMatch(text, /…$/);
|
||||
});
|
||||
|
||||
it('drops the L<line> prefix when line is 0', () => {
|
||||
const text = renderTemplate(
|
||||
[finding('side-tab', 0, { name: 'X' })],
|
||||
|
||||
Reference in New Issue
Block a user