fix(guide,suggest): word agent guidance no stronger than the evidence #2
3 changed files with 139 additions and 11 deletions
fix(suggest): frame the agent plan as a ranking and name every target
Three ways the agent plan claimed more than it knew:
- It opened with "Work them in this order" and nothing else. It now opens with
"Static heuristics; confirm each against the cited file. Order is a score,
not a mandate."
- Under the default wire-up policy a dead handler still scored "no product
judgement needed (+15)". Wiring one up means deciding which control should
dispatch the event, which is product judgement; the bonus now applies only
under --policy remove, and buildWorkPlan takes the policy.
- A grouped action used its first finding's remediation, so a union with five
dead handlers said "dispatch LoadItems from the UI" and nothing about the
other four. A group now gets an action built from all of its targets
("dispatch each of A, B, C ..."), and targets include the variant and member
lists that grouped findings carry.
commit
138baad67c
|
|
@ -647,7 +647,11 @@ class SuggestCommand extends Command<int> {
|
||||||
policy: _readPolicy(results),
|
policy: _readPolicy(results),
|
||||||
);
|
);
|
||||||
try {
|
try {
|
||||||
final plan = buildWorkPlan(root: project.root, report: analysis.report);
|
final plan = buildWorkPlan(
|
||||||
|
root: project.root,
|
||||||
|
report: analysis.report,
|
||||||
|
policy: _readPolicy(results),
|
||||||
|
);
|
||||||
// Read every numeric option up front. Each was previously read inside the
|
// Read every numeric option up front. Each was previously read inside the
|
||||||
// one output branch that uses it, so `suggest --top abc --format agent`
|
// one output branch that uses it, so `suggest --top abc --format agent`
|
||||||
// silently ignored the malformed value instead of rejecting it.
|
// silently ignored the malformed value instead of rejecting it.
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,5 @@
|
||||||
import 'analysis.dart';
|
import 'analysis.dart';
|
||||||
|
import 'guide.dart';
|
||||||
import 'rules.dart';
|
import 'rules.dart';
|
||||||
|
|
||||||
/// One fixable root cause: a single action that resolves one or more findings.
|
/// One fixable root cause: a single action that resolves one or more findings.
|
||||||
|
|
@ -159,6 +160,7 @@ class WorkPlan {
|
||||||
final buffer = StringBuffer()
|
final buffer = StringBuffer()
|
||||||
..writeln('# pipe plan — $root')
|
..writeln('# pipe plan — $root')
|
||||||
..writeln()
|
..writeln()
|
||||||
|
..writeln(agentCaution)
|
||||||
..write(coverage == null ? '' : '${coverage!.describe()}\n')
|
..write(coverage == null ? '' : '${coverage!.describe()}\n')
|
||||||
..writeln(
|
..writeln(
|
||||||
'$totalFindings findings collapse to $totalCauses root cause(s) across '
|
'$totalFindings findings collapse to $totalCauses root cause(s) across '
|
||||||
|
|
@ -245,6 +247,11 @@ const Map<String, double> _confidenceFactor = {
|
||||||
'low': 0.4,
|
'low': 0.4,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/// Opens the agent plan: the order is a heuristic ranking, not an instruction.
|
||||||
|
const String agentCaution =
|
||||||
|
'Static heuristics; confirm each against the cited file. Order is a score, '
|
||||||
|
'not a mandate.';
|
||||||
|
|
||||||
/// Rules whose fix needs no product judgement, so they can just be done.
|
/// Rules whose fix needs no product judgement, so they can just be done.
|
||||||
const Set<String> _certaintyRules = {
|
const Set<String> _certaintyRules = {
|
||||||
ruleDeadHandler,
|
ruleDeadHandler,
|
||||||
|
|
@ -253,17 +260,24 @@ const Set<String> _certaintyRules = {
|
||||||
ruleLoadingFlagNotReset,
|
ruleLoadingFlagNotReset,
|
||||||
};
|
};
|
||||||
|
|
||||||
int _scoreOf(List<Finding> group) {
|
/// Whether [rule]'s fix needs no product judgement under [policy]. Removing a
|
||||||
|
/// dead handler needs none; wiring one up - the default - means deciding which
|
||||||
|
/// control should dispatch the event, which is exactly product judgement.
|
||||||
|
bool _isCertain(String rule, FixPolicy policy) =>
|
||||||
|
_certaintyRules.contains(rule) &&
|
||||||
|
!(rule == ruleDeadHandler && policy == FixPolicy.wireUp);
|
||||||
|
|
||||||
|
int _scoreOf(List<Finding> group, FixPolicy policy) {
|
||||||
final head = group.first;
|
final head = group.first;
|
||||||
final base = _severityBase[head.severity] ?? 10;
|
final base = _severityBase[head.severity] ?? 10;
|
||||||
final factor = _confidenceFactor[head.confidence] ?? 1.0;
|
final factor = _confidenceFactor[head.confidence] ?? 1.0;
|
||||||
final breadth = ((group.length - 1).clamp(0, 5)) * 10;
|
final breadth = ((group.length - 1).clamp(0, 5)) * 10;
|
||||||
final uxCritical = head.data['uxCritical'] == true ? 25 : 0;
|
final uxCritical = head.data['uxCritical'] == true ? 25 : 0;
|
||||||
final certainty = _certaintyRules.contains(head.rule) ? 15 : 0;
|
final certainty = _isCertain(head.rule, policy) ? 15 : 0;
|
||||||
return (base * factor).round() + breadth + uxCritical + certainty;
|
return (base * factor).round() + breadth + uxCritical + certainty;
|
||||||
}
|
}
|
||||||
|
|
||||||
String _whyOf(List<Finding> group, int score) {
|
String _whyOf(List<Finding> group, int score, FixPolicy policy) {
|
||||||
final head = group.first;
|
final head = group.first;
|
||||||
final base = _severityBase[head.severity] ?? 10;
|
final base = _severityBase[head.severity] ?? 10;
|
||||||
final parts = <String>[
|
final parts = <String>[
|
||||||
|
|
@ -273,8 +287,7 @@ String _whyOf(List<Finding> group, int score) {
|
||||||
'one fix clears ${group.length} findings (+'
|
'one fix clears ${group.length} findings (+'
|
||||||
'${((group.length - 1).clamp(0, 5)) * 10})',
|
'${((group.length - 1).clamp(0, 5)) * 10})',
|
||||||
if (head.data['uxCritical'] == true) 'user-visible failure (+25)',
|
if (head.data['uxCritical'] == true) 'user-visible failure (+25)',
|
||||||
if (_certaintyRules.contains(head.rule))
|
if (_isCertain(head.rule, policy)) 'no product judgement needed (+15)',
|
||||||
'no product judgement needed (+15)',
|
|
||||||
];
|
];
|
||||||
return '$score = ${parts.join(', ')}';
|
return '$score = ${parts.join(', ')}';
|
||||||
}
|
}
|
||||||
|
|
@ -308,6 +321,15 @@ List<String> _targetsOf(List<Finding> group) {
|
||||||
final targets = <String>[];
|
final targets = <String>[];
|
||||||
for (final finding in group) {
|
for (final finding in group) {
|
||||||
final data = finding.data;
|
final data = finding.data;
|
||||||
|
// Grouped findings name their members in a list (the variants of a union,
|
||||||
|
// the unread members of a notifier).
|
||||||
|
for (final key in const ['variants', 'members']) {
|
||||||
|
final values = data[key];
|
||||||
|
if (values is! List) continue;
|
||||||
|
for (final value in values.whereType<String>()) {
|
||||||
|
if (value.isNotEmpty && !targets.contains(value)) targets.add(value);
|
||||||
|
}
|
||||||
|
}
|
||||||
for (final key in const [
|
for (final key in const [
|
||||||
'variant',
|
'variant',
|
||||||
'event',
|
'event',
|
||||||
|
|
@ -328,6 +350,40 @@ List<String> _targetsOf(List<Finding> group) {
|
||||||
return targets;
|
return targets;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The action for a group. A single finding's remediation names that one
|
||||||
|
/// finding's target, so for a group of several it would name only the first -
|
||||||
|
/// "dispatch LoadItems from the UI" for a group of five handlers. A group gets
|
||||||
|
/// an action built from all of its targets instead.
|
||||||
|
String _actionOf(List<Finding> group, List<String> targets, FixPolicy policy) {
|
||||||
|
final head = group.first;
|
||||||
|
final single = head.remediation ?? head.message;
|
||||||
|
if (group.length < 2 || targets.length < 2) return single;
|
||||||
|
final list = targets.join(', ');
|
||||||
|
final owner = head.data['owner'];
|
||||||
|
final ownerName = owner is String ? owner : 'the bloc';
|
||||||
|
return switch (head.rule) {
|
||||||
|
ruleDeadHandler =>
|
||||||
|
policy == FixPolicy.wireUp
|
||||||
|
? 'Wire them up: dispatch each of $list from the control that should '
|
||||||
|
'trigger it, after checking none is dispatched from a directory '
|
||||||
|
'that was not scanned (--dir). Removing a handler needs explicit '
|
||||||
|
'approval.'
|
||||||
|
: 'Remove the handler for each of $list once nothing outside the '
|
||||||
|
'scanned directories dispatches it (--dir), or dispatch it from '
|
||||||
|
'the UI.',
|
||||||
|
ruleEventWithoutHandler =>
|
||||||
|
'Add an `on<...>` handler to $ownerName for each of $list, or stop '
|
||||||
|
'dispatching it.',
|
||||||
|
ruleRenderedNotEmitted =>
|
||||||
|
policy == FixPolicy.wireUp
|
||||||
|
? 'Emit each of $list from $ownerName where the UI expects it. '
|
||||||
|
'Removing the branches that match them needs explicit approval.'
|
||||||
|
: 'Remove the branch matching each of $list, or emit it from '
|
||||||
|
'$ownerName if the UI is right.',
|
||||||
|
_ => '${ruleFixFor(policy)[head.rule] ?? single} Applies to each of: $list.',
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
String _groupKey(Finding finding) {
|
String _groupKey(Finding finding) {
|
||||||
final data = finding.data;
|
final data = finding.data;
|
||||||
for (final key in const [
|
for (final key in const [
|
||||||
|
|
@ -349,7 +405,11 @@ String _groupKey(Finding finding) {
|
||||||
|
|
||||||
/// Build the plan from a report. Only new findings are planned, so a ratcheted
|
/// Build the plan from a report. Only new findings are planned, so a ratcheted
|
||||||
/// baseline yields an empty plan.
|
/// baseline yields an empty plan.
|
||||||
WorkPlan buildWorkPlan({required String root, required FindingReport report}) {
|
WorkPlan buildWorkPlan({
|
||||||
|
required String root,
|
||||||
|
required FindingReport report,
|
||||||
|
FixPolicy policy = FixPolicy.wireUp,
|
||||||
|
}) {
|
||||||
final newFindings = report.newFindings;
|
final newFindings = report.newFindings;
|
||||||
final order = <String>[];
|
final order = <String>[];
|
||||||
final groups = <String, List<Finding>>{};
|
final groups = <String, List<Finding>>{};
|
||||||
|
|
@ -369,21 +429,22 @@ WorkPlan buildWorkPlan({required String root, required FindingReport report}) {
|
||||||
final group = groups[key]!;
|
final group = groups[key]!;
|
||||||
final head = group.first;
|
final head = group.first;
|
||||||
final file = head.file ?? '(unknown)';
|
final file = head.file ?? '(unknown)';
|
||||||
final score = _scoreOf(group);
|
final score = _scoreOf(group, policy);
|
||||||
|
final targets = _targetsOf(group);
|
||||||
final action = FixAction(
|
final action = FixAction(
|
||||||
rule: head.rule,
|
rule: head.rule,
|
||||||
severity: head.severity,
|
severity: head.severity,
|
||||||
confidence: head.confidence,
|
confidence: head.confidence,
|
||||||
label: _labelOf(group) ?? file,
|
label: _labelOf(group) ?? file,
|
||||||
action: head.remediation ?? head.message,
|
action: _actionOf(group, targets, policy),
|
||||||
locations: [
|
locations: [
|
||||||
for (final finding in group)
|
for (final finding in group)
|
||||||
if (finding.location.isNotEmpty) finding.location,
|
if (finding.location.isNotEmpty) finding.location,
|
||||||
]..sort(),
|
]..sort(),
|
||||||
targets: _targetsOf(group),
|
targets: targets,
|
||||||
resolves: group.length,
|
resolves: group.length,
|
||||||
score: score,
|
score: score,
|
||||||
why: _whyOf(group, score),
|
why: _whyOf(group, score, policy),
|
||||||
file: file,
|
file: file,
|
||||||
line: head.line,
|
line: head.line,
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -331,4 +331,67 @@ void main() {
|
||||||
);
|
);
|
||||||
expect(plan.ordered.map((action) => action.label), ['A2', 'A9', 'M', 'Z']);
|
expect(plan.ordered.map((action) => action.label), ['A2', 'A9', 'M', 'Z']);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('the agent plan opens with a caution about what its order means', () {
|
||||||
|
final plan = buildWorkPlan(
|
||||||
|
root: '/root',
|
||||||
|
report: _report([
|
||||||
|
_finding(
|
||||||
|
rule: ruleDeadHandler,
|
||||||
|
severity: PipeSeverity.info,
|
||||||
|
file: 'lib/a.dart',
|
||||||
|
line: 1,
|
||||||
|
data: {'event': 'A'},
|
||||||
|
),
|
||||||
|
]),
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
plan.toAgentBlock(),
|
||||||
|
contains('Order is a score, not a mandate.'),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('wiring up a dead handler is not scored as judgement-free', () {
|
||||||
|
// Under the default wire-up policy the fix is to add a control that
|
||||||
|
// dispatches the event, which is a product decision; only removal needs
|
||||||
|
// no judgement.
|
||||||
|
final findings = [
|
||||||
|
_finding(
|
||||||
|
rule: ruleDeadHandler,
|
||||||
|
severity: PipeSeverity.info,
|
||||||
|
file: 'lib/a.dart',
|
||||||
|
line: 1,
|
||||||
|
data: {'event': 'A'},
|
||||||
|
),
|
||||||
|
];
|
||||||
|
final wireUp = buildWorkPlan(root: '/root', report: _report(findings));
|
||||||
|
final remove = buildWorkPlan(
|
||||||
|
root: '/root',
|
||||||
|
report: _report(findings),
|
||||||
|
policy: FixPolicy.remove,
|
||||||
|
);
|
||||||
|
expect(wireUp.ordered.single.why, isNot(contains('no product judgement')));
|
||||||
|
expect(remove.ordered.single.why, contains('no product judgement (+15)'));
|
||||||
|
expect(remove.ordered.single.score, wireUp.ordered.single.score + 15);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a grouped action names every target, not only the first', () {
|
||||||
|
final plan = buildWorkPlan(
|
||||||
|
root: '/root',
|
||||||
|
report: _report([
|
||||||
|
for (final event in ['Alpha', 'Beta', 'Gamma'])
|
||||||
|
_finding(
|
||||||
|
rule: ruleDeadHandler,
|
||||||
|
severity: PipeSeverity.info,
|
||||||
|
file: 'lib/bloc.dart',
|
||||||
|
line: 1,
|
||||||
|
data: {'union': 'Events', 'owner': 'Bloc', 'event': event},
|
||||||
|
remediation: 'Wire it up: dispatch $event from the UI.',
|
||||||
|
),
|
||||||
|
]),
|
||||||
|
);
|
||||||
|
final action = plan.ordered.single;
|
||||||
|
expect(action.resolves, 3);
|
||||||
|
expect(action.action, contains('dispatch each of Alpha, Beta, Gamma'));
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue