fix: stop verify, reports and briefs from claiming more than pipe checked #1
4 changed files with 121 additions and 24 deletions
fix(guide,suggest): stop packing at the first entry that does not fit
Both packers skipped an entry that did not fit the budget and kept going, so a large top-ranked finding vanished while smaller, lower-ranked ones were packed in behind it - presented as the most important work. Packing now stops at the first entry that does not fit and counts it and everything after it as omitted. suggest's agent plan also reserved no room for its own omission note, so the note itself could push the plan over budget; it now reserves room for the largest possible note unless the entry is the last one. When no entry fit, the guide said "this header alone exceeds the budget" even when the header had room and it was the top entry that was too large. It now says which of the two it was, with the size.
commit
39b309af97
|
|
@ -371,17 +371,24 @@ Guidance buildGuidance({
|
|||
// a small overflow), so reserve room for it up front and keep the whole
|
||||
// artifact inside the budget instead of letting the note push it over.
|
||||
const noteReserve = 220;
|
||||
for (final group in groups) {
|
||||
if (selected.length >= maxFindings) {
|
||||
omitted += group.length;
|
||||
continue;
|
||||
// Length of the first entry that did not fit, for the all-omitted message.
|
||||
int? firstMissLength;
|
||||
for (var i = 0; i < groups.length; i++) {
|
||||
final block = selected.length >= maxFindings
|
||||
? null
|
||||
: _groupBlock(groups[i], selected.length + 1, policy);
|
||||
if (block == null ||
|
||||
runningLength + block.length + noteReserve > budgetChars) {
|
||||
// Stop at the first entry that does not fit. Skipping it and packing the
|
||||
// smaller, lower-ranked entries behind it presented them as the top of
|
||||
// the list while the finding that outranked them silently vanished.
|
||||
firstMissLength = block?.length;
|
||||
for (final rest in groups.skip(i)) {
|
||||
omitted += rest.length;
|
||||
}
|
||||
break;
|
||||
}
|
||||
final block = _groupBlock(group, selected.length + 1, policy);
|
||||
if (runningLength + block.length + noteReserve > budgetChars) {
|
||||
omitted += group.length;
|
||||
continue;
|
||||
}
|
||||
selected.add(group);
|
||||
selected.add(groups[i]);
|
||||
blocks.add(block);
|
||||
runningLength += block.length;
|
||||
}
|
||||
|
|
@ -404,8 +411,14 @@ Guidance buildGuidance({
|
|||
buffer.writeln('No new findings.');
|
||||
} else if (included == 0) {
|
||||
buffer.writeln(
|
||||
'All $omitted findings omitted: this header alone exceeds the '
|
||||
'$budgetChars-character budget. Raise --budget or use --format md.',
|
||||
runningLength + noteReserve > budgetChars
|
||||
? 'All $omitted findings omitted: the header alone '
|
||||
'($runningLength characters) leaves no room in the '
|
||||
'$budgetChars-character budget. Raise --budget or use --format md.'
|
||||
: 'All $omitted findings omitted: the top-ranked entry '
|
||||
'(${firstMissLength ?? 0} characters) does not fit in what the '
|
||||
'header leaves of the $budgetChars-character budget. Raise '
|
||||
'--budget or use --format md.',
|
||||
);
|
||||
} else if (omitted > 0) {
|
||||
buffer
|
||||
|
|
|
|||
|
|
@ -159,12 +159,20 @@ class WorkPlan {
|
|||
'${files.length} file(s). Work them in this order.',
|
||||
)
|
||||
..writeln();
|
||||
final actions = ordered;
|
||||
String omissionNote(int omitted) =>
|
||||
'$omitted further cause(s) omitted to stay within $budgetChars '
|
||||
'characters. Run `pipe suggest` for the full plan.\n';
|
||||
// Room for the omission note, which must never be the thing that pushes
|
||||
// the plan over budget. Sized for the largest possible count.
|
||||
final noteReserve = omissionNote(actions.length).length;
|
||||
var omitted = 0;
|
||||
var included = 0;
|
||||
for (final action in ordered) {
|
||||
for (var i = 0; i < actions.length; i++) {
|
||||
final action = actions[i];
|
||||
if (included >= maxActions) {
|
||||
omitted++;
|
||||
continue;
|
||||
omitted = actions.length - i;
|
||||
break;
|
||||
}
|
||||
final block = StringBuffer()
|
||||
..writeln(
|
||||
|
|
@ -183,19 +191,19 @@ class WorkPlan {
|
|||
'${action.targets.length > 6 ? ', +${action.targets.length - 6} more' : ''})'}'
|
||||
'; verify with `${action.verify}`',
|
||||
);
|
||||
if (buffer.length + block.length > budgetChars) {
|
||||
omitted++;
|
||||
continue;
|
||||
final isLast = i == actions.length - 1;
|
||||
if (buffer.length + block.length + (isLast ? 0 : noteReserve) >
|
||||
budgetChars) {
|
||||
// Stop at the first cause that does not fit rather than skipping it and
|
||||
// packing lower-ranked ones behind it, which would present a
|
||||
// lower-ranked cause as if it outranked the one that was dropped.
|
||||
omitted = actions.length - i;
|
||||
break;
|
||||
}
|
||||
buffer.write(block);
|
||||
included++;
|
||||
}
|
||||
if (omitted > 0) {
|
||||
buffer.writeln(
|
||||
'$omitted further cause(s) omitted to stay within $budgetChars '
|
||||
'characters. Run `pipe suggest` for the full plan.',
|
||||
);
|
||||
}
|
||||
if (omitted > 0) buffer.write(omissionNote(omitted));
|
||||
return buffer.toString();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -84,6 +84,51 @@ void main() {
|
|||
expect(tight.brief, contains('omitted'));
|
||||
});
|
||||
|
||||
test('packing stops at the first entry that does not fit', () {
|
||||
// An oversized top entry used to be skipped while lower-ranked entries were
|
||||
// packed in after it, and when nothing fit the brief blamed "this header
|
||||
// alone" even though the header had room.
|
||||
final report = FindingReport(
|
||||
root: analysis.project.root,
|
||||
findings: [
|
||||
Finding(
|
||||
rule: ruleEventWithoutHandler,
|
||||
severity: PipeSeverity.error,
|
||||
confidence: 'high',
|
||||
message: 'huge',
|
||||
file: 'lib/${'nested/' * 300}big.dart',
|
||||
line: 1,
|
||||
data: const {'event': 'Huge'},
|
||||
),
|
||||
for (var i = 0; i < 5; i++)
|
||||
Finding(
|
||||
rule: ruleDeadHandler,
|
||||
severity: PipeSeverity.info,
|
||||
confidence: 'high',
|
||||
message: 'small',
|
||||
file: 'lib/f$i.dart',
|
||||
line: 1,
|
||||
data: {'event': 'Small$i'},
|
||||
),
|
||||
],
|
||||
suppressed: const [],
|
||||
);
|
||||
final brief = buildGuidance(
|
||||
root: analysis.project.root,
|
||||
flow: analysis.flow,
|
||||
report: report,
|
||||
budgetChars: 2500,
|
||||
);
|
||||
// No ranked entry at all: the smaller ones must not stand in for the top.
|
||||
expect(
|
||||
RegExp(r'^\d+\. \[', multiLine: true).hasMatch(brief.brief),
|
||||
isFalse,
|
||||
);
|
||||
expect(brief.omittedFindings, 6);
|
||||
expect(brief.brief, contains('the top-ranked entry'));
|
||||
expect(brief.brief, isNot(contains('header alone')));
|
||||
});
|
||||
|
||||
test('does not claim completeness when it truncates', () {
|
||||
final full = guidance(budget: 200000, maxFindings: 500);
|
||||
expect(full.omittedFindings, 0);
|
||||
|
|
|
|||
|
|
@ -278,4 +278,35 @@ void main() {
|
|||
expect(block, contains('PARTIAL'));
|
||||
expect(plan.toJson()['coverage'], coverage.toJson());
|
||||
});
|
||||
|
||||
test('packing stops at the first cause that does not fit', () {
|
||||
// The top cause is too large for the budget. It used to be skipped while
|
||||
// smaller, lower-ranked causes were packed in behind it, so the plan read
|
||||
// as if they were the most important work.
|
||||
final plan = buildWorkPlan(
|
||||
root: '/root',
|
||||
report: _report([
|
||||
_finding(
|
||||
rule: ruleEventWithoutHandler,
|
||||
severity: PipeSeverity.error,
|
||||
file: 'lib/${'nested/' * 80}big.dart',
|
||||
line: 1,
|
||||
data: {'event': 'Huge'},
|
||||
),
|
||||
for (var i = 0; i < 5; i++)
|
||||
_finding(
|
||||
rule: ruleDeadHandler,
|
||||
severity: PipeSeverity.info,
|
||||
file: 'lib/f$i.dart',
|
||||
line: 1,
|
||||
data: {'event': 'Small$i'},
|
||||
),
|
||||
]),
|
||||
);
|
||||
expect(plan.ordered.first.label, 'Huge');
|
||||
final block = plan.toAgentBlock(budgetChars: 500, maxActions: 10);
|
||||
expect(block, isNot(contains('Small')));
|
||||
expect(block, contains('6 further cause(s) omitted'));
|
||||
expect(block.length, lessThanOrEqualTo(500));
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue