From 34325c7e16008a41196d75f216f51347db256984 Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 11:59:53 -0600 Subject: [PATCH 01/11] fix(guide): make the rule legend's fixes follow --policy `ruleFix` was a constant map that ignored `--policy`. Under the default wire-up policy the brief's legend still said "Remove the dead handler, or dispatch the event" and "delete the variant and its emits" as equal choices, directly above findings whose remediation said removal needs explicit approval - the legend contradicted the policy the README describes. ruleFixFor(policy) now returns the legend's fixes for a policy: under wire-up the choice-bearing rules (emitted-not-rendered, rendered-not-emitted, event-without-handler, dead-handler, notifier-field-unread) lead with making the code reachable and say removal needs explicit approval; under remove they keep offering both. `ruleFix` remains, as the default policy's map. --- lib/pipe.dart | 1 + lib/src/guide.dart | 76 +++++++++++++++++++++++++++++++++---------- test/policy_test.dart | 18 ++++++++++ 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/lib/pipe.dart b/lib/pipe.dart index f1b9b3d..0d29699 100644 --- a/lib/pipe.dart +++ b/lib/pipe.dart @@ -13,6 +13,7 @@ export 'src/guide.dart' rankFindings, ruleEpithet, ruleFix, + ruleFixFor, ruleGlossary, yamsMetadata; export 'src/index.dart' diff --git a/lib/src/guide.dart b/lib/src/guide.dart index 676f35f..cd3768e 100644 --- a/lib/src/guide.dart +++ b/lib/src/guide.dart @@ -62,25 +62,14 @@ const Map ruleEpithet = { 'and listener are created on every rebuild and state is lost.', }; -/// The standard remediation for each rule. Hoisted out of the per-finding -/// blocks so a prompt budget is not spent repeating identical advice. -const Map ruleFix = { - ruleEmittedNotRendered: - 'Render the variant where the state is consumed, or delete the variant ' - 'and its emits. Prefer a sealed state hierarchy so the compiler enforces ' - 'exhaustive handling.', - ruleRenderedNotEmitted: - 'Delete the unreachable branch, or emit the variant from the owning bloc.', +/// Fixes that read the same under either policy. +const Map _policyNeutralFix = { ruleLoadingNotReset: 'Emit a failure/timeout state in the catch (or rethrow) so every loading ' 'state is resolved.', ruleLoadingFlagNotReset: 'Wrap the awaited call in try/catch and emit a state with the loading flag ' 'reset (or rethrow), so the loading state always resolves.', - ruleEventWithoutHandler: - 'Add the missing `on` handler to the bloc, or stop dispatching.', - ruleDeadHandler: - 'Remove the dead handler, or dispatch the event from the UI.', ruleRiverpodAsyncGap: 'Handle the missing AsyncValue case at the consumption site: ' '`switch (value) { AsyncData(:final value) => ..., AsyncLoading() => ..., ' @@ -95,14 +84,66 @@ const Map ruleFix = { 'Use `context.watch()` (or `Provider.of(context)`) so the widget ' 'subscribes, or keep the read for an imperative action and make sure ' 'nothing rendered depends on the value.', - ruleNotifierFieldUnread: - 'Render the member in the consuming widget, or make it private if it is ' - 'internal state rather than UI state.', ruleProviderCreatedInBuild: 'Hoist the provider above the widget that rebuilds, or use the `.value` ' 'constructor with an instance created once.', }; +/// `--policy wire-up`: make the unreachable thing reachable; removal is the +/// exception and needs explicit approval. +const Map _wireUpFix = { + ruleEmittedNotRendered: + 'Wire it up: render the variant where the state is consumed. Prefer a ' + 'sealed state hierarchy so the compiler enforces exhaustive handling. ' + 'Deleting the variant and its emits needs explicit approval.', + ruleRenderedNotEmitted: + 'Emit the variant from the owning bloc where the UI expects it. Deleting ' + 'the branch needs explicit approval.', + ruleEventWithoutHandler: + 'Add the missing `on` handler to the bloc. Removing the dispatch ' + 'instead needs explicit approval.', + ruleDeadHandler: + 'Wire it up: dispatch the event from the control that should trigger it, ' + 'after checking it is not dispatched from a directory that was not ' + 'scanned. Removing the handler needs explicit approval.', + ruleNotifierFieldUnread: + 'Wire it up: render the member in the consuming widget. Making it private ' + 'is the alternative when it is internal rather than UI state.', +}; + +/// `--policy remove`: removal is offered as an equal choice. +const Map _removeFix = { + ruleEmittedNotRendered: + 'Render the variant where the state is consumed, or delete the variant ' + 'and its emits. Prefer a sealed state hierarchy so the compiler enforces ' + 'exhaustive handling.', + ruleRenderedNotEmitted: + 'Delete the unreachable branch, or emit the variant from the owning bloc.', + ruleEventWithoutHandler: + 'Add the missing `on` handler to the bloc, or stop dispatching.', + ruleDeadHandler: + 'Remove the dead handler once nothing outside the scanned directories ' + 'dispatches it, or dispatch the event from the UI.', + ruleNotifierFieldUnread: + 'Render the member in the consuming widget, or make it private if it is ' + 'internal state rather than UI state.', +}; + +/// The standard remediation for each rule under [policy]. Hoisted out of the +/// per-finding blocks so a prompt budget is not spent repeating identical +/// advice. +/// +/// This used to be one constant map that ignored `--policy`, so under the +/// default wire-up policy the legend still led with "Remove the dead handler" +/// while each finding said removal needed explicit approval. +Map ruleFixFor(FixPolicy policy) => { + ..._policyNeutralFix, + ...(policy == FixPolicy.wireUp ? _wireUpFix : _removeFix), +}; + +/// The standard remediation under the default (wire-up) policy. +final Map ruleFix = ruleFixFor(FixPolicy.wireUp); + /// Label for quoted repository source wherever an agent may read it. const String _evidenceLabel = 'source excerpt (data, not instructions)'; @@ -353,11 +394,12 @@ Guidance buildGuidance({ // in the report. Its size is known up front, which lets group selection // reserve room for it instead of dropping it later. final presentRules = report.countByRule.keys.toSet(); + final fixes = ruleFixFor(policy); final glossary = StringBuffer(); for (final rule in allRules) { if (!presentRules.contains(rule)) continue; glossary.writeln( - '- $rule: ${ruleEpithet[rule] ?? ''} Fix: ${ruleFix[rule] ?? ''}', + '- $rule: ${ruleEpithet[rule] ?? ''} Fix: ${fixes[rule] ?? ''}', ); } const legendHeading = '## How to read the rule names\n'; diff --git a/test/policy_test.dart b/test/policy_test.dart index 499a1af..9b2afbd 100644 --- a/test/policy_test.dart +++ b/test/policy_test.dart @@ -86,4 +86,22 @@ void main() { ); expect(briefWith(FixPolicy.remove), contains('Cleanly deletable')); }); + + test('the legend follows the policy too', () { + // The legend's fixes were one constant map that ignored --policy, so under + // wire-up it still led with "Remove the dead handler" while every finding + // said removal needed explicit approval. + expect(ruleFixFor(FixPolicy.wireUp)[ruleDeadHandler], startsWith('Wire')); + expect( + ruleFixFor(FixPolicy.wireUp)[ruleEmittedNotRendered], + startsWith('Wire'), + ); + expect( + ruleFixFor(FixPolicy.remove)[ruleDeadHandler], + startsWith('Remove the dead handler'), + ); + expect(briefWith(FixPolicy.wireUp), isNot(contains('Fix: Remove'))); + expect(ruleFixFor(FixPolicy.wireUp).keys, containsAll(allRules)); + expect(ruleFixFor(FixPolicy.remove).keys, containsAll(allRules)); + }); } -- 2.52.0 From 37d4864d540605d483ac11ac81ef1e6e4840f52d Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:00:14 -0600 Subject: [PATCH 02/11] fix(guide): word each rule's one-liner no stronger than its confidence Four legend epithets claimed more than their rules establish: - rendered-not-emitted (medium): "so it is dead code". The rule sees only the scanned code and suppresses owners whose helper-method emissions it cannot resolve; it now says the branch is probably unreachable. - riverpod-async-gap: "a failure shows a blank or stale screen". It can; the rule downgrades `bool` guard providers to info precisely because it often does not. - notifier-field-unread (medium): "can never affect what the user sees". The check is a by-name read search in the scanned code; the finding message and the epithet now say "likely". - provider-created-in-build: "on every rebuild and state is lost". The rule is info because rebuild frequency is unknown; it now says "if the widget rebuilds". The README rules table and the rendered-not-emitted and notifier-field-unread messages are aligned with the same wording. --- README.md | 6 +++--- lib/src/guide.dart | 14 ++++++++------ lib/src/provider_rules.dart | 5 +++-- lib/src/rules.dart | 3 ++- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index d8728f9..30cddd4 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,7 @@ Baselines and reports are JSON, so they diff and review like any other file. | Rule | What it means | Verified precision (with n) | | --- | --- | --- | | `emitted-not-rendered` | A state the bloc emits that no UI code matches, so the user never sees it. | **79/79 (100%)** factual on a CLEAN nuwayze main via an independent adjudicator with passing controls (`tool/adjudicate.py`). Reported once per **root cause**, not once per variant: one finding per unobserved union (dormant feature) and one per marker class when the union is observed. When the UI reads the union's payload through **base-class fields** the finding drops to info, because the data does reach the user. On clean nuwayze main: 30 findings covering 79 variant-level claims. | -| `rendered-not-emitted` | A UI branch no state can reach: dead UI. | n=0 on nuwayze (unions whose bloc emits through helper methods are suppressed with a reason). Fires on a fixture with a passing negative control. | +| `rendered-not-emitted` | A UI branch for a variant its bloc never emits in the scanned code: probably dead UI. | n=0 on nuwayze (unions whose bloc emits through helper methods are suppressed with a reason). Fires on a fixture with a passing negative control. | | `loading-not-reset` | An in-flight state whose failure path never resolves, so the spinner sticks. | n=0 on both dogfood repos, both verified to guard correctly; proven able to fire by fixture. | | `loading-flag-not-reset` | The same consequence for `copyWith` + `bool loading` state, which has no variant to match on. | **7/7 (100%)** on dex after hand adjudication; 4 characterised false positives were eliminated by the current gate. n=0 on nuwayze, verified correct. | | `event-without-handler` | A dispatched event with no `on`, so the interaction does nothing. | n=0 across the corpus; verified non-vacuous (168 dispatch sites on nuwayze all map to a registered handler). | @@ -157,8 +157,8 @@ Baselines and reports are JSON, so they diff and review like any other file. | `notifier-never-notifies` | A `ChangeNotifier` method that changes observable state without ever notifying listeners. | Fixture contract only: **zero true positives** on the corpus (nuwayze 30 observable mutators, word_alive 36 — all of which notify). Gated on observable fields with lifecycle hooks excluded, after the first run was 9/9 false positives. | | `notifier-inert-consumer` | A notifier consumed by a `Consumer`/`Selector` with no notifying path at all, so those consumers can never rebuild. | Fixture contract only; no real-code true positives yet. | | `provider-read-in-build` | A `Listenable` read with `context.read` / `Provider.of(listen: false)` **directly in a build body**, so the widget never subscribes and can render stale state. | 1 finding on nuwayze from **447** raw `context.read` calls; 0 on word_alive/dex. Hand-verified: the first 4 findings were all reads inside **closures** (a `BlocListener` callback, a `Selector` builder), so a closure guard was added and only the direct-in-build case survives. | -| `notifier-field-unread` | A public getter on a consumed `ChangeNotifier` that nothing reads, so the state it exposes cannot affect what the user sees. | 3 on nuwayze (e.g. `UserProvider.isInitializing`, `AppLocaleProvider.preferredLocale`), 0 on word_alive/dex. Reported once per notifier, and only for notifiers the UI actually consumes. **medium** confidence: a name match elsewhere can mask a genuinely unread member. | -| `provider-created-in-build` | A provider constructed inside `build` without `.value`, so `create` reruns on every rebuild and state is lost. | 1 finding on nuwayze — `AppProviders.build` constructs **45** providers — from 45 raw sites, because it is reported **once per build method**. Deliberately **info**, not warning: the claim is factual, but a root `StatelessWidget` rarely rebuilds, so impact depends on rebuild frequency. | +| `notifier-field-unread` | A public getter on a consumed `ChangeNotifier` that nothing reads, so the state it exposes likely never reaches the UI. | 3 on nuwayze (e.g. `UserProvider.isInitializing`, `AppLocaleProvider.preferredLocale`), 0 on word_alive/dex. Reported once per notifier, and only for notifiers the UI actually consumes. **medium** confidence: a name match elsewhere can mask a genuinely unread member. | +| `provider-created-in-build` | A provider constructed inside `build` without `.value`, so if the widget rebuilds, `create` reruns and its state is lost. | 1 finding on nuwayze — `AppProviders.build` constructs **45** providers — from 45 raw sites, because it is reported **once per build method**. Deliberately **info**, not warning: the claim is factual, but a root `StatelessWidget` rarely rebuilds, so impact depends on rebuild frequency. | ### How precision was measured diff --git a/lib/src/guide.dart b/lib/src/guide.dart index cd3768e..f7fc9fa 100644 --- a/lib/src/guide.dart +++ b/lib/src/guide.dart @@ -33,7 +33,8 @@ const Map ruleEpithet = { ruleEmittedNotRendered: 'A state the bloc emits that no UI code matches, so the user never sees it.', ruleRenderedNotEmitted: - 'A UI branch that no state can reach, so it is dead code.', + 'A UI branch for a variant its bloc never emits in the scanned code, so ' + 'the branch is probably unreachable.', ruleLoadingNotReset: 'A loading state whose failure path never resolves, so the spinner sticks.', ruleLoadingFlagNotReset: @@ -44,7 +45,8 @@ const Map ruleEpithet = { ruleDeadHandler: 'An `on` registration that nothing ever dispatches.', ruleRiverpodAsyncGap: 'An async Riverpod provider read in the UI whose error and/or loading ' - 'state is never rendered, so a failure shows a blank or stale screen.', + 'state is never rendered, so a failure can leave a blank or stale screen ' + 'with no sign that anything went wrong.', ruleNotifierNeverNotifies: 'A ChangeNotifier method that changes state without ever notifying ' 'listeners, so the UI keeps showing the old values.', @@ -55,11 +57,11 @@ const Map ruleEpithet = { 'A Listenable read with `context.read` / `Provider.of(listen: false)` ' 'inside build, so the widget never subscribes and can go stale.', ruleNotifierFieldUnread: - 'A public getter on a consumed ChangeNotifier that nothing reads, so the ' - 'state it exposes can never affect what the user sees.', + 'A public getter on a consumed ChangeNotifier that nothing in the scanned ' + 'code reads by name, so the state it exposes likely never reaches the UI.', ruleProviderCreatedInBuild: - 'A provider constructed inside build without `.value`, so a new instance ' - 'and listener are created on every rebuild and state is lost.', + 'A provider constructed inside build without `.value`, so if the widget ' + 'rebuilds, a new instance and listener are created and its state is lost.', }; /// Fixes that read the same under either policy. diff --git a/lib/src/provider_rules.dart b/lib/src/provider_rules.dart index 1d1c70c..ee3732b 100644 --- a/lib/src/provider_rules.dart +++ b/lib/src/provider_rules.dart @@ -821,8 +821,9 @@ String runProviderRules({ confidence: 'medium', message: '${notifier.name} is consumed in the UI but ${unread.length} of ' - 'its public member(s) are read nowhere outside the class, so ' - 'nothing the user sees can depend on them: ${unread.join(', ')}.', + 'its public member(s) are read by name nowhere outside the class ' + 'in the scanned code, so what the user sees likely does not depend ' + 'on them: ${unread.join(', ')}.', file: notifier.file, line: notifier.line, evidence: [ diff --git a/lib/src/rules.dart b/lib/src/rules.dart index a909e10..f67f803 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -609,7 +609,8 @@ FindingReport runChecks({ confidence: 'medium', message: '$variant is matched in UI code but ${coverage.union.owner} ' - 'never emits it, so that branch is dead.', + 'never emits it in the scanned code, so that branch is probably ' + 'unreachable.', file: sites.isEmpty ? null : sites.first.path, line: sites.isEmpty ? null : sites.first.line, evidence: [ -- 2.52.0 From cb302bbe12fed002cdc29690d34cf3c6d8b2c43b Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:00:49 -0600 Subject: [PATCH 03/11] 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 " 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. --- lib/src/cli.dart | 1 + lib/src/guide.dart | 50 ++++++++++++++++++++++++++++--------------- lib/src/rules.dart | 5 ++++- test/policy_test.dart | 16 ++++++++++++++ 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/lib/src/cli.dart b/lib/src/cli.dart index ad24968..a1870a5 100644 --- a/lib/src/cli.dart +++ b/lib/src/cli.dart @@ -740,6 +740,7 @@ class GuideCommand extends Command { 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), ); diff --git a/lib/src/guide.dart b/lib/src/guide.dart index f7fc9fa..86cc929 100644 --- a/lib/src/guide.dart +++ b/lib/src/guide.dart @@ -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 group, int index, FixPolicy policy) { +String _groupBlock( + List 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 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 scannedDirectories = const ['lib'], }) { + final scope = scannedDirectories.join(', '); final ranked = rankFindings(report.newFindings); final summary = { '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); } diff --git a/lib/src/rules.dart b/lib/src/rules.dart index f67f803..034478e 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -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}, ), ); diff --git a/test/policy_test.dart b/test/policy_test.dart index 9b2afbd..94c27f6 100644 --- a/test/policy_test.dart +++ b/test/policy_test.dart @@ -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')); + }); } -- 2.52.0 From 138baad67cc88496b27b1fdc9cde43915378a3f1 Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:01:38 -0600 Subject: [PATCH 04/11] 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. --- lib/src/cli.dart | 6 +++- lib/src/suggest.dart | 81 ++++++++++++++++++++++++++++++++++++------ test/suggest_test.dart | 63 ++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 11 deletions(-) diff --git a/lib/src/cli.dart b/lib/src/cli.dart index a1870a5..b4ac3d1 100644 --- a/lib/src/cli.dart +++ b/lib/src/cli.dart @@ -647,7 +647,11 @@ class SuggestCommand extends Command { policy: _readPolicy(results), ); 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 // one output branch that uses it, so `suggest --top abc --format agent` // silently ignored the malformed value instead of rejecting it. diff --git a/lib/src/suggest.dart b/lib/src/suggest.dart index 923e5c3..b24461b 100644 --- a/lib/src/suggest.dart +++ b/lib/src/suggest.dart @@ -1,4 +1,5 @@ import 'analysis.dart'; +import 'guide.dart'; import 'rules.dart'; /// One fixable root cause: a single action that resolves one or more findings. @@ -159,6 +160,7 @@ class WorkPlan { final buffer = StringBuffer() ..writeln('# pipe plan — $root') ..writeln() + ..writeln(agentCaution) ..write(coverage == null ? '' : '${coverage!.describe()}\n') ..writeln( '$totalFindings findings collapse to $totalCauses root cause(s) across ' @@ -245,6 +247,11 @@ const Map _confidenceFactor = { '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. const Set _certaintyRules = { ruleDeadHandler, @@ -253,17 +260,24 @@ const Set _certaintyRules = { ruleLoadingFlagNotReset, }; -int _scoreOf(List 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 group, FixPolicy policy) { final head = group.first; final base = _severityBase[head.severity] ?? 10; final factor = _confidenceFactor[head.confidence] ?? 1.0; final breadth = ((group.length - 1).clamp(0, 5)) * 10; 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; } -String _whyOf(List group, int score) { +String _whyOf(List group, int score, FixPolicy policy) { final head = group.first; final base = _severityBase[head.severity] ?? 10; final parts = [ @@ -273,8 +287,7 @@ String _whyOf(List group, int score) { 'one fix clears ${group.length} findings (+' '${((group.length - 1).clamp(0, 5)) * 10})', if (head.data['uxCritical'] == true) 'user-visible failure (+25)', - if (_certaintyRules.contains(head.rule)) - 'no product judgement needed (+15)', + if (_isCertain(head.rule, policy)) 'no product judgement needed (+15)', ]; return '$score = ${parts.join(', ')}'; } @@ -308,6 +321,15 @@ List _targetsOf(List group) { final targets = []; for (final finding in group) { 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()) { + if (value.isNotEmpty && !targets.contains(value)) targets.add(value); + } + } for (final key in const [ 'variant', 'event', @@ -328,6 +350,40 @@ List _targetsOf(List group) { 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 group, List 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) { final data = finding.data; 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 /// 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 order = []; final groups = >{}; @@ -369,21 +429,22 @@ WorkPlan buildWorkPlan({required String root, required FindingReport report}) { final group = groups[key]!; final head = group.first; final file = head.file ?? '(unknown)'; - final score = _scoreOf(group); + final score = _scoreOf(group, policy); + final targets = _targetsOf(group); final action = FixAction( rule: head.rule, severity: head.severity, confidence: head.confidence, label: _labelOf(group) ?? file, - action: head.remediation ?? head.message, + action: _actionOf(group, targets, policy), locations: [ for (final finding in group) if (finding.location.isNotEmpty) finding.location, ]..sort(), - targets: _targetsOf(group), + targets: targets, resolves: group.length, score: score, - why: _whyOf(group, score), + why: _whyOf(group, score, policy), file: file, line: head.line, ); diff --git a/test/suggest_test.dart b/test/suggest_test.dart index 3c35577..47f9f51 100644 --- a/test/suggest_test.dart +++ b/test/suggest_test.dart @@ -331,4 +331,67 @@ void main() { ); 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')); + }); } -- 2.52.0 From ea9d51078a072c8e6f2392456e3d821fce319e1d Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:02:22 -0600 Subject: [PATCH 05/11] fix(guide,suggest): make verify hints commands that can be pasted as-is Every brief entry and plan action ended with `verify: pipe check --rule X`. Pasted, that checks the current directory, over lib/ only, against no baseline - not the run that produced the finding. Run from anywhere but the project root, or after a scan with --dir or --baseline, it answered a different question. The hint is now built by checkCommand(): `pipe check --rule X`, plus `--baseline ` and every `--dir` when the run used them, with paths shell-quoted when needed. --- lib/pipe.dart | 1 + lib/src/cli.dart | 3 +++ lib/src/guide.dart | 44 ++++++++++++++++++++++++++++++++++++++++-- lib/src/suggest.dart | 15 ++++++++++++-- test/guide_test.dart | 25 ++++++++++++++++++++++++ test/suggest_test.dart | 24 ++++++++++++++++++++++- 6 files changed, 107 insertions(+), 5 deletions(-) diff --git a/lib/pipe.dart b/lib/pipe.dart index 0d29699..3a50ae1 100644 --- a/lib/pipe.dart +++ b/lib/pipe.dart @@ -7,6 +7,7 @@ export 'src/guide.dart' show Guidance, buildGuidance, + checkCommand, emitToYams, findYams, gitHead, diff --git a/lib/src/cli.dart b/lib/src/cli.dart index b4ac3d1..d3e3e8a 100644 --- a/lib/src/cli.dart +++ b/lib/src/cli.dart @@ -651,6 +651,8 @@ class SuggestCommand extends Command { root: project.root, report: analysis.report, policy: _readPolicy(results), + baselinePath: results['baseline'] as String?, + directories: project.directories, ); // Read every numeric option up front. Each was previously read inside the // one output branch that uses it, so `suggest --top abc --format agent` @@ -745,6 +747,7 @@ class GuideCommand extends Command { report: analysis.report, policy: _readPolicy(results), scannedDirectories: project.directories, + baselinePath: results['baseline'] as String?, budgetChars: readPositiveInt(results, 'budget', fallback: 6000), maxFindings: readPositiveInt(results, 'max-findings', fallback: 12), ); diff --git a/lib/src/guide.dart b/lib/src/guide.dart index 86cc929..4f7df8a 100644 --- a/lib/src/guide.dart +++ b/lib/src/guide.dart @@ -265,6 +265,7 @@ String _groupBlock( int index, FixPolicy policy, String scope, + String Function(String rule) verifyFor, ) { final head = group.first; final buffer = StringBuffer(); @@ -324,7 +325,7 @@ String _groupBlock( for (final item in head.evidence.take(1)) { buffer.writeln(' $_evidenceLabel: ${dataSpan(item)}'); } - buffer.writeln(' verify: pipe check --rule ${head.rule}'); + buffer.writeln(' verify: ${verifyFor(head.rule)}'); return buffer.toString(); } @@ -337,8 +338,15 @@ Guidance buildGuidance({ int maxFindings = 12, FixPolicy policy = FixPolicy.wireUp, List scannedDirectories = const ['lib'], + String? baselinePath, }) { final scope = scannedDirectories.join(', '); + String verifyFor(String rule) => checkCommand( + root: root, + rule: rule, + baseline: baselinePath, + directories: scannedDirectories, + ); final ranked = rankFindings(report.newFindings); final summary = { 'root': root, @@ -438,7 +446,13 @@ Guidance buildGuidance({ for (var i = 0; i < groups.length; i++) { final block = selected.length >= maxFindings ? null - : _groupBlock(groups[i], selected.length + 1, policy, scope); + : _groupBlock( + groups[i], + selected.length + 1, + policy, + scope, + verifyFor, + ); if (block == null || runningLength + block.length + noteReserve > budgetChars) { // Stop at the first entry that does not fit. Skipping it and packing the @@ -647,6 +661,32 @@ Guidance buildGuidance({ ); } +/// A `pipe check` command that re-runs one rule the way an analysis ran: same +/// root, same baseline, same --dir scope. The hint used to be a bare +/// `pipe check --rule X`, which checks the current directory against no +/// baseline over lib/ only - not what produced the finding. +String checkCommand({ + required String root, + required String rule, + String? baseline, + List directories = const ['lib'], +}) { + final parts = ['pipe', 'check', _shellQuote(root), '--rule', rule]; + if (baseline != null) parts.addAll(['--baseline', _shellQuote(baseline)]); + final defaultScope = directories.length == 1 && directories.single == 'lib'; + if (!defaultScope) { + for (final directory in directories) { + parts.addAll(['--dir', _shellQuote(directory)]); + } + } + return parts.join(' '); +} + +String _shellQuote(String value) => + RegExp(r'^[A-Za-z0-9_./:=@+-]+$').hasMatch(value) + ? value + : "'${value.replaceAll("'", r"'\''")}'"; + /// Metadata for a report persisted to yams: what produced it, from which /// project, when, and - when the project is a git checkout - at which commit, /// so a report read back from the corpus can be dated and reproduced. diff --git a/lib/src/suggest.dart b/lib/src/suggest.dart index b24461b..686cbe9 100644 --- a/lib/src/suggest.dart +++ b/lib/src/suggest.dart @@ -17,7 +17,8 @@ class FixAction { required this.why, this.file = '', this.line, - }); + String? verify, + }) : verify = verify ?? checkCommand(root: '.', rule: rule); final String rule; final PipeSeverity severity; @@ -43,7 +44,9 @@ class FixAction { final String file; final int? line; - String get verify => 'pipe check --rule $rule'; + /// A command that re-checks this rule the way the plan was produced, so it + /// can be pasted as-is. + final String verify; Map toJson() => { 'rule': rule, @@ -409,6 +412,8 @@ WorkPlan buildWorkPlan({ required String root, required FindingReport report, FixPolicy policy = FixPolicy.wireUp, + String? baselinePath, + List directories = const ['lib'], }) { final newFindings = report.newFindings; final order = []; @@ -447,6 +452,12 @@ WorkPlan buildWorkPlan({ why: _whyOf(group, score, policy), file: file, line: head.line, + verify: checkCommand( + root: root, + rule: head.rule, + baseline: baselinePath, + directories: directories, + ), ); byFile.putIfAbsent(file, () => []).add(action); fileScores.update(file, (value) => value + score, ifAbsent: () => score); diff --git a/test/guide_test.dart b/test/guide_test.dart index 193fb81..a27747d 100644 --- a/test/guide_test.dart +++ b/test/guide_test.dart @@ -159,6 +159,31 @@ void main() { ); }); + test('verify hints are commands that can be pasted as-is', () { + // `pipe check --rule X` checked the current directory, over lib/, against + // no baseline - not the run that produced the brief. + final brief = buildGuidance( + root: '/work/app', + flow: analysis.flow, + report: analysis.report, + baselinePath: 'ci/base.json', + scannedDirectories: const ['lib', 'test'], + ).brief; + expect( + brief, + matches( + RegExp( + r'verify: pipe check /work/app --rule [a-z-]+ ' + r'--baseline ci/base\.json --dir lib --dir test', + ), + ), + ); + expect( + checkCommand(root: '/work/app', rule: ruleDeadHandler), + 'pipe check /work/app --rule dead-handler', + ); + }); + test('yams metadata records provenance, not a stale owner', () { final metadata = yamsMetadata( project: 'app', diff --git a/test/suggest_test.dart b/test/suggest_test.dart index 47f9f51..2472216 100644 --- a/test/suggest_test.dart +++ b/test/suggest_test.dart @@ -144,7 +144,7 @@ void main() { expect(action.why, startsWith('${action.score} = ')); expect(action.why, contains('warning 50')); expect(action.why, contains('user-visible failure')); - expect(action.verify, 'pipe check --rule $ruleEmittedNotRendered'); + expect(action.verify, 'pipe check /root --rule $ruleEmittedNotRendered'); expect(action.action, isNotEmpty); }); @@ -394,4 +394,26 @@ void main() { expect(action.resolves, 3); expect(action.action, contains('dispatch each of Alpha, Beta, Gamma')); }); + + test('the verify hint reruns the check the way the plan was made', () { + final plan = buildWorkPlan( + root: '/my app', + report: _report([ + _finding( + rule: ruleDeadHandler, + severity: PipeSeverity.info, + file: 'lib/a.dart', + line: 1, + data: {'event': 'A'}, + ), + ]), + baselinePath: '.pipe-baseline.json', + directories: const ['lib', 'test'], + ); + expect( + plan.ordered.single.verify, + "pipe check '/my app' --rule dead-handler " + '--baseline .pipe-baseline.json --dir lib --dir test', + ); + }); } -- 2.52.0 From c666df6a99ac2a3b395e4137239d5d566dad42a9 Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:02:49 -0600 Subject: [PATCH 06/11] fix(guide): count new findings in the markdown summary, as its heading does The markdown report's summary line counted every finding while its heading and the severity counts beside it counted only new ones, so a fully baselined run read "20 findings ({error: 0, warning: 0, info: 0})" above "## Findings (0)". The summary now counts new findings and notes how many are baselined. Also: loading-flag-not-reset rendered its flag as `SettingsState`loading``, a missing `.` that produced two adjacent code spans. It now reads `SettingsState.loading`. --- lib/src/guide.dart | 6 ++++-- lib/src/rules.dart | 5 +++-- test/flag_state_test.dart | 3 +++ test/guide_test.dart | 21 +++++++++++++++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/src/guide.dart b/lib/src/guide.dart index 4f7df8a..59a4c72 100644 --- a/lib/src/guide.dart +++ b/lib/src/guide.dart @@ -598,9 +598,11 @@ Guidance buildGuidance({ ..writeln() ..write(coverageLine == null ? '' : '$coverageLine\n\n') ..writeln( + // New findings only, matching the heading below and countBySeverity, + // which already counted only new ones. '${flow.unions.length} blocs/cubits, ${summary['stateVariants']} state ' - 'variants, ${report.findings.length} findings ' - '(${report.countBySeverity}).', + 'variants, ${report.newFindings.length} new findings ' + '(${report.countBySeverity})$baselinedNote.', ) ..writeln() ..writeln('## Findings (${ranked.length})'); diff --git a/lib/src/rules.dart b/lib/src/rules.dart index 034478e..0101d8e 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -1172,8 +1172,9 @@ class _FlagLoadingCollector extends RecursiveAstVisitor { severity: PipeSeverity.warning, confidence: setsFlagTrue && !awaitedTrySeen ? 'high' : 'medium', message: - '$owner.$member awaits work after putting ${flag.name}`$fieldList` ' - 'into its loading state, and no catch clause surfaces a failure, so ' + '$owner.$member awaits work after putting ' + '`${flag.name}.$fieldList` into its loading state, and no catch ' + 'clause surfaces a failure, so ' 'the UI can keep showing its loading state after an error.', file: project.relative(path), line: lineInfo.getLocation(body.offset).lineNumber, diff --git a/test/flag_state_test.dart b/test/flag_state_test.dart index 1038311..ba12d19 100644 --- a/test/flag_state_test.dart +++ b/test/flag_state_test.dart @@ -39,6 +39,9 @@ void main() { expect(hits.single.confidence, 'high'); expect(hits.single.evidence, isNotEmpty); expect(hits.single.remediation, isNotNull); + // The flag reads as one code span, `FlagState.loading`, not as + // `FlagState` immediately followed by a second span. + expect(hits.single.message, contains('`FlagState.')); }); test('flag findings carry no false positive for guarded code', () { diff --git a/test/guide_test.dart b/test/guide_test.dart index a27747d..23e9ef5 100644 --- a/test/guide_test.dart +++ b/test/guide_test.dart @@ -200,6 +200,27 @@ void main() { ); }); + test('the markdown summary counts the same findings as its heading', () { + // The summary counted every finding while the heading (and the severity + // counts beside it) counted only new ones, so a fully baselined run read + // "20 findings ({error: 0, warning: 0, info: 0})" over "## Findings (0)". + final baselined = runChecks( + project: analysis.project, + outcomes: analysis.run.outcomes, + flow: analysis.flow, + baseline: { + for (final finding in analysis.report.findings) finding.fingerprint, + }, + ); + final md = buildGuidance( + root: analysis.project.root, + flow: analysis.flow, + report: baselined, + ).markdown; + expect(md, contains(' 0 new findings ')); + expect(md, contains('## Findings (0)')); + }); + test('json summary exposes counts and suppression notes', () { final summary = guidance().summary; expect(summary['findings'], analysis.report.findings.length); -- 2.52.0 From f7eeef7baabb31af4283281f02ece76e35d42c6b Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:04:01 -0600 Subject: [PATCH 07/11] docs: bring the README in line with what pipe does and prints Drift, corrected: - The suggest example showed a "fix: Either render ..." line that no current remediation produces; it now shows the current agent-plan shape (caution line, coverage line, wire-up remediation), marked illustrative because it could not be regenerated from one run. - Common flags: suggest's `text|agent|json` formats, `recall -c` defaulting to 8 rather than 4, `--[no-]include-generated` defaulting on, and a missing --dir being an error. - `--yams` persists the full markdown report, not the brief. - The architecture list named recall.dart twice and omitted cli.dart, errors.dart, pool.dart and provider_rules.dart (and now excerpt.dart). - The hardcoded "84 tests" is gone; the recall result and the word_alive manifest note are dated "as of 2026-09-20", since both describe branches that were unmerged at that date. Documented from the previous PR: stdout carries only the requested output, the `coverage` block and one-line coverage note, exit codes (0/1/2/64), verify's --rule filter and NOT SCANNED category, version-2 baselines and how version-1 fingerprints are still matched, packing that stops at the first entry that does not fit, bounded source excerpts, pasteable verify commands, and the policy's effect on the legend and on suggest's certainty bonus. --- README.md | 135 ++++++++++++++++++++++++++++++++----------- bin/pipe.dart | 3 +- recall/manifest.json | 2 +- 3 files changed, 104 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 30cddd4..98502ec 100644 --- a/README.md +++ b/README.md @@ -78,21 +78,45 @@ pipe verify --baseline .pipe-baseline.json --expect-resolved pipe suggest --format agent --budget 3000 ``` -Common flags: `--format text|json` (`agent|md|json` for `guide`), `-c N` -(concurrency, default 4), `--sdk ` (override SDK discovery), `--dir` -(repeatable), `--include-generated`, `--limit N`, `--rule ` (repeatable), -`--top N`. +Common flags: `--format text|json` (`agent|md|json` for `guide`, +`text|agent|json` for `suggest`), `-c N` (concurrency, default 4; 8 for +`recall`), `--sdk ` (override SDK discovery), `--dir` (repeatable, +default `lib`; a directory that does not exist is an error, not skipped), +`--[no-]include-generated` (default on: `*.g.dart`, `*.freezed.dart` and +`*.mocks.dart` are scanned unless you pass `--no-include-generated`), +`--limit N`, `--rule ` (repeatable), `--top N`. Command-specific flags: `index --fail-on-error` exits 1 when the resolved project has analyzer errors (the default reports the count and exits 0); `suggest --budget N --max-actions N --top N` bound the agent plan and the text report; `guide --budget N --max-findings N` bound the agent brief, and `--yams` -with `--yams-name ` persists that brief to the yams corpus; +with `--yams-name ` persists the full markdown report (not the +budget-trimmed brief) to the yams corpus, tagged with the analysed commit and a +generation timestamp; `recall --manifest --root ` locate the labelled-bug manifest and the directory holding the repos it names, and `recall --strict` exits non-zero when a case it expects to still find is missed. Every numeric flag rejects a value that is not a positive integer rather than falling back to its default. +Output contract: + +- **stdout carries only the requested output.** Status lines such as + `wrote baseline to ...` and `persisted to yams as ...`, and warnings such as a + partial scan, go to stderr, so `--format json` output always parses. +- **Every report states its coverage.** `check`, `guide` and `suggest` JSON carry + a `coverage` block `{discovered, selected, resolved, failed, skipped, + complete}`, and the text outputs, the guide brief and the suggest agent plan + open with a one-line coverage note that says PARTIAL whenever a discovered + file was not analysed. `skipped` counts files the analyzer does not cover: + those your `analysis_options.yaml` `exclude:` names, or files outside any + analysis context. The note is part of the brief's header, so budget trimming + never drops it. +- **Exit codes:** 0 success; 1 a gate failed (`check --fail-on`, + `verify --expect-resolved`, `recall --strict`, `index --fail-on-error`, or + `guide --yams` could not persist); 2 pipe could not do what was asked (a bad + path or `--dir`, an unreadable project, a baseline that cannot be read or + written); 64 invalid usage. + `pipe` finds the right SDK itself: it reads `flutterRoot` from the target's `.dart_tool/package_config.json`, then `FLUTTER_ROOT`, then `flutter` on `PATH`. Flutter projects need Flutter's bundled Dart SDK because `dart:ui` is not part of @@ -120,7 +144,8 @@ is an alias for it). `pipe verify` closes the other half of the loop — proving a fix rather than only reporting a problem. It compares a fresh analysis against the baseline and prints what is `resolved`, what `still` remains, and what is `new`, exiting non-zero with -`--expect-resolved` while anything accepted is still reported: +`--expect-resolved` while anything accepted is still reported or was not +scanned: ```text $ pipe verify . --baseline .pipe-baseline.json @@ -135,9 +160,25 @@ RESOLVED Baselines are self-describing (each entry carries the rule plus a readable label), so a resolved finding is named rather than shown as an opaque hash. +`verify` only calls a finding resolved when it looked for it: + +- With `--rule`, accepted findings of other rules are set aside as out of scope + (counted, not judged), because a rule that did not run reports nothing. +- An accepted finding in a file this scan did not analyse — excluded by + `analysis_options.yaml`, filtered by `--no-include-generated`, or outside every + `--dir` — is listed as **NOT SCANNED**, never as resolved. +- A scan in which any file failed to resolve, or one truncated by `--limit`, is + refused outright. + A baseline stores a **fingerprint** per finding (`rule + file + semantic -identity` such as union/variant/provider/owner/member), so moving code down a -file does not resurrect accepted findings. `--fail-on-finding` only counts new +identity` such as union/variant/provider/owner/member; for two swallowed catches +in one method, the catch's index within that method), so moving code does not +resurrect accepted findings. Baselines are written as `"version": 2`. Version-1 +baselines, whose fingerprints embedded a line number for four rules +(`emitted-not-rendered` on a dormant union, `notifier-field-unread`, +`notifier-inert-consumer`, `loading-not-reset`), are still matched, but those +entries stay line-sensitive until you rewrite the baseline with +`--write-baseline`. `--fail-on-finding` only counts new error-severity findings, and severity counts in reports are computed over new findings, so a ratcheted baseline does not keep a build red. @@ -231,8 +272,9 @@ The fixture has a regression guard (`patternView`) so this cannot come back. This is also why the precision numbers above are stated as *factual* precision: they say how often the claim holds, not how often the issue is worth acting on. -Every finding carries `file:line`, a `path:line: snippet` evidence string, a -confidence, a UX severity, and a remediation. If a rule cannot be validated it is +Every finding carries `file:line`, a `path:line: excerpt` evidence string (string +literals elided, at most 120 characters), a confidence, a UX severity, and a +remediation. If a rule cannot be validated it is demoted or deleted, not defended. ### Recall, measured not assumed @@ -257,11 +299,11 @@ Precision alone is not evidence. Two mechanisms pin recall: into a permanent guard against the real code rather than a deleted expectation. -Current result: **5/5 accounted for** (2 open bugs found, 3 fixed bugs still -quiet), 0 regressions — measured against the **working tree**, so a repo checked -out on an unmerged fix branch reports its cases as missed. The two nuwayze cases -are wired up on branches (`wire/nuwayze-*`) that are not merged yet, so they still -read as `open` here; they flip to `fixed` when those land. +Result as of 2026-09-20: **5/5 accounted for** (2 open bugs found, 3 fixed bugs +still quiet), 0 regressions — measured against the **working tree**, so a repo +checked out on an unmerged fix branch reports its cases as missed. At that date +the two nuwayze cases were wired up on branches (`wire/nuwayze-*`) that were not +yet merged, so they read as `open`; they flip to `fixed` when those land. Recall is still measured against a small labelled set. It bounds the risk of false negatives; it does not prove they are absent. @@ -284,9 +326,10 @@ approval rather than offered as an equal choice. `--policy wire-up` (default) / `--policy remove` flips the emphasis for teams that would rather delete. It changes the wording of every remediation that presents a -choice, the `decision:` line in the agent brief, and the heading of the -dead-handler section (`Unreachable handlers - wire up the control` versus -`Cleanly deletable`). +choice, the fixes in the brief's rule legend, the `decision:` line in the agent +brief, the heading of the dead-handler section (`Unreachable handlers - wire up +the control` versus `Cleanly deletable`), and whether `suggest` scores a dead +handler as needing no product judgement (only under `remove`). This is a policy, not a finding: pipe still reports the same facts either way. @@ -298,14 +341,23 @@ the plan is presented both globally and **per file**. Only new findings are planned, so a ratcheted baseline yields an empty plan. ```text +# pipe plan — /path/to/app + +Static heuristics; confirm each against the cited file. Order is a score, not a mandate. +Coverage: 814 of 814 discovered file(s) analysed (0 failed, 0 skipped by analysis_options or context). +146 findings collapse to 58 root cause(s) across 41 file(s). Work them in this order. + 1. [warning/high] emitted-not-rendered — CommentsState (CommentsBloc) - why: 125 = warning 50 x high (x1.0), one fix clears 6 findings (+50), user-visible failure (+25) - fix: Either render CommentsLoading in the consuming widget (or delete it and its emits) ... - where: lib/blocs/comment/comment_bloc.dart:143, ... +2 more - clears 6 finding(s) (CommentSubmitError, CommentSubmitSuccess, CommentSubmitting, - CommentsError, CommentsLoaded, CommentsLoading); verify with `pipe check --rule emitted-not-rendered` + why: 75 = warning 50 x high (x1.0), user-visible failure (+25) + fix: Wire it up: decide whether the feature ships, and if it does, mount CommentsBloc in a screen and render its states. Removing the bloc and its DI registration is the exception and needs explicit approval. + where: lib/blocs/comment/comment_bloc.dart:143 + clears 1 finding(s) (CommentSubmitError, CommentSubmitSuccess, CommentSubmitting, CommentsError, CommentsLoaded, CommentsLoading); verify with `pipe check /path/to/app --rule emitted-not-rendered` ``` +(Abbreviated and illustrative: the counts are not from a single run. The line +shapes, the wording and the score arithmetic are what `suggest --format agent` +prints; a dormant union is one finding, so it scores no breadth bonus.) + The score is deliberately simple and always printed, so the order can be argued with rather than trusted blindly: @@ -315,7 +367,7 @@ with rather than trusted blindly: | confidence | high ×1.0, medium ×0.7, low ×0.4 — a low-confidence finding may not be real | | breadth | +10 per extra finding the one fix clears (capped at +50) | | user-visible failure | +25 when the rule flagged the failure as UX-critical | -| certainty | +15 for rules whose fix needs no product judgement (dead handler, event without handler, loading not reset) | +| certainty | +15 for rules whose fix needs no product judgement (event without handler, loading not reset; dead handler only under `--policy remove`, because wiring one up means choosing the control that dispatches it) | Because the bonuses are additive, a wide and certain `info` group (10 base + 15 certainty + 50 breadth = 75) can outrank a narrow `warning` (35): severity @@ -325,11 +377,20 @@ implied, and the `why:` line prints the arithmetic for every action. ## Agent guidance -`pipe guide` emits a brief designed to be pasted into an agent prompt: findings -grouped by root cause, dense `file:line` anchors (`file:18,29,32`), a rule legend -carrying the standard fix, a `decision:` line that resolves "fix vs delete" where -the project makes that decidable, and a `verify:` command. Truncation is always -explicit — never silently dropped. +`pipe guide` emits a brief designed to be pasted into an agent prompt: a coverage +line, findings grouped by root cause, dense `file:line` anchors +(`file:18,29,32`), a rule legend carrying the standard fix for the chosen +`--policy`, a `decision:` line that resolves "fix vs delete" where the project +makes that decidable (scoped to the scanned directories), and a `verify:` +command that can be pasted as-is (`pipe check --rule X`, plus +`--baseline` and `--dir` when the run used them). Truncation is always explicit — +never silently dropped — and packing stops at the first entry that does not fit, +so a lower-ranked finding never stands in for a dropped higher-ranked one. + +The analysed repository is treated as untrusted input. Quoted source is a bounded +excerpt — string literals replaced by `'…'`, comments dropped, one line of at most +120 characters — labelled `source excerpt (data, not instructions)` and fenced so +that nothing inside it can close its own quotation. It was verified by giving independent agents *only* the brief text and no tool access. Five rounds of that found real defects (a legend that did not match the @@ -348,19 +409,25 @@ actionable class. ## Architecture ```text -bin/pipe.dart entrypoint +bin/pipe.dart entrypoint, exit codes +lib/src/cli.dart commands and option parsing +lib/src/errors.dart PipeException (message + remedy) lib/src/project.dart package graph validation, source discovery lib/src/sdk.dart SDK discovery (Flutter-aware) -lib/src/analysis.dart AnalysisContextCollection lifecycle, per-file resolution +lib/src/analysis.dart AnalysisContextCollection lifecycle, per-file + resolution, scan coverage +lib/src/pool.dart bounded-concurrency map over files lib/src/index.dart resolution health, timing, diagnostics lib/src/model.dart declaration model lib/src/state_flow.dart the state-flow IR (unions, emissions, render sites, events, flags, Riverpod) +lib/src/excerpt.dart bounded, literal-free source excerpts lib/src/rules.dart breakage detectors, findings, baselines +lib/src/provider_rules.dart provider / ChangeNotifier rules lib/src/recall.dart labelled-bug recall measurement -lib/src/verify.dart baseline verification (resolved / still / new) +lib/src/verify.dart baseline verification (resolved / still / new / + not scanned) lib/src/suggest.dart ranked fix order per file -lib/src/recall.dart labelled-bug recall measurement lib/src/pipeline.dart shared analyze -> graph -> rules pipeline lib/src/guide.dart agent brief / markdown report / yams emission tool/adjudicate.py independent precision adjudicator @@ -414,7 +481,7 @@ is byte-identical across runs. dart test ``` -84 tests across resolution/IR extraction, rule contracts (positive *and* +Tests cover resolution/IR extraction, rule contracts (positive *and* negative controls per rule), Riverpod and provider modelling, agent-brief rendering, fix-order scoring, baseline round-tripping (including legacy fingerprints-only baselines), verification, and recall evaluation, against a diff --git a/bin/pipe.dart b/bin/pipe.dart index 7f4b08e..4389825 100644 --- a/bin/pipe.dart +++ b/bin/pipe.dart @@ -8,7 +8,8 @@ import 'package:pipe/src/errors.dart'; /// /// - 0: success. /// - 1: a gate failed: `check --fail-on`, `verify --expect-resolved`, -/// `recall --strict`, or `guide --yams` could not persist. +/// `recall --strict`, `index --fail-on-error`, or `guide --yams` could not +/// persist. /// - 2: pipe could not do what was asked (a [PipeException], or a filesystem /// error such as an unreadable or unwritable path). /// - 64: invalid command-line usage. diff --git a/recall/manifest.json b/recall/manifest.json index 10a025f..5b6c553 100644 --- a/recall/manifest.json +++ b/recall/manifest.json @@ -31,7 +31,7 @@ "match": { "provider": "subscriptionStreamProvider" }, - "note": "fixed on branch fix/subscription-stream-entitlement (PR #38): the tier is now read from confirmed AsyncData only. Guard holds while that branch is checked out; becomes a default-branch guard once merged.", + "note": "fixed on branch fix/subscription-stream-entitlement (PR #38): the tier is now read from confirmed AsyncData only. | As of 2026-09-20 that branch was unmerged, so the guard holds only while it is checked out; it becomes a default-branch guard once merged.", "status": "fixed" }, { -- 2.52.0 From edaaec0a9434cb99faa43b406c5145920b0f1455 Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:04:25 -0600 Subject: [PATCH 08/11] chore: align the hooks, PR template, CODEOWNERS and commitlint with the repo - The pre-push hook skipped on PIPE_SKIP_PREPUSH_CI_GATE while the gate script it runs skipped on PIPE_SKIP_PREPUSH_GATE, so each name worked in only one place. Both now use PIPE_SKIP_PREPUSH_GATE. - The PR template asked for a hand-written CHANGELOG entry; release-please generates CHANGELOG.md from commit subjects, so the item now says that. It also asked for a "Stability note" the repo does not have; removed. - CODEOWNERS listed /docs/**, which does not exist. - commitlint accepted `merge` but rejected `security`, a type release-please-config.json maps to its Security section; `security` is now allowed. --- .githooks/pre-push | 7 +++++-- .github/CODEOWNERS | 1 - .github/PULL_REQUEST_TEMPLATE.md | 7 ++++--- commitlint.config.js | 1 + 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.githooks/pre-push b/.githooks/pre-push index 5754429..59a15f0 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -11,8 +11,11 @@ while IFS= read -r git_env_name; do done <<< "$git_local_env" unset git_local_env git_env_name -if [ "${PIPE_SKIP_PREPUSH_CI_GATE:-0}" != "1" ]; then +# One skip variable, PIPE_SKIP_PREPUSH_GATE, shared with the gate script. The +# hook used to check PIPE_SKIP_PREPUSH_CI_GATE while the script checked +# PIPE_SKIP_PREPUSH_GATE, so each documented name worked in only one place. +if [ "${PIPE_SKIP_PREPUSH_GATE:-0}" != "1" ]; then "$REPO_ROOT/scripts/pre-push-ci-gate.sh" else - echo "[pre-push] PIPE_SKIP_PREPUSH_CI_GATE=1; skipping local CI gate" >&2 + echo "[pre-push] PIPE_SKIP_PREPUSH_GATE=1; skipping local CI gate" >&2 fi diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index b84d7c4..0ff9644 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -6,6 +6,5 @@ /tool/** @trvon /bin/** @trvon /test/** @trvon -/docs/** @trvon /.github/** @trvon /scripts/** @trvon diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 28e84d2..ef45574 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -23,9 +23,10 @@ - [ ] Recall manifest still passes (`pipe recall --root --strict`) ## Documentation -- [ ] CHANGELOG updated (user-visible changes) -- [ ] README updated (rules table / precision numbers) -- [ ] Stability note updated (if interfaces change) +- [ ] CHANGELOG entry: automated — release-please builds it from the + Conventional Commit subjects, so check those read well; do not edit + CHANGELOG.md by hand +- [ ] README updated (rules table / precision numbers / flags and output) ## AI usage - [ ] No AI used diff --git a/commitlint.config.js b/commitlint.config.js index a525b77..3528f20 100644 --- a/commitlint.config.js +++ b/commitlint.config.js @@ -8,6 +8,7 @@ module.exports = { [ 'feat', // New feature 'fix', // Bug fix + 'security', // Security fix (release-please "Security" section) 'perf', // Performance improvement 'docs', // Documentation 'chore', // Maintenance (deps, CI config, etc.) -- 2.52.0 From b304b61a4832f8414027ba45f7939d0e5d93c45b Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:10:11 -0600 Subject: [PATCH 09/11] test(suggest): match the judgement label the scorer prints --- test/suggest_test.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/suggest_test.dart b/test/suggest_test.dart index 2472216..5234534 100644 --- a/test/suggest_test.dart +++ b/test/suggest_test.dart @@ -345,10 +345,7 @@ void main() { ), ]), ); - expect( - plan.toAgentBlock(), - contains('Order is a score, not a mandate.'), - ); + expect(plan.toAgentBlock(), contains('Order is a score, not a mandate.')); }); test('wiring up a dead handler is not scored as judgement-free', () { @@ -371,7 +368,10 @@ void main() { 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.why, + contains('no product judgement needed (+15)'), + ); expect(remove.ordered.single.score, wireUp.ordered.single.score + 15); }); -- 2.52.0 From 6ac40b04c45e803f918b98194269fe7aa1f2a635 Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:10:13 -0600 Subject: [PATCH 10/11] chore: apply dart format to the files this branch changed --- lib/src/guide.dart | 8 +------- lib/src/suggest.dart | 3 ++- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/src/guide.dart b/lib/src/guide.dart index 59a4c72..cac9971 100644 --- a/lib/src/guide.dart +++ b/lib/src/guide.dart @@ -446,13 +446,7 @@ Guidance buildGuidance({ for (var i = 0; i < groups.length; i++) { final block = selected.length >= maxFindings ? null - : _groupBlock( - groups[i], - selected.length + 1, - policy, - scope, - verifyFor, - ); + : _groupBlock(groups[i], selected.length + 1, policy, scope, verifyFor); if (block == null || runningLength + block.length + noteReserve > budgetChars) { // Stop at the first entry that does not fit. Skipping it and packing the diff --git a/lib/src/suggest.dart b/lib/src/suggest.dart index 686cbe9..e25f8d4 100644 --- a/lib/src/suggest.dart +++ b/lib/src/suggest.dart @@ -383,7 +383,8 @@ String _actionOf(List group, List targets, FixPolicy policy) { '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.', + _ => + '${ruleFixFor(policy)[head.rule] ?? single} Applies to each of: $list.', }; } -- 2.52.0 From dd74828e1d5137fb3b364179f5c19e1c6d011796 Mon Sep 17 00:00:00 2001 From: trvon Date: Tue, 22 Sep 2026 12:13:39 -0600 Subject: [PATCH 11/11] fix(suggest): drop "work them in this order" from the agent plan 7ee8020 added the caution that the order is a score, not a mandate, but the summary line two lines below still told the agent to work them in order. The README fix-order example was hand-written with invented counts; it is now the real output of the plan for the stuck_loading fixture. --- README.md | 28 +++++++++++++++++----------- lib/src/suggest.dart | 2 +- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 98502ec..ee8b45b 100644 --- a/README.md +++ b/README.md @@ -341,22 +341,28 @@ the plan is presented both globally and **per file**. Only new findings are planned, so a ratcheted baseline yields an empty plan. ```text -# pipe plan — /path/to/app +# pipe plan — test/fixtures/stuck_loading Static heuristics; confirm each against the cited file. Order is a score, not a mandate. -Coverage: 814 of 814 discovered file(s) analysed (0 failed, 0 skipped by analysis_options or context). -146 findings collapse to 58 root cause(s) across 41 file(s). Work them in this order. +Coverage: 12 of 12 discovered file(s) analysed (0 failed, 0 skipped by analysis_options or context). +20 findings collapse to 16 root cause(s) across 8 file(s). -1. [warning/high] emitted-not-rendered — CommentsState (CommentsBloc) - why: 75 = warning 50 x high (x1.0), user-visible failure (+25) - fix: Wire it up: decide whether the feature ships, and if it does, mount CommentsBloc in a screen and render its states. Removing the bloc and its DI registration is the exception and needs explicit approval. - where: lib/blocs/comment/comment_bloc.dart:143 - clears 1 finding(s) (CommentSubmitError, CommentSubmitSuccess, CommentSubmitting, CommentsError, CommentsLoaded, CommentsLoading); verify with `pipe check /path/to/app --rule emitted-not-rendered` +1. [error/high] event-without-handler — HandlerGapEvent (GapBloc) + why: 115 = error 100 x high (x1.0), no product judgement needed (+15) + fix: Add `on(...)` to GapBloc, or stop dispatching UnhandledTap. + where: lib/gaps.dart:74 + clears 1 finding(s) (UnhandledTap); verify with `pipe check test/fixtures/stuck_loading --rule event-without-handler` +2. [warning/high] loading-not-reset — ItemBloc.twoSwallowedAwaits + why: 85 = warning 50 x high (x1.0), one fix clears 3 findings (+20), no product judgement needed (+15) + fix: Emit a failure/timeout state in the catch (or rethrow) so every loading state is resolved. Applies to each of: , twoSwallowedAwaits. + where: lib/item_bloc.dart:35, lib/item_bloc.dart:40, lib/item_bloc.dart:53 + clears 3 finding(s) (, twoSwallowedAwaits); verify with `pipe check test/fixtures/stuck_loading --rule loading-not-reset` +14 further cause(s) omitted to stay within 1500 characters. Run `pipe suggest` for the full plan. ``` -(Abbreviated and illustrative: the counts are not from a single run. The line -shapes, the wording and the score arithmetic are what `suggest --format agent` -prints; a dormant union is one finding, so it scores no breadth bonus.) +(Real output of `pipe suggest test/fixtures/stuck_loading --format agent +--budget 1500`, run from the repository root. The budget cut the plan to its top +two causes; the omission line says so.) The score is deliberately simple and always printed, so the order can be argued with rather than trusted blindly: diff --git a/lib/src/suggest.dart b/lib/src/suggest.dart index e25f8d4..bb901a9 100644 --- a/lib/src/suggest.dart +++ b/lib/src/suggest.dart @@ -167,7 +167,7 @@ class WorkPlan { ..write(coverage == null ? '' : '${coverage!.describe()}\n') ..writeln( '$totalFindings findings collapse to $totalCauses root cause(s) across ' - '${files.length} file(s). Work them in this order.', + '${files.length} file(s).', ) ..writeln(); final actions = ordered; -- 2.52.0