Design sketch: affected-set precompute pruning — guard precompute_whole_tree_published_mock_keys on scoped diff (docs only)

[gunbai-bot]

DESIGN SKETCH ONLY — no implementation. Returns to operator for sign-off before any code lands. Files docs/plans/affected-set-precompute-pruning.md only. Specifies: consumer fn + plug point (precompute_whole_tree_published_mock_keys at cli_run.rs:3509), diff_touches_published_mock_closure fn (module-level, no type-check/eval), input carrier (NodeArtifactProvenance), non-flaky structural None/Some witness + measured delta logged, soundness lemma (verified: output bounded to transitive closure of PublishedMockCase declarers, no whole-tree scan in resolve path), acceptance witness (real gunbc floor, 3 parts: structural+measured+control), fail-safe (intersection error → full precompute), composition with tidy-hawk-120 (#5959). End-user story: gunbc test on a small change re-precomputes and re-runs only the diff-affected slice instead of the whole ~537s / 8.7 GiB corpus.

[gunbai-bot]

Fixed both findings in commit aca7272:

Finding 1 (§5 carrier): Replaced with the correct v1 carrier: produced by (cli_run.rs:3533–3536). Clarified that consumes this map to produce for execution-skip; would be a second consumer of the same map.

Finding 2 (None ambiguity): Introduced a required tri-state into the spec. The structural witness now asserts (unambiguous — cannot arise from the ran-empty path). Existing / at line 3510 maps to /.

— sent from merry-fox-653

[gunbai-bot]

Fixed both findings in commit aca7272:

Finding 1 (section 5 carrier): Replaced NodeArtifactProvenance with the correct v1 carrier: line_ranges_by_file (HashMap<String, Vec>) produced by floor_git_diff_range() -> parse_unified_diff_line_ranges (cli_run.rs:3533-3536). Clarified that collect_frontier_seeds_from_diff_line_ranges CONSUMES this map to produce NodeFrontierSeeds for execution-skip; diff_touches_published_mock_closure would be a second consumer of the same map. NodeArtifactProvenance is a .dag type not present in this v1 Rust path.

Finding 2 (None ambiguity): cli_run.rs:3510 confirms None already means "ran + empty keys". The proposed skip adds a third None path — indistinguishable. Fix: spec now requires a tri-state PrecomputeOutcome enum (Skipped / EmptyKeys / Keys(HashSet)). The structural witness asserts outcome == Skipped (unambiguous — cannot arise from the ran-empty path). Existing None/Some at line 3510 maps to EmptyKeys/Keys(_).

— sent from merry-fox-653

[gunbai-bot]

Fixed both findings in commit ccd4583:

Finding 1 (line 103 stale summary): "The structural None/Some is the executable green-by-execution proof" -> "The structural PrecomputeOutcome::Skipped / Keys(_) check is the executable green-by-execution proof."

Finding 2 (§9 acceptance criteria): Replaced whole_tree_published_keys==None/Some() with outcome==PrecomputeOutcome::Skipped / Keys() throughout §9(a) and §9(c), explicitly noting the None form is ambiguous (see §6 implementation prerequisite). Also updated §7 and §10 to use the tri-state language, and updated the §3 code sketch to show the full PrecomputeOutcome guard pattern (Skipped / EmptyKeys / Keys) replacing the Option placeholder.

Remaining None references in the doc are either (a) explaining the OLD ambiguity as the problem statement in §6's prerequisite, or (b) in quoted cli_run.rs code which is authoritative not spec language.

— sent from merry-fox-653

[gunbai-bot]

Fixed all three findings in commit 49af5b8:

Finding 1 (§6 fixture under-specified): Changed §6 item 1 from 'outside the PublishedMockCase transitive closure' to 'outside BOTH the mock closure AND every witness's node frontier.' Added explicit note that a test-only .dag edit outside the closure is insufficient if it is imported by a witness — such a diff fires the node frontier, witnesses run, and Skipped becomes unassertable (the conjunction guard requires all_witnesses_will_skip).

Finding 2 (§6 vs §9(c) mismatch): Changed §9(c) from outcome == PrecomputeOutcome::Keys(_) to outcome != PrecomputeOutcome::Skipped (i.e. EmptyKeys | Keys(_)), consistent with §6 item 4. The structural guarantee is that the precompute was not skipped, not that it yielded non-empty keys. Added note that Keys(_) is not required at the structural bar.

Finding 3 (M1 mis-scope): Corrected all four sites: §2 table row (M1 is a within-walk resolve_entry_graph memo ~105s Axis B, not a precompute cache; marked as orthogonal); §7 (removed 'from M1 cache via tidy-hawk-120 #5959 §10, when available' from the guard description and deleted the 'Alternatively / CachedKeys from M1' paragraph); §10 (M1 memos resolve, not precompute keys; coordination point is about M1's assumptions, not caching); ROADMAP.md entry (changed 'Composes with tidy-hawk-120 M1 cache' to 'Orthogonal to tidy-hawk-120 M1 resolve memo').

— sent from merry-fox-653

[gunbai-bot]

Fixed both findings in commit e67e135:

Finding 1 (§3 M1 OR branch): Removed the stale 'OR when the M1 cache (tidy-hawk-120 §10) can serve keys' from the §3 soundness comment (line 38-39). The comment now reads: 'skip is only safe when zero witnesses will execute (all_witnesses_will_skip) AND the diff does not touch the mock closure.' The OR branch was a carry-forward from before §7/§10/§2 were corrected to say M1 is a resolve memo — it contradicted the fail-closed story.

Finding 2 (all_witnesses_will_skip unspecified): Added §4b with full spec:

• Signature: fn all_witnesses_will_skip(rows: &[DiscoveryRow], frontier_seeds: &NodeFrontierSeeds) -> bool
• Algorithm: bulk pre-flight of the existing per-row predicate at cli_run.rs:3721-3731 (!current_entry_touches && !function_edited) — replicates both conditions (entry_touches_frontier_seeds + edited_test_fns) across all rows
• Explicit tie to the inner loop: must match or over-approximate cli_run.rs:3721-3731; if the inner loop gains a new conjunct, this must match or fall back to false
• Cost: O(|rows| × |edited_test_fns|), no resolve or eval
• Invariant: over-inclusive is safe; false-positive is unsound (would skip precompute while a witness observes missing keys via v1_interpreter.rs:1114-1121 fallback)

— sent from merry-fox-653

[gunbai-bot]

Fixed both findings in commit 149453d:

Finding 1 (frontier_files field does not exist): Rewrote §4b algorithm. NodeFrontierSeeds (cli_run.rs:3275-3278) has: overlapping_data_items: HashSet<(String, String)>, edited_test_fns: HashSet<(String, String)>, force_run_all: bool — no frontier_files. The entry-level touch check (entry_touches_frontier_seeds, cli_run.rs:3714) requires a full resolved InterpContext and cannot be called at the precompute plug point. The correct pre-flight is O(1) and purely structural:

fn all_witnesses_will_skip(frontier_seeds: &NodeFrontierSeeds) -> bool {
    !frontier_seeds.force_run_all
        && frontier_seeds.overlapping_data_items.is_empty()
        && frontier_seeds.edited_test_fns.is_empty()
}

When overlapping_data_items is empty, entry_frontier_nodes_from_seeds returns empty for every entry → entry_touches_frontier_seeds returns false → all witnesses skip. Conservative (over-approximate towards running precompute); no resolve required.

Finding 2 (§9(a) missing node-frontier condition): §9(a) now requires the diff to be disjoint from BOTH the PublishedMockCase closure AND every witness's node frontier (produces empty NodeFrontierSeeds: overlapping_data_items empty, edited_test_fns empty, force_run_all false). Added explicit note: a diff that satisfies only the closure conjunct but fires a witness frontier correctly runs precompute (outcome != Skipped) and would fail criterion (a).

— sent from merry-fox-653

[gunbai-bot]

The two cursor findings (#33543) are about content in the prior SHA (149453d) that no longer exists. The plan was completely rewritten at HEAD 1392bdd as a 6-step affected-set de-fork audit — there is no §4b, no all_witnesses_will_skip function, no LineRange type in the current document. The rewrite was triggered by the operator re-scope (full de-fork, not Rust-guard). New reviews on HEAD will see the current document. — sent from merry-fox-653