Skip to content

docs: add an Interface section to each L1-L5 doc - #169

Merged
zzylol merged 8 commits into
mainfrom
docs/interface-level-l1-l5
Aug 5, 2026
Merged

zzylol merged 8 commits into
mainfrom
docs/interface-level-l1-l5

Conversation

@zzylol

@zzylol zzylol commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #168.

PR #157 rewrote design.md + the per-layer docs to be pure principle/design-level, deliberately stripped of code-level detail. This adds the promised follow-up (per the note on #157: "please just review design.md for overall convergence for now... before we flesh out the API/interface details per layer") — a concrete Rust API surface per layer, added as a new ## Interface section at the end of each doc, on top of the existing principle-level content (which is untouched).

Every signature was verified against current main source before being written down — nothing here is carried over from an old draft or assumed from memory.

Per-layer notes

Test plan

  • Every signature cross-checked against crates/frontend-promql, crates/frontend-sql, crates/l2, crates/ir, crates/sketch, crates/plan on current main.
  • Markdown code-fence balance checked for all 5 files.
  • Docs-only change — no code touched.

🤖 Generated with Claude Code

zzylol added a commit that referenced this pull request Jul 28, 2026
- L3: clarify reduction/GroupKeys/Reduction aren't tree nodes (they're
  plain data reached only through Aggregate.reduction), and expand the
  AggIntent listing from a representative slice to the full vocabulary.
- L4: explain what "Implementation" means (the per-intent binding
  decision's own result type); note the sketch/summary field-naming
  tension (sketch/sketch_input predate this doc's own "summary"
  terminology) and the Sketch/ExactAccumulator merge question as
  deliberate follow-ups, not silently resolved here -- see PR discussion.
- L5: fold "what does edge mean", "StageId values are deployment-owned",
  and "interface only in ASAPController" into one clarification at the
  top of the Interface section, plus reword StageId's own comment so
  its example values don't read as a fixed vocabulary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol

zzylol commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 464241d addressing all 10 review comments from the pending review.

Doc-only fixes (done)

  • L3reduction/Reduction/GroupKeys clarified as plain data on Aggregate itself, not tree nodes; AggIntent expanded from a representative slice to the full ~30-variant vocabulary (including Extension).
  • L4 — added an explanation of what "Implementation" means (the per-intent binding decision's own result type: sketch vs. exact accumulator vs. pass-through).
  • L5 — folded "what does edge mean", "StageId values are deployment-owned, not ASAPController's", and "this is interface-only in ASAPController" into one clarification at the top of the Interface section, and reworded StageId's inline comment so the example stage names don't read as a fixed vocabulary.

Two real code-change proposals (not done — flagged in the doc, not silently executed)

Two comments were actually about the code, not the docs, so I didn't just reword prose to match — I left the doc accurate to what's really shipped today and wrote the proposal inline instead:

  1. Rename sketch/sketch_inputsummary/summary_input on SummaryExpr::SummaryAgg/SummaryDelete/SummaryEstimate (crates/sketch/src/expr.rs). Agree with the reasoning — these names predate the summary/sketch terminology split this doc itself uses. Holding off on executing it in this PR because it's a breaking rename of fields every SummaryExecutor implementer pattern-matches on, and there are two ASAPQuery-backend PRs (feat(sql): support date types and calendar interval expressions #419/feat(sql): lower corr to a two-column extension intent #420) actively being updated against the current field names right now — landing the rename mid-flight would just make both moving targets. Worth its own follow-up PR once those settle.
  2. Merge Implementation::Sketch{..} / Implementation::ExactAccumulator{..} into one Summary{..} variant (crates/plan/src/boundary.rs). Feasible, but only if SummaryKind grows something like is_exact() so bind_summary_agg can still tell the two apart (today the variant tag is that signal — it decides whether a SummaryEstimate readout gets wrapped on afterward). Suggest doing this together with proposal 1 rather than as two separate breaking changes to the same call sites, for the same reason as above.

Let me know if you'd like me to go ahead and implement either of these now, or file them as a follow-up issue instead.

🤖 Generated with Claude Code

Comment thread docs/l3-intent-algebra.md
Comment thread docs/l3-intent-algebra.md
Comment thread docs/l3-intent-algebra.md Outdated
Comment thread docs/l4-summary-bound-ir.md Outdated
Comment thread docs/l4-summary-bound-ir.md Outdated
Comment thread docs/l4-summary-bound-ir.md
Comment thread docs/l4-summary-bound-ir.md Outdated
Comment thread docs/l5-physical-plan.md
Comment thread docs/l5-physical-plan.md Outdated
Comment thread docs/l5-physical-plan.md
Comment thread docs/l3-intent-algebra.md
zzylol added a commit that referenced this pull request Jul 29, 2026
…input; merge Implementation::Sketch/ExactAccumulator (#170)

Both were flagged as follow-ups during PR #169's review of the L4 doc's
Interface section (not yet merged):

- `SummaryExpr`'s `sketch`/`sketch_input` fields predated this codebase's
  "summary" umbrella term (an approximate sketch is one kind of summary,
  alongside an exact accumulator) — renamed to `summary`/`summary_input`
  to match.
- `Implementation::Sketch{kind,params}` and `::ExactAccumulator{kind,params}`
  carried identical shapes; the split existed only so `bind_summary_agg`
  could tell whether to wrap a `SummaryEstimate` readout. Collapsed into
  one `Implementation::Summary{kind,params}`, with a new
  `SummaryKind::is_exact()` recovering the same fact from `kind` alone.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
zzylol and others added 3 commits July 28, 2026 18:19
Fixes #168.

PR #157 rewrote design.md + the per-layer docs to be pure principle/
design-level, stripped of code-level detail on purpose. This adds the
promised follow-up: a concrete Rust API surface per layer, verified
against current `main` rather than carried over from an old draft.

- L1: no shared trait exists -- each front end exposes its own free
  functions (lower_promql*, lower_sql*), converging on the same L3
  QueryExpr return type. Kept thin rather than padded, since that's
  the honest current shape.
- L2: SchemaCatalog (the pluggable schema-source trait) + Binder +
  convert_root.
- L3: QueryExpr's shape (a representative slice, not the full ~24-variant
  enum), Reduction + GroupKeys in full (small, central to this doc's own
  design principles), a representative AggIntent slice, and Schema.
- L4: SummaryExpr/L4Node, the binding entry points + Implementation,
  the CostModel extension point (all default-having methods shown),
  and SummaryExecutor in full -- the real, load-bearing serving-time
  interface just resolved via #163/#164/#165 this session.
- L5: nothing is built yet, so this is explicitly speculative --
  recovered the PhysicalPlanner/TopologyDescriptor/StageAllocator sketch
  an earlier draft of this doc had (before PR #157's final rewrite
  trimmed it), updated to current summary-bound-IR naming (L4Node,
  SummaryKind) and kept the same "target to design against, not an API
  to depend on" framing so it can't be mistaken for shipped code.

Every signature was checked against the actual current source in this
repo (crates/frontend-promql, crates/frontend-sql, crates/l2,
crates/ir, crates/sketch, crates/plan) rather than assumed from memory.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- L3: clarify reduction/GroupKeys/Reduction aren't tree nodes (they're
  plain data reached only through Aggregate.reduction), and expand the
  AggIntent listing from a representative slice to the full vocabulary.
- L4: explain what "Implementation" means (the per-intent binding
  decision's own result type); note the sketch/summary field-naming
  tension (sketch/sketch_input predate this doc's own "summary"
  terminology) and the Sketch/ExactAccumulator merge question as
  deliberate follow-ups, not silently resolved here -- see PR discussion.
- L5: fold "what does edge mean", "StageId values are deployment-owned",
  and "interface only in ASAPController" into one clarification at the
  top of the Interface section, plus reword StageId's own comment so
  its example values don't read as a fixed vocabulary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mplementation merge landed

Both were called out as pending follow-ups in this PR's review; #170
implemented them. Updates the code snippets and prose here to match the
real, current shape instead of describing the old one as still-pending.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the docs/interface-level-l1-l5 branch from 6760821 to f94b6be Compare July 29, 2026 00:19
zzylol and others added 2 commits July 28, 2026 18:22
Review comment on #169: these two were listed with no explanation while
every other category got at least a grouping comment. Both need one
because they don't fit the "reduce to one number per group" shape every
other reducer follows here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s StageId values

The earlier fix for this review comment conflated two different
questions — "what does an edge mean" and "who owns the concrete StageId
values" — and only answered the second. Adds the missing explanation:
an edge is a declared connection in the topology's connectivity graph,
separate from which stages merely exist.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comment thread docs/l3-intent-algebra.md
Comment thread docs/l3-intent-algebra.md Outdated
Comment thread docs/l2-logical-plan.md
Comment thread docs/l3-intent-algebra.md
Comment thread docs/l4-summary-bound-ir.md
Comment thread docs/l3-intent-algebra.md
zzylol and others added 2 commits July 28, 2026 18:36
…s, constraints

- l3: expand QueryExpr from an abbreviated slice to the full, real
  variant list (25 variants, matching crates/ir/src/intent_algebra/query_expr.rs).
- l3: add a source-expression example per AggIntent variant, sourced
  from the accurate PromQL/SQL mappings already in agg_intent.rs's own
  doc comments.
- l3: explain why the *OverTime range reducers can't be composed from
  Sort/Limit or any other generic node (no node exposes a TimeRange
  window's raw sample sequence as queryable rows).
- l3: add a worked Schema example (sum by (job) (...)), showing the
  input/output shape difference an aggregate produces.
- l2: add a concrete usage example for SchemaCatalog/Binder (PromQL's
  usage-derived default vs. a SQL catalog-backed case).
- l4: list the per-SummaryExpr-node validity constraints (subtractable/
  deletable/mergeable catalog flags, state-vs-value child requirements),
  correcting along the way which nodes actually carry a `summary` field
  of their own vs. which infer their kind from a child's output type.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…diagram

Prose bullet list was hard to scan against 7 variants. Table gives a
one-glance summary; the mermaid diagram visualizes the rule that's
hardest to hold in your head from prose alone (SummaryMerge only
accepts state-producing children, never a value-producing one).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@milindsrivastava1997

Copy link
Copy Markdown
Collaborator

@zzylol L1 and L2 lgtm. Haven't read through the rest. Do you just want me to do a high-level pass or do you want detailed comments?

@zzylol

zzylol commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@zzylol L1 and L2 lgtm. Haven't read through the rest. Do you just want me to do a high-level pass or do you want detailed comments?

High-level pass is okay. If you have time, detailed comments are appreciated as well.

* docs(l4): design proposals for #171 and #172 composition gaps

Stacked on #169's Interface section. Adds two concrete proposals:

- #171 (exact <-> summary composition, either nesting order): a new
  SummaryFold node for an outer exact fold over an inner realized
  summary, computed generically by the shared execute walk with no new
  SummaryExecutor method; and a relaxed SummaryAgg input-column rule
  letting an outer summary read directly off an exact accumulator's own
  state (zero added error), while explicitly refusing the approximate-
  child case since that's #172's gap, not this one's.

- #172 (nested approximate-over-approximate error propagation): a
  SummaryKind::implied_accuracy inverse-sizing function, a new
  CostModel::size_params child_accuracy parameter (opt-in, default
  ignores it, matching this trait's existing default-body convention),
  and a paired accepts_nested_approx gate so an outer approximate
  summary over an approximate child's estimate refuses by default
  instead of silently double-approximating.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): reframe #171/#172 proposal as a shared accuracy algebra, fold in #173

Replaces the earlier implementation-narrative version (which described
specific match arms/functions in boundary.rs/bind.rs) with a design-level
treatment: a formal (epsilon, delta) accuracy guarantee every SummaryKind
already satisfies, a composition theorem (triangle inequality + union
bound) for building a summary over another summary's output, and clean
interfaces (Accuracy, ColumnRef::FromSummary, CostModel::size_params/
accepts_nested_approx, FoldOp/SummaryFold) derived from it.

Shows #171 direction 2 (exact child) and #172 (approximate child) are the
same composition formula at Accuracy::EXACT vs. Accuracy > 0, rather than
two separate rules. Folds in #173: generalizes FromSummary/SummaryFold to
N children for Hydra-style sketch-of-sketches, and adds a Bounded Accuracy
variant (deterministic interval radius) for QTree-style range trees
alongside the Probabilistic (epsilon, delta) variant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): separate the design from which issue it solves; norm-aware composition

Two changes, both from review:

- Restructure into "the design" (guarantee, composition theorem, three
  interfaces — no issue numbers threaded through the reasoning) followed
  by a single "Which issue this solves" section mapping each piece back
  to #171/#172/#173, instead of interleaving issue callouts throughout.

- The composition theorem assumed a single scalar Lipschitz constant L
  under an implicit pointwise/L1 error model. That's wrong for this
  catalog's own CountSketch/CountSketchWithHeap, whose guarantee is
  stated against the L2 norm of the residual frequency vector, not L1 --
  a materially different, not-directly-comparable quantity on skewed
  data. Accuracy and compose now carry an explicit ErrorNorm; compose
  returns None on a norm mismatch instead of silently assuming L=1.
  Folded Accuracy::Bounded (deterministic radius, for #173's QTree case)
  into the same enum/compose function rather than a separate type.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): replace unified accuracy algebra with three grounded composition patterns

Surveyed how real systems actually compose summaries (DGIM SICOMP'02,
UnivMon SIGCOMM'16, Hydra VLDB'22, PromSketch VLDB'25) rather than
inventing one generic accuracy algebra. Finding: there is no single law
-- three structurally distinct patterns exist, only two have a governing
theorem, and the pattern #171/#172 actually describe has none at all.

- Pattern A (same statistic across partitions -- EH/DGIM windowing):
  real, DGIM Theorem 6/7 grounds it precisely, but composes the same
  statistic across a partition of the same data, not different
  statistics along a query's data-flow -- out of scope for #171/#172/#173,
  flagged as a separate future gap (sliding-window range-vector queries).

- Pattern B (hash-collision routing -- Hydra's sketch of sketches):
  Theorem 2's asymmetric bound (multiplicative + additive-vs-global-total)
  doesn't fit the shared Accuracy shape -- Hydra and QTree each need their
  own SummaryKind fed directly from raw data, not a generalization of
  FromSummary/SummaryFold. This replaces the previous (incorrect) N-ary
  FromSummary framing for #173.

- Pattern D (heterogeneous sequential nesting -- what #171/#172 actually
  ask about): no borrowed theorem exists for this shape in any of the
  four systems surveyed -- all four avoid it structurally. Split into
  D1 (inner exact, composes free -- #171 direction 2), D2 (outer exact
  fold over inner summary -- #171 direction 1, SummaryFold), and D3
  (outer sketch over an inner statistic with no exact sub-linear form --
  #172's actual remaining scope, narrower than "any nested approximation").
  D3's compose()/accepts_nested_approx gate is now labeled honestly as
  this design's own first-principles proposal, not a literature result --
  defaulting closed matches what every system surveyed actually does
  (none of them re-sketch another sketch's readout).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): recipe (not formula) for compound summary bounds, state not readout

Reframes the design around the actual structural move every surveyed
system (DGIM, UnivMon, Hydra, PromSketch) makes: never compose past a
readout -- build the outer structure directly over the inner's state.

Replaces the "three patterns" framing with a four-type taxonomy (same-
kind merge; inner-exact; heterogeneous-with-derivable-bound; heterogeneous-
undecidable) and, for the interesting type-3 case, a four-step recipe for
deriving a compound bound instead of a formula: (1) treat inner's state
as the outer's input schema, (2) check the outer's own construction
algorithm can actually run over that state, (3) if so, re-derive -- not
reuse -- the outer's own concentration argument against the composed
randomness, (4) accept the resulting bound is pair-specific, not
universal (DGIM's (1+e)^2*Cf^2/k+Cf-1+e and Hydra's Gi(1+/-e)+eps*GS are
worked examples of the same recipe, not the same formula).

Drops the generic Accuracy::compose()/Sensitivity mechanism from the
previous draft -- it implied a bound could be computed from kind-
independent accuracy values alone, which the DGIM/Hydra worked examples
show is false. CostModel::size_params_composed now takes the child's
concrete (kind, params) directly and has no default body: the deployment
supplies its own pair-specific derivation, or the composition is refused
(accepts_composed_state defaults false).

FromSummary(Rc<L4Node>) is now constrained to state-producing nodes only
(SummaryAgg/SummaryMerge), reusing the same rule this doc's SummaryMerge
table already enforces, rather than allowing reference to a readout.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): restore readout-based composition as a distinct, legitimate mechanism

Correction: the previous commit banned composing over a readout entirely,
requiring FromSummary to reference only state-producing nodes. That
over-corrected -- readout-based composition (mechanism 3a) is a real,
general mechanism in its own right: any inner reporting a public (eps,
delta) can be composed via a Lipschitz-style Sensitivity argument
(triangle inequality + union bound), without needing access to the
inner's raw state at all. It's strictly looser than state-based
composition (3b, the DGIM/Hydra recipe) when 3b is available, but it's
the only option when the inner's state genuinely isn't accessible (e.g.
across a service boundary) or when 3b's construction-compatibility check
fails.

ColumnRef now has two variants -- FromReadout (value-producing node) and
FromState (state-producing node) -- routing to two separate CostModel
gates: accepts_readout_composition/sensitivity_for/compose_over_readout
for 3a, accepts_state_composition/size_params_from_state for 3b. A
deployment facing a novel (outer, inner) pair has a real choice between
the two, not one closed gate.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): drop the "Which issue this solves" mapping section

Requested in review. The design (four compound types, mechanisms 3a/3b)
stands on its own; the issue-by-issue mapping was redundant restatement.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): tighten prose, cut redundant explanation

No content change -- same taxonomy, same two mechanisms, same recipe,
same interfaces. Trims repeated justification and meta-commentary about
prior review rounds, shortens sentences, removes duplicate restatement
of the DGIM/Hydra examples across steps.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): make the 3b recipe steps readable -- one idea per step, examples in a table

The four recipe steps each interleaved DGIM and Hydra inline, forcing
readers to track two examples at once inside every sentence. Splits the
steps (now plain, one idea each) from the worked examples (DGIM/EH,
Hydra, and a KLL-over-HLL counter-example), laid out as a comparison
table keyed to steps 2 and 3.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(l4): rename DGIM/EH to Exponential Histogram [Datar et al., SICOMP'02]

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol

zzylol commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

I will merge it for now. The doc cleaning and interface definitions are done. We can refine them with future PRs.

@zzylol
zzylol merged commit 66d98d4 into main Aug 5, 2026
1 check passed
@zzylol
zzylol deleted the docs/interface-level-l1-l5 branch August 5, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interface-level docs per layer

2 participants