fix(guide,suggest): word agent guidance no stronger than the evidence #2
4 changed files with 54 additions and 18 deletions
fix(guide): scope "nothing references it" to the directories scanned
The brief said "nothing outside the bloc references it" and, under
--policy remove, that removing a dead handler "is safe even without product
context", while the scan defaults to --dir lib. The dead-handler finding
itself was scoped in f0fc57e; these three statements were not. A handler
dispatched only from test/ still read as safe to delete.
The decision line and the dead-handler section now say "nothing under <dirs>"
and tell the reader to rule out unscanned directories (rerun with --dir)
before wiring or removing; the remove-policy remediation in rules.dart says
the same. buildGuidance takes the scanned directories, which the guide
command passes from the project.
commit
cb302bbe12
|
|
@ -740,6 +740,7 @@ class GuideCommand extends Command<int> {
|
|||
flow: analysis.flow,
|
||||
report: analysis.report,
|
||||
policy: _readPolicy(results),
|
||||
scannedDirectories: project.directories,
|
||||
budgetChars: readPositiveInt(results, 'budget', fallback: 6000),
|
||||
maxFindings: readPositiveInt(results, 'max-findings', fallback: 12),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -220,7 +220,10 @@ String? _groupLabel(Finding finding) {
|
|||
/// Turn a finding into the concrete decision the reader has to make. Without
|
||||
/// this, "render it or delete it" is not actionable: the reader cannot tell
|
||||
/// whether the whole feature is unwired.
|
||||
String? _decisionLine(Finding finding, FixPolicy policy) {
|
||||
///
|
||||
/// [scope] names the scanned directories: "nothing references it" is only ever
|
||||
/// established for them.
|
||||
String? _decisionLine(Finding finding, FixPolicy policy, String scope) {
|
||||
if (finding.rule != ruleEmittedNotRendered) return null;
|
||||
final observed = finding.data['unionObservedInUi'] == true;
|
||||
final sites = finding.data['unionRenderSites'];
|
||||
|
|
@ -236,12 +239,14 @@ String? _decisionLine(Finding finding, FixPolicy policy) {
|
|||
}
|
||||
if (owners.isEmpty) {
|
||||
return policy == FixPolicy.wireUp
|
||||
? 'wire it up: nothing outside the bloc references it either, so the '
|
||||
'feature is unwired end to end - mount it in a screen and render '
|
||||
'its states. Removing it needs explicit approval.'
|
||||
: 'no UI renders this state type and nothing outside the bloc '
|
||||
'references it, so nothing is wired up: this is safe to delete '
|
||||
'unless the feature is still to come.';
|
||||
? 'wire it up: nothing under $scope outside the bloc references it '
|
||||
'either, so the feature looks unwired end to end - mount it in a '
|
||||
'screen and render its states. Removing it needs explicit '
|
||||
'approval.'
|
||||
: 'no UI renders this state type and nothing under $scope outside the '
|
||||
'bloc references it, so nothing scanned wires it up: this is safe '
|
||||
'to delete once no unscanned directory uses it and the feature is '
|
||||
'not still to come.';
|
||||
}
|
||||
final shown = owners.take(3).join(', ');
|
||||
final suffix = owners.length > 3 ? ', +${owners.length - 3} more' : '';
|
||||
|
|
@ -255,7 +260,12 @@ String? _decisionLine(Finding finding, FixPolicy policy) {
|
|||
'it.';
|
||||
}
|
||||
|
||||
String _groupBlock(List<Finding> group, int index, FixPolicy policy) {
|
||||
String _groupBlock(
|
||||
List<Finding> group,
|
||||
int index,
|
||||
FixPolicy policy,
|
||||
String scope,
|
||||
) {
|
||||
final head = group.first;
|
||||
final buffer = StringBuffer();
|
||||
final label = _groupLabel(head);
|
||||
|
|
@ -307,7 +317,7 @@ String _groupBlock(List<Finding> group, int index, FixPolicy policy) {
|
|||
if (parts.isNotEmpty) {
|
||||
buffer.writeln(' where: ${parts.join(' | ')}');
|
||||
}
|
||||
final decision = _decisionLine(head, policy);
|
||||
final decision = _decisionLine(head, policy, scope);
|
||||
if (decision != null) buffer.writeln(' decision: $decision');
|
||||
// Evidence quotes the analysed repository, which is untrusted input. It is
|
||||
// labelled as data and fenced so nothing in it can close its own quotation.
|
||||
|
|
@ -326,7 +336,9 @@ Guidance buildGuidance({
|
|||
int budgetChars = 6000,
|
||||
int maxFindings = 12,
|
||||
FixPolicy policy = FixPolicy.wireUp,
|
||||
List<String> scannedDirectories = const ['lib'],
|
||||
}) {
|
||||
final scope = scannedDirectories.join(', ');
|
||||
final ranked = rankFindings(report.newFindings);
|
||||
final summary = <String, Object?>{
|
||||
'root': root,
|
||||
|
|
@ -426,7 +438,7 @@ Guidance buildGuidance({
|
|||
for (var i = 0; i < groups.length; i++) {
|
||||
final block = selected.length >= maxFindings
|
||||
? null
|
||||
: _groupBlock(groups[i], selected.length + 1, policy);
|
||||
: _groupBlock(groups[i], selected.length + 1, policy, scope);
|
||||
if (block == null ||
|
||||
runningLength + block.length + noteReserve > budgetChars) {
|
||||
// Stop at the first entry that does not fit. Skipping it and packing the
|
||||
|
|
@ -479,8 +491,9 @@ Guidance buildGuidance({
|
|||
);
|
||||
}
|
||||
|
||||
// Certain edits: a dead handler can be removed without product context, so
|
||||
// surface those anchors even when they did not fit the ranked entries above.
|
||||
// Dead handlers: once the scan's scope is confirmed, removing one needs no
|
||||
// product context, so surface those anchors even when they did not fit the
|
||||
// ranked entries above.
|
||||
final dead = report.newFindings
|
||||
.where((f) => f.rule == ruleDeadHandler)
|
||||
.toList();
|
||||
|
|
@ -500,11 +513,14 @@ Guidance buildGuidance({
|
|||
)
|
||||
..writeln(
|
||||
policy == FixPolicy.wireUp
|
||||
? 'Nothing dispatches these, so the handler is unreachable. Wire it '
|
||||
'up by adding the control that dispatches the event; removing '
|
||||
'the handler needs explicit approval: $anchors'
|
||||
: 'Nothing dispatches these, so removing the `on<...>` registration '
|
||||
'is safe even without product context: $anchors',
|
||||
? 'Nothing under $scope dispatches these, so the handler is '
|
||||
'unreachable from there. Check that no unscanned directory '
|
||||
'dispatches them (rerun with --dir), then wire it up by adding '
|
||||
'the control that dispatches the event; removing the handler '
|
||||
'needs explicit approval: $anchors'
|
||||
: 'Nothing under $scope dispatches these. Once no unscanned '
|
||||
'directory does either (rerun with --dir), removing the '
|
||||
'`on<...>` registration needs no product context: $anchors',
|
||||
);
|
||||
if (buffer.length + section.length < budgetChars) buffer.write(section);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -715,7 +715,10 @@ FindingReport runChecks({
|
|||
'approval. Before wiring, check the event is not simply '
|
||||
'dispatched from a directory that was not scanned (add it '
|
||||
'with --dir): test-only handlers are not missing features.'
|
||||
: 'Remove the dead handler, or dispatch $event from the UI.',
|
||||
: 'Remove the dead handler once nothing outside '
|
||||
'${project.directories.join(', ')} dispatches $event '
|
||||
'(widen the scan with --dir to check), or dispatch '
|
||||
'$event from the UI.',
|
||||
data: {'union': union.name, 'owner': union.owner, 'event': event},
|
||||
),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -104,4 +104,20 @@ void main() {
|
|||
expect(ruleFixFor(FixPolicy.wireUp).keys, containsAll(allRules));
|
||||
expect(ruleFixFor(FixPolicy.remove).keys, containsAll(allRules));
|
||||
});
|
||||
|
||||
test('removal claims are scoped to the scanned directories', () {
|
||||
// The scan defaults to --dir lib, so "nothing outside the bloc references
|
||||
// it" and "safe even without product context" claimed more than was
|
||||
// checked: a handler dispatched only from test/ read as safe to delete.
|
||||
for (final policy in FixPolicy.values) {
|
||||
final brief = briefWith(policy);
|
||||
expect(brief, contains('nothing under lib'), reason: '$policy');
|
||||
expect(brief, isNot(contains('without product context')));
|
||||
expect(brief, isNot(contains('nothing outside the bloc')));
|
||||
}
|
||||
final dead = reportWith(
|
||||
FixPolicy.remove,
|
||||
).findings.firstWhere((finding) => finding.rule == ruleDeadHandler);
|
||||
expect(dead.remediation, contains('nothing outside lib'));
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue