[milindsrivastava1997]

Summary

asap_planner's text output is terse by default. It prints the memo structures with a legend for each stage, and the details move behind --verbose (renamed from --printing-full-candidates). On quantile-grouped.json --run-until stage3 the output drops from 1270 lines to 499. Closes #1261.

Review guide

Everything is in crates/devtools/src/planning_stages_text.rs. Suggested order:

1. section and the *_LEGEND constants: the legends, one label per line.
2. write_memo: the used by / reached by / grid column, and the properties line now printed only with --verbose.
3. write_stored_logical_edges: Pass 2's default edge lines.
4. write_physical_by_logical_candidate and write_physical_pruned: Stage 2's default layout and the pruned counts.

Behavior: before → after

Stage 2, candidate C4 (quantile-grouped.json):

before:
    C4 = G1[0] (QueryTime, None)  TimeRange 300s
        input 0 <- G0:
            C0 = G0[0] (QueryTime, None)                           ✓   selected
            C1 = G0[0] (QueryTime, OneEvaluation(Memory))          ✓
            C2 = G0[0] (IngestionTime, None)                       ✗ α^phys = 0: consumer (QueryTime, None) cannot read producer (IngestionTime, None)
            C3 = G0[0] (IngestionTime, AcrossEvaluations(Memory))  ✓
after:
G1[0]  TimeRange 300s <- G0  exact
    C4  (QueryTime, None)  ★
        input 0 <- G0: reads C0..C1, C3 · uses C0

Memo groups, before and after:

before:
G4:   SummaryAgg Kll   <- G1
    ts=data.ts:Unknown state=computed:NonNegative region=data.region:Unknown $promql_series_identity=... · reads only time series
after:
G4:   SummaryAgg Kll   <- G1   used by G5 · reached by q1 · every 10000 ms

Interfaces

- asap_planner ... [--printing-full-candidates]
+ asap_planner ... [--verbose]

- pub fn planning_stages_text(stages, evidence, plan_cost, printing_full_candidates: bool) -> String
+ pub fn planning_stages_text(stages, evidence, plan_cost, verbose: bool) -> String

Algorithm changes

None. Planning is unchanged; only the text rendering changes.

Decisions and open questions

• One flag. --verbose replaces --printing-full-candidates, with no alias.
• Memo structures, not recomputed ones. By default:
  • Pass 2 prints the stored LogicalASAPMemoCandidates.edges, as producer ids grouped by whether they meet the accuracy requirement.
  • The recomputed reason for each producer is printed only with --verbose.
  • used by / reached by come from MemoGroup.parents / queries.
• θ folding stays. Candidates that differ only in θ are still folded by default. The memo stores each θ as its own candidate; The memo stores every θ variant as its own candidate, instead of a compressed set #1274 discusses that.
• Stage 3's pruned list is now counts per cause, matching Pass 2 and Stage 2.
• Open. The JSON output still uses the recomputed edges, and the user-facing strings still say α^log / α^phys. Both are tracked in asap_planner and viewer: Pass 2 edges are recomputed instead of read from the stored list, and are labeled with α symbols #1275.

Verification

• New test the_default_text_is_the_terse_form_and_verbose_adds_the_details. It checks that:
  • the default output has no properties line, per-producer rows or α^ reasons, and --verbose has all of them;
  • every section starts with a legend;
  • every group shows reached by;
  • Stage 2 lists each physical candidate exactly once, in id order;
  • ★ marks exactly the plan's candidates.
• Existing tests updated:
  • the_text_memo_after_pass_2_prints_every_group_with_its_properties and the_text_folds_candidates_that_differ_only_in_theta: flag renamed to --verbose.
  • stage3_selects_candidates_stage2_lists: matches the new Stage 2 line shape.
• Ran cargo test -p asap-devtools --test devtools asap_planner::the_ asap_planner_acceptance, the devtools lib tests, and cargo clippy -p asap-devtools --tests.
• Planner snapshots (crates/devtools/tests/corpus_snapshots/) are unchanged; they are JSON.

Follow-ups

• asap_planner and viewer: Pass 2 edges are recomputed instead of read from the stored list, and are labeled with α symbols #1275: JSON edges from the stored list; plain-English names in place of α^log / α^phys.
• The memo stores every θ variant as its own candidate, instead of a compressed set #1274: θ variants stored as separate candidates.
• Pass 2's default edge lines scan the whole edge list for each input (marked ponytail:). Index them by (consumer, input) if printing gets slow on large memos.

🤖 Generated with Claude Code

https://claude.ai/code/session_014qhwq6eHKg6PF5nMxwGhWG

[milindsrivastava1997]

Code review: 10 findings. The first three (prune reason, ✗ list, verbose legends) change what users see.

[milindsrivastava1997]

Misleading prune reason. For a candidate Pass 2 pruned, LogicalASAPMemoCandidates::edges keeps no edges (edges exist only between surviving candidates), so this prints input i <- G<k>: no surviving producer even when G has many surviving producers that meet the requirement. The real cause is that the consumer itself was pruned. This label is also not in PASS2_LEGEND.

[milindsrivastava1997]

Fixed in f0d4764. Pass 2 keeps no edges into a pruned candidate, so a candidate marked pruned now lists no input lines; its pruned flag already gives the reason. The legend says so ("a pruned candidate lists no inputs: Pass 2 keeps no edges into it"). The terse-text test checks that no input line follows a pruned candidate.

[milindsrivastava1997]

Legend overstates the terse ✗ list. The default path omits producers Pass 2 pruned and producers of another θ, and the old "N pruned producers" count was removed. E.g. G7 with 280 candidates, 270 pruned for BelowRequiredAccuracy, shows ✓ G7[..] with no hint that 270 producers are missing. Either restore a count or narrow the legend.

[milindsrivastava1997]

Fixed in f0d4764, with a count rather than only a narrower legend. Each line now ends with · N other(s): the candidates of the input group that have no stored edge, because they were pruned or have another θ. For example: input 0 <- G2: ✓ G2[0, 2] · 1 other. The legend now says the ✓/✗ entries are the edges Pass 2 kept, and that --verbose lists each other producer with the reason.

[milindsrivastava1997]

Legends don't depend on --verbose. Verbose output prints C3 = G1[0] ... ✓ selected rows and no ★, reads or uses, but gets the same STAGE2_LEGEND/PASS2_LEGEND, which describe ★/reads/uses and not selected, ✓, ✗ or C<n> = G<g>[<i>]. Same applies to the Stage 2 legend at L146.

[milindsrivastava1997]

Fixed in f0d4764. With --verbose, Pass 2 gets PASS2_VERBOSE_EDGES_LEGEND: one row per candidate of the input group, ✓ / ✗ with the reason / no edge. Stage 2 gets STAGE2_VERBOSE_LEGEND: C<n> = G<g>[<i>] (timing, lifetime), ✓ / ✗ with the reason, and selected. The terse legends are unchanged.

[milindsrivastava1997]

Assumes contiguous physical candidates per logical candidate. A header is printed whenever logical_candidate_id changes. Stage 2's catalog builds them contiguously today, but PhysicalASAPMemoCandidates::new neither enforces nor documents this (stage3 cost.rs/select.rs fixtures construct it directly). Interleaving would print the same G1[0] header twice, breaking the "each logical candidate once" promise in the user docs. Group explicitly, or document/assert the invariant.

[milindsrivastava1997]

Fixed in f0d4764. write_physical_by_logical_candidate now builds a BTreeMap<CandidateForMemoGroupId, Vec<usize>> first, so each logical candidate prints once whatever order Stage 2 lists them in. The test now checks that the set of listed C<n> is exactly 0..count, instead of relying on id order.

[milindsrivastava1997]

O(candidates × edges) on the default path. This scans all of logical.edges for every (consumer, input). With ~280 θ variants per group the edge list is already large. The edges are grouped by (c, i), so a partition_point/chunk_by lookup or one precomputed map would make it linear. The ponytail: note defers this, but terse is now the default.

[milindsrivastava1997]

Fixed in f0d4764. write_memo buckets logical.edges once into a BTreeMap<(consumer, input), Vec<&LogicalEdge>>, which write_stored_logical_edges looks up. That makes it linear, and the ponytail: note is gone.

[milindsrivastava1997]

Unused work in terse mode. edges_per_input(physical) is built here (and accepted_metrics(&logical.memo) at L423) even when verbose is false, then discarded. Compute both only inside the verbose branch.

[milindsrivastava1997]

Fixed in f0d4764. edges_per_input is built only inside write_physical_edges (the verbose path), and accepted_metrics only when verbose. Likewise, the stored-edge map is built only when not verbose.

[milindsrivastava1997]

Nit: .enumerate().filter(|_| verbose) after the terse writer call is a hidden if/else. An explicit if verbose { ... } else { write_physical_by_logical_candidate(..) } reads more clearly.

[milindsrivastava1997]

Fixed in f0d4764. write_physical is now an explicit if verbose { write_physical_edges(..) } else { write_physical_by_logical_candidate(..) }, followed by the shared pruned list.

[milindsrivastava1997]

Nit: duplicates grid_text (L310) with different wording ("no known grid" vs "no known evaluation grid", "every N ms" vs "evaluated every N ms at 0"). The same group's grid is phrased differently depending on --verbose. Reuse grid_text, perhaps with a short variant.

[milindsrivastava1997]

Fixed in f0d4764. There's one grid_text, used by both modes: every 10000 ms, every 10000 ms at 5000 when the phase is nonzero, or no known evaluation grid.

[milindsrivastava1997]

Nit: dead parameter. This is now the only call site of write_logical_edges and passes folded = None, so the pruned-producer/other-θ counting branches and the "and N pruned producers" output (L639–660) are unreachable, and the doc comment at L614 describes behavior that can't happen. Remove the parameter and its branches.

[milindsrivastava1997]

Fixed in f0d4764. Removed the folded parameter, the pruned/other-θ counting branches, and the "and N pruned producers" output. The doc comment now describes only the recomputed rows.

[milindsrivastava1997]

Fragile count. Counting Stage 3 lines starting with n and asserting it equals the number of ★ candidates assumes every plan node has selected_physical_candidate set and no two nodes share a candidate. Either case fails the test although ★ marks exactly the plan's candidates. Compare against the set of C<n> named in the plan lines instead.

[milindsrivastava1997]

Fixed in f0d4764. The test now collects the set of C<n> named in the plan lines (n<k>: C<c> = ...) and asserts it equals the set of ★ candidates.