From 187b83efd32140ff7b8c0d0c60c0a1423195bc91 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:02:44 +0000 Subject: [PATCH 1/9] chore: sync the agent standards' key-code-interfaces PR section Brings ProjectASAP/agent-standards#9 into this repository: the asap-pr-authoring skill, the pull-request template and AGENTS.md now ask every PR description for a Key code interfaces added / modified section. --- .agents/skills/asap-pr-authoring/SKILL.md | 12 +++++++++--- .github/PULL_REQUEST_TEMPLATE/projectasap.md | 7 +++++++ AGENTS.md | 3 ++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/.agents/skills/asap-pr-authoring/SKILL.md b/.agents/skills/asap-pr-authoring/SKILL.md index 590cc4056..b08978d6a 100644 --- a/.agents/skills/asap-pr-authoring/SKILL.md +++ b/.agents/skills/asap-pr-authoring/SKILL.md @@ -13,11 +13,17 @@ Write the pull-request description top-down: 1. **Why**: the user, design, correctness, or MVP need. 2. **What**: the observable behavior and scope changed. 3. **How**: the core implementation or algorithm, without narrating every file. -4. **Before this PR**: the previous end-to-end behavior, limitation, failure +4. **Key code interfaces added / modified**: every public interface the PR adds, + changes, or removes: types, functions, methods, traits, fields, enum + variants, module paths, CLI flags, and wire, file, or document formats. Give + each as a short signature excerpt with its crate or module path, copied from + the diff, and one line on what changed and why. Write "None" when the PR + changes no interface (for example, documentation or tests only). +5. **Before this PR**: the previous end-to-end behavior, limitation, failure mode, architecture, or workflow. -5. **After this PR**: the new end-to-end behavior and what users, developers, or +6. **After this PR**: the new end-to-end behavior and what users, developers, or operators can do differently. -6. **Verification**: focused tests and applicable end-to-end evidence. +7. **Verification**: focused tests and applicable end-to-end evidence. Include a compact before/after example, screenshot, or diagram when it provides real evidence or makes behavior easier to understand. For significant design diff --git a/.github/PULL_REQUEST_TEMPLATE/projectasap.md b/.github/PULL_REQUEST_TEMPLATE/projectasap.md index ecfa590c6..74d87aa72 100644 --- a/.github/PULL_REQUEST_TEMPLATE/projectasap.md +++ b/.github/PULL_REQUEST_TEMPLATE/projectasap.md @@ -10,6 +10,13 @@ What observable behavior and scope does this PR change? What is the core algorithm, interface, or design decision? Keep this top-down. +## Key code interfaces added / modified + +List every public interface this PR adds, changes, or removes: types, functions, +methods, traits, fields, enum variants, module paths, CLI flags, and wire, file, +or document formats. Give each as a short signature excerpt with its crate or +module path and one line on what changed. Write "None" if no interface changes. + ## Before this PR Describe the previous end-to-end behavior, limitation, failure mode, diff --git a/AGENTS.md b/AGENTS.md index 3bad13863..2e7d26e93 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,7 +23,8 @@ original behavior, then confirm it passes after the fix. - Commit at incremental logical boundaries when commits are requested. - Keep pull-request descriptions top-down and concise. Explain why, what, and - how. Explicitly describe **Before this PR** and **After this PR**, using an + how, and list the **Key code interfaces added / modified** (or "None"). + Explicitly describe **Before this PR** and **After this PR**, using an end-to-end example, screenshot, measurement, or diagram appropriate to the change. - Agents must not complete fields reserved for human judgment, approval, or From aebcce87beabde98bdc8eeda4d6e846932cc4902 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:49:44 +0000 Subject: [PATCH 2/9] chore: sync the key-code-interfaces layout from agent-standards File-grouped declaration blocks with links at a fixed commit and a usage snippet, following PR #537 (ProjectASAP/agent-standards#9). --- .agents/skills/asap-pr-authoring/SKILL.md | 19 ++++++--- .../references/key-code-interfaces-example.md | 39 +++++++++++++++++++ .github/PULL_REQUEST_TEMPLATE/projectasap.md | 9 +++-- 3 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 .agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md diff --git a/.agents/skills/asap-pr-authoring/SKILL.md b/.agents/skills/asap-pr-authoring/SKILL.md index b08978d6a..c2e377f5f 100644 --- a/.agents/skills/asap-pr-authoring/SKILL.md +++ b/.agents/skills/asap-pr-authoring/SKILL.md @@ -13,12 +13,21 @@ Write the pull-request description top-down: 1. **Why**: the user, design, correctness, or MVP need. 2. **What**: the observable behavior and scope changed. 3. **How**: the core implementation or algorithm, without narrating every file. -4. **Key code interfaces added / modified**: every public interface the PR adds, +4. **Key code interfaces added / modified**: the public interfaces the PR adds, changes, or removes: types, functions, methods, traits, fields, enum - variants, module paths, CLI flags, and wire, file, or document formats. Give - each as a short signature excerpt with its crate or module path, copied from - the diff, and one line on what changed and why. Write "None" when the PR - changes no interface (for example, documentation or tests only). + variants, module paths, CLI flags, and wire, file, or document formats. + Write "None" when no interface changes (for example, documentation or tests + only). Use this layout: + - Group by source file. Head each group with links to the files at a fixed + commit, i.e. a permalink to the PR head or the merge commit, not a branch. + - Under each group, put one fenced code block in the source language. It + holds the real declarations copied from the diff: type definitions with + their fields or variants, and function, method and trait signatures with + bodies elided (`;` or `/* ... */`). Add a short trailing comment where the + meaning or the change isn't obvious (`// new`, `// was X`, `// removed`). + - End with a short **Usage** snippet that shows how a caller combines the + main interfaces, when that helps. + See [references/key-code-interfaces-example.md](references/key-code-interfaces-example.md). 5. **Before this PR**: the previous end-to-end behavior, limitation, failure mode, architecture, or workflow. 6. **After this PR**: the new end-to-end behavior and what users, developers, or diff --git a/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md b/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md new file mode 100644 index 000000000..cd2994a2a --- /dev/null +++ b/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md @@ -0,0 +1,39 @@ +# Example: Key code interfaces section + +An excerpt in the expected layout: links per file group, real declarations, and a usage snippet. + +## Key code interfaces + +[`crates/types/src/ir/node.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/node.rs), [`scalar.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/scalar.rs) + +```rust +pub enum Operator> { NonASAP(NonASAPOp), ASAP(ASAPOp) } +pub enum ScalarExpr> { /* ScalarSubquery(C), Exists { subquery: C, .. }, ... */ } +pub enum QueryRoot> { Operator(C), Scalar(ScalarExpr) } + +impl Operator { + pub fn children(&self) -> Vec<&C>; // operator inputs, then scalar-referenced nodes + pub fn map_children(&self, f: impl FnMut(&C) -> D) -> Operator; +} +``` + +[`crates/types/src/ir/flat.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/flat.rs) + +```rust +pub type NodeId = usize; + +pub struct FlatDag { + pub nodes: Vec, // node i is nodes[i]; children before parents + pub roots: Vec>, // one per input root, in order +} + +/// Also returns the original `Rc` for each id. +pub fn flatten(roots: &[QueryRoot]) -> (FlatDag, Vec>); +``` + +Usage: + +```rust +let (flat, originals) = flatten(&[QueryRoot::Operator(Rc::clone(&root))]); +let json = serde_json::to_string(&flat)?; +``` diff --git a/.github/PULL_REQUEST_TEMPLATE/projectasap.md b/.github/PULL_REQUEST_TEMPLATE/projectasap.md index 74d87aa72..d2238c292 100644 --- a/.github/PULL_REQUEST_TEMPLATE/projectasap.md +++ b/.github/PULL_REQUEST_TEMPLATE/projectasap.md @@ -12,10 +12,11 @@ What is the core algorithm, interface, or design decision? Keep this top-down. ## Key code interfaces added / modified -List every public interface this PR adds, changes, or removes: types, functions, -methods, traits, fields, enum variants, module paths, CLI flags, and wire, file, -or document formats. Give each as a short signature excerpt with its crate or -module path and one line on what changed. Write "None" if no interface changes. +Group by source file, with links to the files at a fixed commit. Under each group, +put one fenced code block holding the real declarations: types with their fields +or variants, and function/method/trait signatures with bodies elided. Add short +comments for what changed (`// new`, `// was X`, `// removed`). End with a short +usage snippet when it helps. Write "None" if no interface changes. ## Before this PR From 5339b44dd3de2211b291dd6b8679fd688126b017 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:00:36 +0000 Subject: [PATCH 3/9] chore: sync the per-interface explanations rule from agent-standards --- .agents/skills/asap-pr-authoring/SKILL.md | 10 +++++++-- .../references/key-code-interfaces-example.md | 21 +++++++++++++++++-- .github/PULL_REQUEST_TEMPLATE/projectasap.md | 5 +++-- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/.agents/skills/asap-pr-authoring/SKILL.md b/.agents/skills/asap-pr-authoring/SKILL.md index c2e377f5f..d2b46661d 100644 --- a/.agents/skills/asap-pr-authoring/SKILL.md +++ b/.agents/skills/asap-pr-authoring/SKILL.md @@ -23,8 +23,14 @@ Write the pull-request description top-down: - Under each group, put one fenced code block in the source language. It holds the real declarations copied from the diff: type definitions with their fields or variants, and function, method and trait signatures with - bodies elided (`;` or `/* ... */`). Add a short trailing comment where the - meaning or the change isn't obvious (`// new`, `// was X`, `// removed`). + bodies elided (`;` or `/* ... */`). Mark each change with a trailing + comment (`// new`, `// was X`, `// removed`). + - Explain every interface. Above each declaration, write a short `///` (or + the language's doc-comment form) saying what it is, what it is for, and + when a caller uses it. For an enum variant or an error, say when it is + produced and whether it reflects bad input, an unsatisfiable request, or an + internal bug. For a field, say what it holds and its default. Keep each + explanation to one or two lines. - End with a short **Usage** snippet that shows how a caller combines the main interfaces, when that helps. See [references/key-code-interfaces-example.md](references/key-code-interfaces-example.md). diff --git a/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md b/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md index cd2994a2a..342689e7c 100644 --- a/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md +++ b/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md @@ -7,13 +7,30 @@ An excerpt in the expected layout: links per file group, real declarations, and [`crates/types/src/ir/node.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/node.rs), [`scalar.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/scalar.rs) ```rust +/// One relational or ASAP operator; `C` is how it refers to its inputs +/// (`Rc` nodes in a DAG, `NodeId` in a flat DAG). // new pub enum Operator> { NonASAP(NonASAPOp), ASAP(ASAPOp) } +/// A per-row expression; it may read other operators' results through +/// subquery variants. // changed: generic over C pub enum ScalarExpr> { /* ScalarSubquery(C), Exists { subquery: C, .. }, ... */ } +/// What one query returns: a relation, or a single scalar. // new pub enum QueryRoot> { Operator(C), Scalar(ScalarExpr) } impl Operator { - pub fn children(&self) -> Vec<&C>; // operator inputs, then scalar-referenced nodes - pub fn map_children(&self, f: impl FnMut(&C) -> D) -> Operator; + /// Every node this operator reads: its inputs first, then the nodes its + /// scalar expressions reference. Used for traversal and CSE. + pub fn children(&self) -> Vec<&C>; // new + /// Rebuilds the operator with each child mapped, e.g. `Rc` to `NodeId`. + pub fn map_children(&self, f: impl FnMut(&C) -> D) -> Operator; // new +} + +/// Why plan selection could not finish (distinct from a candidate being +/// rejected, which is recorded in `Selection.rejected`). +pub enum SelectionError { + // ... + /// Rebuilding a candidate with the sketch sizes its δ needs failed. An + /// internal bug: resizing should never break a schema. + Sizing { candidate: String, error: SizingError }, // new } ``` diff --git a/.github/PULL_REQUEST_TEMPLATE/projectasap.md b/.github/PULL_REQUEST_TEMPLATE/projectasap.md index d2238c292..de5994dfb 100644 --- a/.github/PULL_REQUEST_TEMPLATE/projectasap.md +++ b/.github/PULL_REQUEST_TEMPLATE/projectasap.md @@ -14,8 +14,9 @@ What is the core algorithm, interface, or design decision? Keep this top-down. Group by source file, with links to the files at a fixed commit. Under each group, put one fenced code block holding the real declarations: types with their fields -or variants, and function/method/trait signatures with bodies elided. Add short -comments for what changed (`// new`, `// was X`, `// removed`). End with a short +or variants, and function/method/trait signatures with bodies elided. Mark what +changed (`// new`, `// was X`, `// removed`), and explain each interface in a short +doc comment: what it is, what it is for, and when it is used or produced. End with a short usage snippet when it helps. Write "None" if no interface changes. ## Before this PR From 88d1d412105ce0426f628971f902b32d33c89bc4 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:53:50 +0000 Subject: [PATCH 4/9] chore: sync the issue-authoring skill and issue-template Example sections from agent-standards --- .agents/projectasap-managed-files.txt | 1 + .agents/skills/asap-issue-authoring/SKILL.md | 29 +++++++++++++++++++ .../projectasap-correctness-bug.md | 6 ++++ .../projectasap-documentation.md | 6 ++++ .../projectasap-existing-design-feature.md | 6 ++++ .../ISSUE_TEMPLATE/projectasap-exploration.md | 6 ++++ .../projectasap-new-design-feature.md | 6 ++++ .../ISSUE_TEMPLATE/projectasap-performance.md | 6 ++++ .../ISSUE_TEMPLATE/projectasap-refactor.md | 6 ++++ 9 files changed, 72 insertions(+) create mode 100644 .agents/skills/asap-issue-authoring/SKILL.md diff --git a/.agents/projectasap-managed-files.txt b/.agents/projectasap-managed-files.txt index ee1c9d15b..8e27cc589 100644 --- a/.agents/projectasap-managed-files.txt +++ b/.agents/projectasap-managed-files.txt @@ -4,6 +4,7 @@ AGENTS.md .agents/skills/asap-documentation .agents/skills/asap-design-review .agents/skills/asap-pr-authoring +.agents/skills/asap-issue-authoring .agents/skills/asap-start-work .agents/skills/asap-performance-investigation .agents/skills/asap-implement-existing-design diff --git a/.agents/skills/asap-issue-authoring/SKILL.md b/.agents/skills/asap-issue-authoring/SKILL.md new file mode 100644 index 000000000..f26a41fe5 --- /dev/null +++ b/.agents/skills/asap-issue-authoring/SKILL.md @@ -0,0 +1,29 @@ +--- +name: asap-issue-authoring +description: Write ProjectASAP GitHub issues that a reader understands without prior context. Use when asked to open, draft or rewrite an issue, a tracking issue, or a follow-up. +--- + +# Issues + +Write the issue top-down so a reader new to the topic understands the problem +before the solution: + +1. **Problem, by example.** Start with one concrete, minimal example of the + problem: the input (a query, a workload, a command, a data shape), what + happens today, and what should happen instead. Use real syntax and real + numbers; show the current and the desired plan, output, or behavior side by + side when that is clearer. Prefer one small example over an abstract + description, and add a second only when it shows a different case. +2. **Why it happens / why it isn't done today.** The cause in the code or the + design, named precisely (types, functions, rules), and what already works + nearby. +3. **Proposal or goal.** What a fix or feature would change, at the level of + interfaces and behavior, with alternatives when the choice is open. +4. **Scope and acceptance.** What is in and out of scope, and the test that + would show it is done (for a bug, the regression test that fails today). +5. **Status and related work.** Decisions already made (with dates), why it is + deferred if it is, and links to related issues and PRs. + +Never invent behavior or numbers: derive the example from the code, a run, or +a measurement, and say which. Keep people and company names out. Use the +repository's issue templates when one fits, and keep their sections. diff --git a/.github/ISSUE_TEMPLATE/projectasap-correctness-bug.md b/.github/ISSUE_TEMPLATE/projectasap-correctness-bug.md index 0db4264ce..a53a252d2 100644 --- a/.github/ISSUE_TEMPLATE/projectasap-correctness-bug.md +++ b/.github/ISSUE_TEMPLATE/projectasap-correctness-bug.md @@ -5,6 +5,12 @@ title: "[Bug] " labels: bug --- +## Example + +A concrete, minimal example of the problem: the input (query, workload, +command or data shape), what happens today, and what should happen instead. +Use real syntax and numbers. + ## Current behavior ## Expected behavior and invariant diff --git a/.github/ISSUE_TEMPLATE/projectasap-documentation.md b/.github/ISSUE_TEMPLATE/projectasap-documentation.md index 9957b4062..b4176bfa6 100644 --- a/.github/ISSUE_TEMPLATE/projectasap-documentation.md +++ b/.github/ISSUE_TEMPLATE/projectasap-documentation.md @@ -5,6 +5,12 @@ title: "[Docs] " labels: documentation --- +## Example + +A concrete, minimal example of the problem: the input (query, workload, +command or data shape), what happens today, and what should happen instead. +Use real syntax and numbers. + ## Audience Choose one: user, developer, designer/architect, or researcher. diff --git a/.github/ISSUE_TEMPLATE/projectasap-existing-design-feature.md b/.github/ISSUE_TEMPLATE/projectasap-existing-design-feature.md index 477b3376b..5f031786f 100644 --- a/.github/ISSUE_TEMPLATE/projectasap-existing-design-feature.md +++ b/.github/ISSUE_TEMPLATE/projectasap-existing-design-feature.md @@ -5,6 +5,12 @@ title: "[Feature] " labels: enhancement --- +## Example + +A concrete, minimal example of the problem: the input (query, workload, +command or data shape), what happens today, and what should happen instead. +Use real syntax and numbers. + ## User need and MVP ## Authoritative design diff --git a/.github/ISSUE_TEMPLATE/projectasap-exploration.md b/.github/ISSUE_TEMPLATE/projectasap-exploration.md index e43becabc..5d202747c 100644 --- a/.github/ISSUE_TEMPLATE/projectasap-exploration.md +++ b/.github/ISSUE_TEMPLATE/projectasap-exploration.md @@ -5,6 +5,12 @@ title: "[Exploration] " labels: exploration --- +## Example + +A concrete, minimal example of the problem: the input (query, workload, +command or data shape), what happens today, and what should happen instead. +Use real syntax and numbers. + ## Question and hypothesis ## Success signal diff --git a/.github/ISSUE_TEMPLATE/projectasap-new-design-feature.md b/.github/ISSUE_TEMPLATE/projectasap-new-design-feature.md index 640e47def..9ce8b3890 100644 --- a/.github/ISSUE_TEMPLATE/projectasap-new-design-feature.md +++ b/.github/ISSUE_TEMPLATE/projectasap-new-design-feature.md @@ -5,6 +5,12 @@ title: "[Design] " labels: enhancement --- +## Example + +A concrete, minimal example of the problem: the input (query, workload, +command or data shape), what happens today, and what should happen instead. +Use real syntax and numbers. + ## Problem and why now ## Intended users and MVP diff --git a/.github/ISSUE_TEMPLATE/projectasap-performance.md b/.github/ISSUE_TEMPLATE/projectasap-performance.md index fe2779b57..87875dc61 100644 --- a/.github/ISSUE_TEMPLATE/projectasap-performance.md +++ b/.github/ISSUE_TEMPLATE/projectasap-performance.md @@ -5,6 +5,12 @@ title: "[Performance] " labels: performance --- +## Example + +A concrete, minimal example of the problem: the input (query, workload, +command or data shape), what happens today, and what should happen instead. +Use real syntax and numbers. + ## User or operational impact ## Metric and target diff --git a/.github/ISSUE_TEMPLATE/projectasap-refactor.md b/.github/ISSUE_TEMPLATE/projectasap-refactor.md index 7fcdb115d..492aa59cc 100644 --- a/.github/ISSUE_TEMPLATE/projectasap-refactor.md +++ b/.github/ISSUE_TEMPLATE/projectasap-refactor.md @@ -5,6 +5,12 @@ title: "[Refactor] " labels: refactor --- +## Example + +A concrete, minimal example of the problem: the input (query, workload, +command or data shape), what happens today, and what should happen instead. +Use real syntax and numbers. + ## Concrete maintenance problem ## Behavior that must remain unchanged From 74d5ac5a867fbb3669bae7ebca46e40dcc07c77b Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 20:06:02 +0000 Subject: [PATCH 5/9] chore: sync end-to-end usage and fully listed members rules from agent-standards --- .agents/skills/asap-pr-authoring/SKILL.md | 20 +- .../references/key-code-interfaces-example.md | 244 +++++++++++++++--- .github/PULL_REQUEST_TEMPLATE/projectasap.md | 14 +- 3 files changed, 231 insertions(+), 47 deletions(-) diff --git a/.agents/skills/asap-pr-authoring/SKILL.md b/.agents/skills/asap-pr-authoring/SKILL.md index d2b46661d..cc3419522 100644 --- a/.agents/skills/asap-pr-authoring/SKILL.md +++ b/.agents/skills/asap-pr-authoring/SKILL.md @@ -25,14 +25,30 @@ Write the pull-request description top-down: their fields or variants, and function, method and trait signatures with bodies elided (`;` or `/* ... */`). Mark each change with a trailing comment (`// new`, `// was X`, `// removed`). + - Never collapse members into a comment such as `// ...` or + `/* A, B, C unchanged */`; readers take a collapsed list for a removed + one. When a type is shown, list each of its variants or fields with a + one-line `///` and a marker (`// unchanged`, `// new`, `// was X`, + `// removed`), and put a `///` on the type itself saying whether the type + stays or is removed. - Explain every interface. Above each declaration, write a short `///` (or the language's doc-comment form) saying what it is, what it is for, and when a caller uses it. For an enum variant or an error, say when it is produced and whether it reflects bad input, an unsatisfiable request, or an internal bug. For a field, say what it holds and its default. Keep each explanation to one or two lines. - - End with a short **Usage** snippet that shows how a caller combines the - main interfaces, when that helps. + - End with a **Usage** example that is end to end, not isolated calls. + Start from what the user writes (the workload entry or query with its + requirements and the data facts it needs, or the CLI or devtool + invocation). Walk through where the system reaches this PR's interfaces: + which stage or component calls them, and with what values. End at what the + user gets back (for a planner change: the selected plan, its sizes and + cost, and each candidate's accepted or rejected line). Every variable gets + a comment where it is defined saying what it holds and what it is used for + later in the flow; no bare `let x = f(...)` lines, and never a bare + internal call such as `size_params(...)` without the flow that reaches it. + Use real syntax and numbers, and check them by compiling and running the + example (a test, an example binary or a devtool run) at the linked commit. See [references/key-code-interfaces-example.md](references/key-code-interfaces-example.md). 5. **Before this PR**: the previous end-to-end behavior, limitation, failure mode, architecture, or workflow. diff --git a/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md b/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md index 342689e7c..fdb14c7a8 100644 --- a/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md +++ b/.agents/skills/asap-pr-authoring/references/key-code-interfaces-example.md @@ -1,56 +1,220 @@ # Example: Key code interfaces section -An excerpt in the expected layout: links per file group, real declarations, and a usage snippet. +An excerpt from ASAPPlanner #715 (HLL sized from (ε, δ)) in the expected layout: +links per file group at a fixed commit, real declarations with every member +listed and marked, a `///` on every interface, and an end-to-end usage example +whose every variable is commented and whose numbers come from running it. -## Key code interfaces +## Key code interfaces added / modified -[`crates/types/src/ir/node.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/node.rs), [`scalar.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/scalar.rs) +[`crates/types/src/post_asap/sketch.rs`](https://github.com/ProjectASAP/ASAPPlanner/blob/1b5d9577705c7664e711eef9a83ba4f2def0c1da/crates/types/src/post_asap/sketch.rs) ```rust -/// One relational or ASAP operator; `C` is how it refers to its inputs -/// (`Rc` nodes in a DAG, `NodeId` in a flat DAG). // new -pub enum Operator> { NonASAP(NonASAPOp), ASAP(ASAPOp) } -/// A per-row expression; it may read other operators' results through -/// subquery variants. // changed: generic over C -pub enum ScalarExpr> { /* ScalarSubquery(C), Exists { subquery: C, .. }, ... */ } -/// What one query returns: a relation, or a single scalar. // new -pub enum QueryRoot> { Operator(C), Scalar(ScalarExpr) } - -impl Operator { - /// Every node this operator reads: its inputs first, then the nodes its - /// scalar expressions reference. Used for traversal and CSE. - pub fn children(&self) -> Vec<&C>; // new - /// Rebuilds the operator with each child mapped, e.g. `Rc` to `NodeId`. - pub fn map_children(&self, f: impl FnMut(&C) -> D) -> Operator; // new -} - -/// Why plan selection could not finish (distinct from a candidate being -/// rejected, which is recorded in `Selection.rejected`). -pub enum SelectionError { - // ... - /// Rebuilding a candidate with the sketch sizes its δ needs failed. An - /// internal bug: resizing should never break a schema. - Sizing { candidate: String, error: SizingError }, // new +/// Concrete parameters of one sketch instance; equality compares the built shape. +/// The type stays; this PR adds a field to `Hll` and `Kmv`. +pub enum SketchParams { + /// A UnivMon universal sketch (frequency moments, entropy, distinct count). + UnivMon { // unchanged + /// Top-k heap entries per layer. + heap_size: u32, // unchanged + /// Rows of each layer's Count Sketch. + sketch_rows: u32, // unchanged + /// Columns of each layer's Count Sketch. + sketch_cols: u32, // unchanged + /// Number of sampling layers. + layers: u8, // unchanged + /// The δ the heap was sized for when it answers a distinct count; None = no distinct guarantee. + distinct_failure_probability: Option, // unchanged + }, + /// A KLL quantile sketch. + Kll { // unchanged + /// Compactor capacity. + k: u32, // unchanged + /// The δ `k` was sized for when below 0.01; None = the empirical 99% calibration. + failure_probability: Option, // unchanged + }, + /// A Count-Min sketch. + Cms { // unchanged + /// Counters per row. + width: u32, // unchanged + /// Rows. + depth: u32, // unchanged + }, + /// A HyperLogLog distinct-count sketch. + Hll { // unchanged variant; gains relative_error + /// `m = 2^precision` registers. + precision: u8, // unchanged + /// The relative error `precision` was sized for: its guarantee's error bound, + /// at which its failure probability is evaluated. + #[serde(default, skip_serializing_if = "Option::is_none")] + relative_error: Option, // new: the ε precision was sized for; None = RSE 1.04/√m; ignored by equality + }, + /// A DDSketch relative-error quantile sketch. + DDSketch { // unchanged + /// Relative accuracy of each bucket. + alpha: f64, // unchanged + }, + /// A Count-Min sketch with a top-k heap. + CmsWithHeap { // unchanged + /// Counters per row. + width: u32, // unchanged + /// Rows. + depth: u32, // unchanged + /// Heap entries. + heap_size: u32, // unchanged + }, + /// A KMV (k minimum values) distinct-count sketch. + Kmv { // unchanged variant; gains relative_error + /// The number of smallest hashes kept. + k: u32, // unchanged + /// The relative error `k` was sized for: its guarantee's error bound, at which + /// its failure probability is evaluated. + #[serde(default, skip_serializing_if = "Option::is_none")] + relative_error: Option, // new: the ε k was sized for; None = RSE 1/√(k-2); ignored by equality + }, + /// A Theta distinct-count sketch. + Theta { // unchanged + /// Nominal entries. + k: u32, // unchanged + }, + /// A Count Sketch. + CountSketch { // unchanged + /// Counters per row. + width: u32, // unchanged + /// Rows. + depth: u32, // unchanged + }, + /// A Count Sketch with a top-k heap. + CountSketchWithHeap { // unchanged + /// Counters per row. + width: u32, // unchanged + /// Rows. + depth: u32, // unchanged + /// Heap entries. + heap_size: u32, // unchanged + }, } ``` -[`crates/types/src/ir/flat.rs`](https://github.com/ORG/REPO/blob//crates/types/src/ir/flat.rs) +[`crates/asap-aware-mapping/src/accuracy/estimators/hll.rs`](https://github.com/ProjectASAP/ASAPPlanner/blob/1b5d9577705c7664e711eef9a83ba4f2def0c1da/crates/asap-aware-mapping/src/accuracy/estimators/hll.rs) ```rust -pub type NodeId = usize; - -pub struct FlatDag { - pub nodes: Vec, // node i is nodes[i]; children before parents - pub roots: Vec>, // one per input root, in order -} - -/// Also returns the original `Rc` for each id. -pub fn flatten(roots: &[QueryRoot]) -> (FlatDag, Vec>); +/// Pass 1's classic HLL precision for relative error `eps` at failure probability +/// `delta`, without data; plan selection later enlarges it for the source's bound. +pub(crate) fn hll_precision(eps: f64, delta: f64) -> u8; // was hll_precision(eps) +/// Classic HLL's distinct-count guarantee over `source`'s own rows, with δ left +/// symbolic in the source's distinct bound; `None` for a precision outside the contract. +pub(crate) fn classic_guarantee( + precision: u8, + relative_error: Option, // new parameter + source: &str, +) -> Option; +/// The failure probability given the source's distinct bound; 1 (no certificate) +/// outside the contract's range. Plan selection calls it to bind the symbol. +pub(crate) fn classic_failure_probability( + precision: u8, + relative_error: f64, // new parameter + max_distinct: u64, +) -> f64; ``` -Usage: +Usage, end to end: one SQL distinct count with ε = 0.05 and δ = 0.01, planned +through `run_planning_stages` at the merge commit. It starts from what the user +writes, names the stage that calls each interface above with its values, and +ends at what the user gets back. ```rust -let (flat, originals) = flatten(&[QueryRoot::Operator(Rc::clone(&root))]); -let json = serde_json::to_string(&flat)?; +// 1. What the user writes: one dashboard panel, the number of distinct source IPs +// in the last minute, refreshed every 10 s, within ε = 0.05 relative error +// with failure probability δ = 0.01. +let panel = RepeatingEntry { + query: Query("SELECT COUNT(DISTINCT src_ip) FROM flows WHERE ts >= now() - INTERVAL '1 minute'".into()), + demand: RepeatedDemand::FixedInterval(RepetitionInterval(10_000)), + requirements: QueryRequirements { + // The target every candidate plan's guarantee is checked against in Stage 3. + accuracy: AccuracyRequirement::Explicit(RequestedAccuracy { + scope: AccuracyScope::WholeResult, + ..RequestedAccuracy::epsilon_delta(0.05, 0.01) + }), + response_latency: LatencyRequirement::Unspecified, + }, + predictability: Predictability::Predictable { known_at: None }, + time_selection: TimeSelection { + scope: QueryTimeScope::RealTime, + lookback: Some(DurationMs(60_000)), + as_of: None, + }, +}; +// 2. What the deployment declares about the data: at most 1,000 distinct items in +// one evaluation of any distinct count over `flows` (the SQL table name is the +// source key). Stage 1 never reads it; Stage 3 uses it to turn HLL's symbolic δ +// into a number. +let data = DataWorkload { + arrival: DataArrival::ContinuouslyIngesting, + ingestion_rate: declared(Rate(1_000_000.0 / 15.0)), + max_distinct_per_evaluation: declared(BTreeMap::from([("flows".into(), 1_000)])), + ..Default::default() +}; +// The planner input: the query workload plus the data facts. +let workload = PlanningWorkload { + query_workload: QueryWorkload { + language: QueryLanguage::SQL(SqlDialect::DataFusionSQL), + query_batch: None, + repeating_queries: Some(vec![panel]), + }, + data_workload: Some(data), +}; +// The `flows(ts, src_ip)` schema the SQL frontend resolves the query against. +let catalog = asap_frontend_sql::SqlCatalog::new().with_table( + "flows", + Schema::with_time_index( + vec![ + Field::plain("ts", DataType::Timestamp, false), + Field::plain("src_ip", DataType::Utf8, false), + ], + 0, + vec![], + ), +); + +// 3. Stage 0 (query-language frontends): `roots` holds one plan root per workload entry, +// here a distinct count (`aggregate:cardinality`) over a scan of `flows`; Stage 1 reads it. +let roots = lower_sql_roots(&workload, &catalog)?; + +// 4. Stages 1-3 in one call. `run` holds every stage's candidates and the selection, +// which steps 5 and 6 read. +let run = run_planning_stages(&workload, roots, None).expect("Pass 1 enumerates"); +// Stage 1 / Pass 1 offers L1 exact, L2 HLL, L3 KMV and L4 UnivMon for the count. +// For L2 it calls `size_params(Hll, &intent, 0.05, 0.01)`: the smallest m with +// m ≥ 1/(2ε²δ) whose tail certifies δ at ε for every bound up to ⌈1/ε⌉ = 20, +// i.e. `Hll { precision: 15, relative_error: Some(0.05) }`. +// It then calls `hll::classic_guarantee(15, Some(0.05), "flows")`: error bound +// 0.05 (relative), and δ left as the symbol +// `classic_hll_failure_probability[p=15,eps=0.05](max_distinct[flows])`, +// because the distinct count it depends on is a data fact Stage 1 does not read. +// L3 needs no bound: `Kmv { k: 4323 }` certifies δ = 0.009996 for any count. +// +// 5. Stage 3 (plan selection) binds the symbol: it parses (15, 0.05, "flows"), reads +// the bound 1,000 from `max_distinct_per_evaluation`, and calls +// `hll::classic_failure_probability(15, 0.05, 1_000)` = 0.0058 ≤ 0.01, so P2 (HLL) +// meets the target without enlargement (`precision_for` keeps 15; precision 15 +// certifies bounds up to 1,176, precision 18 up to 9,636). +// `selection`: Stage 3's result (selected id, costs, rejections); Err only when no candidate is valid. +let selection = run.stage3_plan_selection?; + +// 6. What the user gets back. +// `selected`: the cheapest valid plan. The analytical cost model prices the +// exact plan P1 at 1.7600001 cost/s and HLL (P2) and KMV (P3) at 1.7600003 (the +// same scan and build, plus an estimate node), so exact is selected; HLL is valid +// but costlier. +assert_eq!(selection.selected, "P1"); +// `costs`: one entry per valid candidate (P1-P4; UnivMon P4 costs 2.96). +assert_eq!(selection.costs.len(), 4); +// `rejected`: P2's line is `valid: true, reason: "costlier: 1.760 vs 1.760 +// cost_per_second"`. With a bound of 10,000 instead, P2 is `valid: false`, +// "failure probability Some(1.0), which misses target ..." (beyond 9,636 even at +// precision 18); with no bound, "failure probability None". +// `hll`: the HLL candidate's line in the selection report, read to show why it was not chosen. +let hll = selection.rejected.iter().find(|r| r.id == "P2").unwrap(); +assert!(hll.valid && hll.reason.starts_with("costlier")); ``` diff --git a/.github/PULL_REQUEST_TEMPLATE/projectasap.md b/.github/PULL_REQUEST_TEMPLATE/projectasap.md index de5994dfb..9018a589e 100644 --- a/.github/PULL_REQUEST_TEMPLATE/projectasap.md +++ b/.github/PULL_REQUEST_TEMPLATE/projectasap.md @@ -13,11 +13,15 @@ What is the core algorithm, interface, or design decision? Keep this top-down. ## Key code interfaces added / modified Group by source file, with links to the files at a fixed commit. Under each group, -put one fenced code block holding the real declarations: types with their fields -or variants, and function/method/trait signatures with bodies elided. Mark what -changed (`// new`, `// was X`, `// removed`), and explain each interface in a short -doc comment: what it is, what it is for, and when it is used or produced. End with a short -usage snippet when it helps. Write "None" if no interface changes. +put one fenced code block holding the real declarations: types with every field +or variant listed (never collapsed into `// ...`), and function/method/trait +signatures with bodies elided. Mark every member (`// unchanged`, `// new`, +`// was X`, `// removed`), say on each type whether it stays or is removed, and +explain each interface in a short doc comment: what it is, what it is for, and when +it is used or produced. End with an end-to-end usage example: from what the user +writes (query, requirements, data facts, or CLI call), through where this PR's +interfaces are called, to what the user gets back, with every variable commented +and the numbers checked by running it. Write "None" if no interface changes. ## Before this PR From 710db339e974a09d252fded7006181f797179fbb Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 20:12:08 +0000 Subject: [PATCH 6/9] chore: sync the code-review interface simplification step from agent-standards --- .agents/skills/asap-code-review/SKILL.md | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/.agents/skills/asap-code-review/SKILL.md b/.agents/skills/asap-code-review/SKILL.md index bec33a265..6df0d9d9a 100644 --- a/.agents/skills/asap-code-review/SKILL.md +++ b/.agents/skills/asap-code-review/SKILL.md @@ -18,6 +18,36 @@ Check whether new conceptual layers, types, or interfaces solve a demonstrated need. Flag duplicated definitions and avoid recommending architecture whose cost exceeds the stated requirement. +Required step: for the interfaces in the PR, self-ask "any ways to simplify +or make them more human-readable?", and give the author choices. For every +type, field, function, method, CLI flag or format the PR adds or changes, +check whether it could be simpler or easier for a human to read: + +- Names are understandable without their comments, and builder or method names + match the field names they set or read. +- Near-duplicate types or functions are merged (for example, two structs that + carry the same fields). +- Reserved or unused fields are removed. +- A struct holding indexes into another list holds the items themselves. +- Counts are integers, not `f64`. +- Display strings live outside planner structs. +- Planner-internal helpers are not methods on public types. +- One function replaces a general function plus a special case of it. + +Report these in a section **Interface simplifications (author's choice)**. Give +each option the current signature, the proposed signature, and the trade-off, +for example: + +- `equal_shares(target, metric, of, shares)` and `equal_share(target, metric, n)` + → one `per_input_target(target, metric, approximate_inputs)`. Trade-off: only + the legacy core uses `shares > 1`; merging needs that core gone first. +- `whole_result_guarantee(guarantee, output_values: Option)` → + `guarantee.over_whole_result(values_per_result: Option)`. Trade-off: a + method on a public type, but a count that cannot be fractional. + +The author picks; the reviewer does not decide. These options do not replace +correctness findings and are listed after them. + Check that tests exercise the changed behavior, including relevant unit and end-to-end paths. For a bug fix, verify that a regression test would fail without the production change. From de39dc72dcce0d4c408563211ecae6e1baec9ec2 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 20:12:35 +0000 Subject: [PATCH 7/9] chore: sync the code-review efficiency step from agent-standards --- .agents/skills/asap-code-review/SKILL.md | 30 ++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/.agents/skills/asap-code-review/SKILL.md b/.agents/skills/asap-code-review/SKILL.md index 6df0d9d9a..e6736faba 100644 --- a/.agents/skills/asap-code-review/SKILL.md +++ b/.agents/skills/asap-code-review/SKILL.md @@ -45,8 +45,30 @@ for example: `guarantee.over_whole_result(values_per_result: Option)`. Trade-off: a method on a public type, but a count that cannot be fractional. -The author picks; the reviewer does not decide. These options do not replace -correctness findings and are listed after them. +The author picks; the reviewer does not decide. + +Required step: review the implementation's performance complexity +(efficiency) as well. For the new or changed code paths, check: + +- Time and space complexity: quadratic or exponential loops, subset + enumeration, recomputation inside loops (for example a reachability walk + per node), unnecessary clones of DAGs or roots, and eager enumeration that + could be lazy or capped. +- Allocations on hot paths. +- How the cost scales with the number of queries, candidates and series. + +Each efficiency finding states the complexity, a realistic input size at which +it hurts, and a concrete cheaper alternative. When the cheaper alternative has +a trade-off, give the author choices here too. + +Review output format, in this order: + +1. **Findings**: correctness, regressions and other defects, by severity. +2. **Interface simplifications (author's choice)**: the options above. +3. **Efficiency**: complexity, the input size where it hurts, the + alternative, and choices where there is a trade-off. + +Write "None" under a section with nothing to report. Check that tests exercise the changed behavior, including relevant unit and end-to-end paths. For a bug fix, verify that a regression test would fail without @@ -56,6 +78,6 @@ For materially affected quality attributes—maintainability, debuggability, scalability, understandability, extensibility, performance, and operability—ask for measurable evidence where practical. Do not invent numerical precision. -Report findings first, ordered by severity, with precise file and line -references. State concrete impact and a viable correction. Keep summaries brief; +Report findings first (section 1 above), ordered by severity, with precise +file and line references. State concrete impact and a viable correction. Keep summaries brief; if no actionable findings remain, say so and identify any material testing gap. From c0bc9393a50794a65e6ecbbb5de55a29c5d01cb2 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 20:12:48 +0000 Subject: [PATCH 8/9] chore: sync the code-review examples from agent-standards --- .agents/skills/asap-code-review/SKILL.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.agents/skills/asap-code-review/SKILL.md b/.agents/skills/asap-code-review/SKILL.md index e6736faba..5cd1cf9d1 100644 --- a/.agents/skills/asap-code-review/SKILL.md +++ b/.agents/skills/asap-code-review/SKILL.md @@ -44,6 +44,14 @@ for example: - `whole_result_guarantee(guarantee, output_values: Option)` → `guarantee.over_whole_result(values_per_result: Option)`. Trade-off: a method on a public type, but a count that cannot be fractional. +- `Pass2Candidate { .. }` and `Pass2Of { .. }`, which carry nearly the same fields + as ready-formatted strings, plus `SharedSummary { group: usize, members: + Vec }`, which indexes into two other lists → one + `Stage1Candidate.pass2: Option` holding the Pass 1 candidate + and the shared build nodes themselves (`shared_builds: + Vec>>`), with display text moved to the devtools. + Trade-off: one more rename across the Pass 2 rules, but readers no longer + follow indexes. The author picks; the reviewer does not decide. @@ -61,6 +69,18 @@ Each efficiency finding states the complexity, a realistic input size at which it hurts, and a concrete cheaper alternative. When the cheaper alternative has a trade-off, give the author choices here too. +For example: + +- `price` recomputes `query_reach(dag)`, a walk over the whole DAG, for every + MatAtQuery node: O(n · (n + e)) per candidate, times every candidate. At + Example 3's 972 candidates with about 50 nodes each, that is about 2.5·10⁶ + node visits instead of 5·10⁴. Alternative: compute it once per candidate + before the loop and pass it in. +- Subset enumeration over one sharing group's members is 2^n. It is capped by + `MAX_GROUP_MEMBERS = 20` (about 10⁶ subsets). Choices: keep the cap and + report a group that hits it, or enumerate lazily and stop at + `--max-combinations`. + Review output format, in this order: 1. **Findings**: correctness, regressions and other defects, by severity. From a1e28c69f1e1d408c9e2a1c367712a78afbb2696 Mon Sep 17 00:00:00 2001 From: zzylol <50204836+zzylol@users.noreply.github.com> Date: Thu, 15 Oct 2026 20:12:56 +0000 Subject: [PATCH 9/9] chore: sync the code-review example wording from agent-standards --- .agents/skills/asap-code-review/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/asap-code-review/SKILL.md b/.agents/skills/asap-code-review/SKILL.md index 5cd1cf9d1..5d6091fcc 100644 --- a/.agents/skills/asap-code-review/SKILL.md +++ b/.agents/skills/asap-code-review/SKILL.md @@ -73,7 +73,7 @@ For example: - `price` recomputes `query_reach(dag)`, a walk over the whole DAG, for every MatAtQuery node: O(n · (n + e)) per candidate, times every candidate. At - Example 3's 972 candidates with about 50 nodes each, that is about 2.5·10⁶ + Example 3's 972 candidates, if each DAG has about 50 nodes, that is about 2.5·10⁶ node visits instead of 5·10⁴. Alternative: compute it once per candidate before the loop and pass it in. - Subset enumeration over one sharing group's members is 2^n. It is capped by