From 3c4c6a0eca50b3128f1baf63eb300a60edae28cf Mon Sep 17 00:00:00 2001 From: AmirHossein Roozbahani Date: Mon, 20 Jul 2026 19:59:24 +0200 Subject: [PATCH 1/4] ci: review PRs with Claude Code + superpowers plugin Runs anthropics/claude-code-action on pull_request, installing the superpowers plugin via the action's plugin_marketplaces/plugins inputs. Requires a CLAUDE_CODE_OAUTH_TOKEN repo secret. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/claude-review.yml | 37 +++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 .github/workflows/claude-review.yml diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml new file mode 100644 index 0000000..ada3d22 --- /dev/null +++ b/.github/workflows/claude-review.yml @@ -0,0 +1,37 @@ +name: Claude PR review + +on: + pull_request: + types: [opened, synchronize, reopened] + +# One review per PR — a new push cancels the in-flight run instead of stacking. +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: write + +jobs: + review: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 # review skill diffs against the merge-base + + - uses: anthropics/claude-code-action@v1 + with: + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + plugin_marketplaces: https://github.com/anthropics/claude-plugins-official.git + plugins: superpowers@claude-plugins-official + claude_args: --model claude-sonnet-5 --max-turns 20 + prompt: | + Review this pull request. + + Use the superpowers:requesting-code-review skill and follow it exactly. + Diff the PR branch against ${{ github.event.pull_request.base.ref }}. + + Report only what you verified in the diff. No speculative findings, no + style nits. If nothing needs changing, say so in one line. From 5f6f2cf031622047725e1bd6ab709eb3e7471cf3 Mon Sep 17 00:00:00 2001 From: AmirHossein Roozbahani Date: Tue, 21 Jul 2026 19:59:33 +0200 Subject: [PATCH 2/4] backlog: prioritize m-1, rule decision-7 (native default, codegen opt-in) - m-1 priorities set: High spine (TASK-5/6/13), Medium (TASK-2/3/15/16), Low (TASK-14/17) - decision-7 accepted + renamed: native is the default engine, codegen is opt-in; TASK-29/34 declassified from blocked-on-open to decided-Low - Closed: TASK-27/30 (CASE both engines), TASK-28 (re-scoped to match-DataFusion folding + retitled), TASK-31, TASK-32, TASK-7 (suite now 0 xfailed) - New: TASK-31 (CASE-branch transformer test), TASK-32 (pydantic-v3 probe), TASK-33 (stale .pyd rebuild guard), TASK-34 (codegen transformer support) Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-framing-native-default-codegen-opt-in.md} | 28 +++++---- ...ture-output-dense-float64-nk-numpy-mode.md | 3 +- ...CSR-materializer-width-invariant-assert.md | 3 +- ...output-scalar-one-hot-as-join-to-domain.md | 3 +- ...ed-assembler-dense-sparse-in-one-SELECT.md | 3 +- ...df-array-multi-hot-opaque-needs-explode.md | 3 +- ...aque-transform-Part-1-review-follow-ups.md | 3 +- ...-27 - CASE-WHEN-support-in-authored-SQL.md | 34 +++++++++-- ...QL-gen-layer-library-wide-CamelCase-fix.md | 29 +++++++--- ...L-surface-containers-UNNEST-unary-minus.md | 32 +++++++++-- ...t-2-authoring-surface-review-follow-ups.md | 3 +- ...degen-CASE-WHEN-support-in-authored-SQL.md | 25 ++++++-- ...ide-a-CASE-branch-native-codegen-parity.md | 53 +++++++++++++++++ ...-not-the-instance-Pydantic-v3-readiness.md | 57 +++++++++++++++++++ ....pyd-when-src-.rs-is-stale-before-tests.md | 29 ++++++++++ ...6\222-Expr-Transform-native-only-today.md" | 52 +++++++++++++++++ .../task-4 - Codegen-only-parity-bugs.md | 2 +- ...tive-swap-native-per-transformer-Tier-0.md | 3 +- ...-6 - ColumnTransformer-assembly-surface.md | 3 +- ...lay-1e-51e-4-decimal-band-vs-DataFusion.md | 32 +++++++++-- 20 files changed, 353 insertions(+), 47 deletions(-) rename backlog/decisions/{decision-7 - two-engine-framing-codegen-vs-native-default-open.md => decision-7 - two-engine-framing-native-default-codegen-opt-in.md} (52%) create mode 100644 backlog/tasks/task-31 - Test-transformer-ref-nested-inside-a-CASE-branch-native-codegen-parity.md create mode 100644 backlog/tasks/task-32 - native-probe-model_fields-on-the-class-not-the-instance-Pydantic-v3-readiness.md create mode 100644 backlog/tasks/task-33 - chore-rebuild-native-_interpreter.pyd-when-src-.rs-is-stale-before-tests.md create mode 100644 "backlog/tasks/task-34 - codegen-transformer-support-resolve_transformers-\342\206\222-Expr-Transform-native-only-today.md" diff --git a/backlog/decisions/decision-7 - two-engine-framing-codegen-vs-native-default-open.md b/backlog/decisions/decision-7 - two-engine-framing-native-default-codegen-opt-in.md similarity index 52% rename from backlog/decisions/decision-7 - two-engine-framing-codegen-vs-native-default-open.md rename to backlog/decisions/decision-7 - two-engine-framing-native-default-codegen-opt-in.md index e21da88..95bcc40 100644 --- a/backlog/decisions/decision-7 - two-engine-framing-codegen-vs-native-default-open.md +++ b/backlog/decisions/decision-7 - two-engine-framing-native-default-codegen-opt-in.md @@ -1,8 +1,8 @@ --- id: decision-7 -title: 'Two-engine framing: codegen vs native as default — OPEN' +title: 'Two-engine framing: native default, codegen opt-in (near-term)' date: '2026-07-19' -status: proposed +status: accepted --- ## Context @@ -17,15 +17,19 @@ n=1 boundary-bound benchmark) and an engine actually exists. ## Decision -**OPEN — not yet ratified.** Whether codegen is adopted as a maintained / default path -vs. the native interpreter is AmirHossein's pending framing call. This record captures -the artifact + the open question, not a decision. +**RULED (2026-07-19, AmirHossein): the native `InferFn` is the default / maintained +serving engine; the codegen engine is OPT-IN for now.** Near-term call — revisitable if a +benchmark or serving need later pushes codegen toward default — not a permanent close. -This framing is load-bearing downstream: -- Blocks TASK-29 (codegen deferred SQL surface) and the framing context on TASK-4. -- The multi-language inference runtimes epic ([[doc-4]]) rests on it — that design treats - the native Rust engine as **one-of-N**, which only holds if two-engine is the accepted - shape. +Downstream consequences of "opt-in": +- TASK-29 (codegen deferred SQL surface) and TASK-34 (codegen transformer support) stay + **Low / not actively prioritized**. Their "precondition: framing decided" is now + *satisfied* (decided → opt-in → low), no longer an open blocker: codegen completeness is + a fast-follow only for someone who opts in, not milestone work. +- The multi-language inference runtimes design ([[doc-4]]) still holds — two engines exist + and native-as-one-of-N is intact; that epic stays parked on its own merits. +- Native remains the parity baseline (oracle = DataFusion, [[decision-1]]); an opted-in + codegen path is still held to that oracle. ## Consequences / notes @@ -37,4 +41,6 @@ This framing is load-bearing downstream: - If oracle parity on the codegen path is wanted, codegen also needs the shared native edge-case fixes (the historical `SUBSTR`/`NaN`/`CAST` set); it already fixed `ROUND(int)` and `COALESCE(int,float)` typing. -- Until this is ratified, codegen stays an artifact on its branch, not a default. +- Ratified **opt-in**: codegen is a maintained-but-opt-in engine, not the default; native + is the default serving path. Revisit the default question only with a serving need or + benchmark that argues for promoting codegen. diff --git a/backlog/tasks/task-13 - Feature-output-dense-float64-nk-numpy-mode.md b/backlog/tasks/task-13 - Feature-output-dense-float64-nk-numpy-mode.md index 10c78fa..3e40731 100644 --- a/backlog/tasks/task-13 - Feature-output-dense-float64-nk-numpy-mode.md +++ b/backlog/tasks/task-13 - Feature-output-dense-float64-nk-numpy-mode.md @@ -4,11 +4,12 @@ title: 'Feature output: dense float64 (n,k) numpy mode' status: To Do assignee: [] created_date: '2026-07-18 15:52' -updated_date: '2026-07-19 01:15' +updated_date: '2026-07-19 15:50' labels: - feature-output milestone: m-1 dependencies: [] +priority: high ordinal: 13000 --- diff --git a/backlog/tasks/task-14 - Feature-output-sparse-COO-struct-column-CSR-materializer-width-invariant-assert.md b/backlog/tasks/task-14 - Feature-output-sparse-COO-struct-column-CSR-materializer-width-invariant-assert.md index a420342..0249a33 100644 --- a/backlog/tasks/task-14 - Feature-output-sparse-COO-struct-column-CSR-materializer-width-invariant-assert.md +++ b/backlog/tasks/task-14 - Feature-output-sparse-COO-struct-column-CSR-materializer-width-invariant-assert.md @@ -6,11 +6,12 @@ title: >- status: To Do assignee: [] created_date: '2026-07-18 15:52' -updated_date: '2026-07-18 23:35' +updated_date: '2026-07-19 15:50' labels: - feature-output milestone: m-1 dependencies: [] +priority: low ordinal: 14000 --- diff --git a/backlog/tasks/task-15 - Feature-output-scalar-one-hot-as-join-to-domain.md b/backlog/tasks/task-15 - Feature-output-scalar-one-hot-as-join-to-domain.md index 462262b..3faa01e 100644 --- a/backlog/tasks/task-15 - Feature-output-scalar-one-hot-as-join-to-domain.md +++ b/backlog/tasks/task-15 - Feature-output-scalar-one-hot-as-join-to-domain.md @@ -4,11 +4,12 @@ title: 'Feature output: scalar one-hot as join-to-domain' status: To Do assignee: [] created_date: '2026-07-18 15:52' -updated_date: '2026-07-18 23:36' +updated_date: '2026-07-19 15:50' labels: - feature-output milestone: m-1 dependencies: [] +priority: medium ordinal: 15000 --- diff --git a/backlog/tasks/task-16 - Feature-output-type-directed-assembler-dense-sparse-in-one-SELECT.md b/backlog/tasks/task-16 - Feature-output-type-directed-assembler-dense-sparse-in-one-SELECT.md index 16aa5cc..f6aa869 100644 --- a/backlog/tasks/task-16 - Feature-output-type-directed-assembler-dense-sparse-in-one-SELECT.md +++ b/backlog/tasks/task-16 - Feature-output-type-directed-assembler-dense-sparse-in-one-SELECT.md @@ -4,11 +4,12 @@ title: 'Feature output: type-directed assembler (dense + sparse in one SELECT)' status: To Do assignee: [] created_date: '2026-07-18 15:52' -updated_date: '2026-07-18 23:36' +updated_date: '2026-07-19 15:50' labels: - feature-output milestone: m-1 dependencies: [] +priority: medium ordinal: 16000 --- diff --git a/backlog/tasks/task-17 - Feature-output-tfidf-array-multi-hot-opaque-needs-explode.md b/backlog/tasks/task-17 - Feature-output-tfidf-array-multi-hot-opaque-needs-explode.md index 293f64a..9453e92 100644 --- a/backlog/tasks/task-17 - Feature-output-tfidf-array-multi-hot-opaque-needs-explode.md +++ b/backlog/tasks/task-17 - Feature-output-tfidf-array-multi-hot-opaque-needs-explode.md @@ -4,11 +4,12 @@ title: 'Feature output: tfidf / array multi-hot (opaque, needs explode)' status: To Do assignee: [] created_date: '2026-07-18 15:52' -updated_date: '2026-07-18 23:36' +updated_date: '2026-07-19 15:50' labels: - feature-output milestone: m-1 dependencies: [] +priority: low ordinal: 17000 --- diff --git a/backlog/tasks/task-2 - Opaque-transform-Part-1-review-follow-ups.md b/backlog/tasks/task-2 - Opaque-transform-Part-1-review-follow-ups.md index b504842..780a36b 100644 --- a/backlog/tasks/task-2 - Opaque-transform-Part-1-review-follow-ups.md +++ b/backlog/tasks/task-2 - Opaque-transform-Part-1-review-follow-ups.md @@ -4,13 +4,14 @@ title: Opaque transform Part-1 review follow-ups status: To Do assignee: [] created_date: '2026-07-18 13:44' -updated_date: '2026-07-19 01:15' +updated_date: '2026-07-19 15:50' labels: - rust - parity - opaque milestone: m-1 dependencies: [] +priority: medium ordinal: 2000 --- diff --git a/backlog/tasks/task-27 - CASE-WHEN-support-in-authored-SQL.md b/backlog/tasks/task-27 - CASE-WHEN-support-in-authored-SQL.md index 292fde2..34a32de 100644 --- a/backlog/tasks/task-27 - CASE-WHEN-support-in-authored-SQL.md +++ b/backlog/tasks/task-27 - CASE-WHEN-support-in-authored-SQL.md @@ -1,11 +1,11 @@ --- id: TASK-27 title: 'native: CASE WHEN support in authored SQL' -status: To Do +status: Done assignee: - Ritchie created_date: '2026-07-18 19:45' -updated_date: '2026-07-19 00:17' +updated_date: '2026-07-19 15:34' labels: - feature - sql-surface @@ -25,8 +25,30 @@ Native-engine (Rust) side of CASE WHEN. native's convert_expr (src/expr_build.rs ## Acceptance Criteria -- [ ] #1 native convert_expr (src/expr_build.rs) gains an SqlExpr::Case branch and _interpreter evaluates CASE WHEN (searched + simple forms, with ELSE + NULL handling) via a new Layer-1 expr node (build + eval + type inference) -- [ ] #2 transform == infer differential parity across a CASE decision-table, incl. an ordinal ladder (Ex/Gd/TA -> 5/4/3) -- [ ] #3 remove the codegen_only skip flag added in TASK-30 so CASE differential cases run on BOTH engines -- [ ] #4 outer joins remain out of scope (separate item) +- [x] #1 native convert_expr (src/expr_build.rs) gains an SqlExpr::Case branch and _interpreter evaluates CASE WHEN (searched + simple forms, with ELSE + NULL handling) via a new Layer-1 expr node (build + eval + type inference) +- [x] #2 transform == infer differential parity across a CASE decision-table, incl. an ordinal ladder (Ex/Gd/TA -> 5/4/3) +- [x] #3 remove the codegen_only skip flag added in TASK-30 so CASE differential cases run on BOTH engines +- [x] #4 outer joins remain out of scope (separate item) + +## Comments + + +author: Iris (PM) +created: 2026-07-19 14:04 +--- +Ritchie picked this up (TASK-30 dependency now Done). Scope confirmed: SqlExpr::Case in src/expr_build.rs + eval in src/expr.rs mirroring codegen semantics (short-circuit, three-valued truthy, common-supertype result typing); then remove the TASK-30 codegen_only skip so tests/test_diff_case.py runs on both backends, plus the window-agg-inside-CASE integration test through SQLTransform.infer_batch (reachable once native supports CASE). +--- + +author: Iris (PM) +created: 2026-07-19 15:34 +--- +Marked Done per Ritchie's merge e2dfeb1. CASE WHEN now complete on both engines (TASK-30 codegen + TASK-27 native). Rebased cleanly onto TASK-28, so the expr_build.rs collision risk I flagged did not materialize. Review's Minor follow-up (transformer-ref-inside-CASE-branch has no test, silent failure mode) captured as TASK-31. +--- + + +## Final Summary + + +Native CASE WHEN shipped and merged to master (e2dfeb1). Added Expr::Case (short-circuit eval; simple form → operand = value), common-supertype result typing mirroring the fixed codegen logic, validation + transformer-resolution recursion. Retired the temporary codegen_only harness flag from TASK-30 so every CASE case runs against the DataFusion oracle on BOTH backends, and added the window-agg-inside-CASE integration test TASK-30 deferred. Rebased cleanly onto master's TASK-28 (identifier folding), rebuilt native. Full suite 484 passed / 16 skipped / 1 xfailed. Final whole-branch review: merge-Yes, no Critical/Important. With TASK-30 (codegen) + TASK-27 (native), CASE WHEN is complete on both engines. Follow-up: TASK-31 (untested resolve_transformers CASE arm) spun off from the review's Minor note. + diff --git a/backlog/tasks/task-28 - Quote-real-column-identifiers-consistently-across-the-SQL-gen-layer-library-wide-CamelCase-fix.md b/backlog/tasks/task-28 - Quote-real-column-identifiers-consistently-across-the-SQL-gen-layer-library-wide-CamelCase-fix.md index 5ca8073..fe347fd 100644 --- a/backlog/tasks/task-28 - Quote-real-column-identifiers-consistently-across-the-SQL-gen-layer-library-wide-CamelCase-fix.md +++ b/backlog/tasks/task-28 - Quote-real-column-identifiers-consistently-across-the-SQL-gen-layer-library-wide-CamelCase-fix.md @@ -1,13 +1,13 @@ --- id: TASK-28 title: >- - Quote real-column identifiers consistently across the SQL-gen layer - (library-wide CamelCase fix) -status: To Do + Fold unquoted identifiers to lowercase like DataFusion (library-wide CamelCase + handling) +status: Done assignee: - Wren created_date: '2026-07-18 19:48' -updated_date: '2026-07-19 00:25' +updated_date: '2026-07-19 14:27' labels: - bug - sql-surface @@ -26,10 +26,11 @@ SEVERITY: high. Wren found (while investigating _compose.py:214) that CamelCase ## Acceptance Criteria -- [ ] #1 authored non-lowercase columns work WITHOUT user double-quoting across: plain passthrough, window-agg OVER, PARTITION BY, composition (frozen + unfit); transformer-ref already fixed (regression-guard) -- [ ] #2 state-key columns stay unquoted (214 invariant preserved + documented with a comment) -- [ ] #3 CamelCase test matrix green; transform == infer parity (decision-1) -- [ ] #4 DESIGN (unresolved): auto-quote/case-sensitive-exact vs match-DataFusion-literally -- AmirHossein + Wren to settle DIRECTLY before AC #1's direction is locked (not PM-ratified) +- [x] #1 Native + codegen FOLD unquoted identifiers to lowercase like the DataFusion oracle (columns, compound col/field parts, SELECT aliases); quoted "Age" stays case-exact everywhere. Root cause was the engines DIDN'T fold — they accepted SQL the oracle rejects. Users quote CamelCase columns themselves (intentional, documented papercut). +- [x] #2 TASK-25 force-quote reverted: transformer refs carry the user's original quoting — {t}(LotArea) folds-and-fails, {t}("LotArea") works. Old test_camelcase_columns_compose (asserted unquoted 'just works') flipped to quoted-works / unquoted-fails. +- [x] #3 state-key columns stay unquoted (_compose.py:214 invariant preserved + documented with a comment). +- [x] #4 transform == infer parity (decision-1) + CamelCase differential matrix green (tests/test_diff_identifier_folding.py, 14/14). +- [x] #5 Ceiling (ponytail note, expr_build.rs): a real CamelCase table/struct-column qualifier is NOT folded — unreachable today (tables are always __THIS__/generated); flagged for when qualified tables become reachable. ## Comments @@ -40,4 +41,16 @@ created: 2026-07-19 00:25 --- Handed to Wren (2026-07-19). The embedded design fork (auto-quote 'just works' vs match-DataFusion-literally) is NOT PM-ratified -- AmirHossein wants to decide it with Wren directly. Wren: ping AmirHossein to settle the direction before implementing AC #1. Rest of the ticket stands as scoped. --- + +author: Iris (PM) +created: 2026-07-19 14:27 +--- +Design fork (AC #4) RESOLVED directly by AmirHossein + Wren → match-DataFusion, not auto-quote. This inverts the earlier Wren+PM recommendation, so I re-scoped AC #1 (fold-like-oracle, users quote CamelCase) and retitled the ticket (was 'Quote real-column identifiers consistently' — now describes folding, since we do the opposite of force-quoting). Marked Done per Wren's merge 7f21dbe. Branch/worktree left up for review before Wren cleans up. +--- + +## Final Summary + + +Resolved as MATCH-DATAFUSION (not auto-quote). Direction settled directly between AmirHossein + Wren — the reverse of the original Wren+PM 'case-sensitive-exact so it just works' recommendation. The real bug was that native + codegen did NOT fold unquoted identifiers, so they accepted SQL the DataFusion oracle rejects; the fix makes both engines fold unquoted identifiers to lowercase like the oracle while quoted identifiers stay case-exact. Users quote CamelCase columns themselves — intentional papercut. Also reverted the TASK-25 force-quote so transformer refs carry the user's original quoting. Merged to master (7f21dbe). Verified: folding matrix 14/14, full Python suite 473 passed, cargo test clean. Branch task-28-identifier-folding + worktree under .claude/worktrees/ still up for diff review before cleanup. + diff --git a/backlog/tasks/task-29 - codegen-implement-deferred-SQL-surface-containers-UNNEST-unary-minus.md b/backlog/tasks/task-29 - codegen-implement-deferred-SQL-surface-containers-UNNEST-unary-minus.md index 2ab7409..d9b81cf 100644 --- a/backlog/tasks/task-29 - codegen-implement-deferred-SQL-surface-containers-UNNEST-unary-minus.md +++ b/backlog/tasks/task-29 - codegen-implement-deferred-SQL-surface-containers-UNNEST-unary-minus.md @@ -1,14 +1,16 @@ --- id: TASK-29 title: 'codegen: implement deferred SQL surface (containers/UNNEST, unary minus, ||)' -status: To Do -assignee: [] +status: In Progress +assignee: + - Ritchie created_date: '2026-07-18 20:14' +updated_date: '2026-07-19 16:18' labels: - codegen - feature - - blocked dependencies: [] +priority: low ordinal: 29000 --- @@ -22,5 +24,27 @@ The codegen engine defers SQL surface it doesn't implement yet -- shows as 16 sk - [ ] #1 each deferred case passes on the codegen backend (the 16-skip set shrinks) - [ ] #2 tests/test_codegen_coverage.py updated as items land (it currently pins the exact skip set) -- [ ] #3 PRECONDITION: codegen-engine framing decision made (default vs opt-in) before this is actively prioritized +- [x] #3 PRECONDITION: codegen-engine framing decision made (default vs opt-in) before this is actively prioritized + +## Comments + + +author: Iris (PM) +created: 2026-07-19 15:43 +--- +QA (Fermi) confirmed the exact 16-skip inventory on current master (2026-07-19): unnest() x5, struct field access x3, struct/list-typed column x3, struct/list construction x1, struct/list comparison x1, named_struct() x1, unary-minus-on-non-literal x1, || operator x1. Split insight to unblock the framing gate: the TWO operator-level defers (unary minus on a non-literal, ||) are cheap scalar ops that are NOT truly framing-gated — same category as CASE (TASK-30), which shipped on codegen regardless of the default-vs-opt-in question. The other 14 (struct/list/UNNEST container surface) are the real framing-gated core. Not spinning a separate ticket yet (no demand for the 2 ops today; native covers them), but documented here so the easy wins can be pulled ahead of the framing decision if codegen-default is chosen or demand appears. +--- + +author: Iris (PM) +created: 2026-07-19 16:13 +--- +decision-7 RULED (2026-07-19): native is default, codegen is opt-in for now. So AC #3's precondition ('framing decision made') is satisfied — decided → opt-in → this stays LOW, a fast-follow only for someone who opts into codegen, not milestone work. Reclassified from 'blocked on an open question' to 'deprioritized by decision' (dropped the 'blocked' label). Revisit only if codegen gets promoted toward default. +--- + +author: Iris (PM) +created: 2026-07-19 16:18 +--- +Dispatched to Ritchie (2026-07-19). AmirHossein's call: Ritchie takes the codegen lane (TASK-29 then TASK-34), Wren takes the rest (m-1 spine). Precondition AC #3 checked — decision-7 is ruled (opt-in), so this is cleared to work. Goal is codegen feature-completeness for opt-in users; still Low importance vs the native default path, but actively worked now. Suggested order within the ticket: the 2 cheap operator defers (unary-minus-on-non-literal, ||) first, then the container surface (struct/list/named_struct/UNNEST). +--- + diff --git a/backlog/tasks/task-3 - Transformer-refs-Part-2-authoring-surface-review-follow-ups.md b/backlog/tasks/task-3 - Transformer-refs-Part-2-authoring-surface-review-follow-ups.md index 38147fd..fecf522 100644 --- a/backlog/tasks/task-3 - Transformer-refs-Part-2-authoring-surface-review-follow-ups.md +++ b/backlog/tasks/task-3 - Transformer-refs-Part-2-authoring-surface-review-follow-ups.md @@ -4,12 +4,13 @@ title: Transformer-refs (Part-2 authoring surface) review follow-ups status: To Do assignee: [] created_date: '2026-07-18 13:44' -updated_date: '2026-07-19 01:15' +updated_date: '2026-07-19 15:50' labels: - python - transformer-refs milestone: m-1 dependencies: [] +priority: medium ordinal: 3000 --- diff --git a/backlog/tasks/task-30 - codegen-CASE-WHEN-support-in-authored-SQL.md b/backlog/tasks/task-30 - codegen-CASE-WHEN-support-in-authored-SQL.md index 58087e5..5df1915 100644 --- a/backlog/tasks/task-30 - codegen-CASE-WHEN-support-in-authored-SQL.md +++ b/backlog/tasks/task-30 - codegen-CASE-WHEN-support-in-authored-SQL.md @@ -1,10 +1,11 @@ --- id: TASK-30 title: 'codegen: CASE WHEN support in authored SQL' -status: In Progress +status: Done assignee: - Ritchie created_date: '2026-07-19 00:17' +updated_date: '2026-07-19 14:04' labels: - feature - sql-surface @@ -25,7 +26,23 @@ Codegen-engine side of CASE WHEN. Teach the codegen expression pipeline about ex ## Acceptance Criteria -- [ ] #1 codegen expr pipeline handles exp.Case (searched + simple forms, with ELSE + NULL handling): IR node + _convert_expr + infer_type + validation + emission as a short-circuiting nested conditional -- [ ] #2 differential parity vs the DataFusion oracle across a CASE decision-table, incl. an ordinal ladder (Ex/Gd/TA -> 5/4/3) -- [ ] #3 add a codegen_only skip flag so CASE differential cases run codegen-only until the native ticket (TASK-27) lands, after which they run on both backends +- [x] #1 codegen expr pipeline handles exp.Case (searched + simple forms, with ELSE + NULL handling): IR node + _convert_expr + infer_type + validation + emission as a short-circuiting nested conditional +- [x] #2 differential parity vs the DataFusion oracle across a CASE decision-table, incl. an ordinal ladder (Ex/Gd/TA -> 5/4/3) +- [x] #3 add a codegen_only skip flag so CASE differential cases run codegen-only until the native ticket (TASK-27) lands, after which they run on both backends + +## Comments + + +author: Iris (PM) +created: 2026-07-19 14:04 +--- +Marked Done per Ritchie's session report (merge a92554f). Native counterpart TASK-27 now In Progress; it will remove the codegen_only skip so CASE runs on both engines. +--- + + +## Final Summary + + +Codegen CASE WHEN shipped and merged to master (a92554f). Searched + simple forms, DataFusion-parity, short-circuiting emission. Suite: 458 passed / 26 skipped / 1 xfailed; whole-branch review verdict merge-Yes. AC #3's codegen_only skip is in place so CASE cases run codegen-only until TASK-27 (native) removes it. + diff --git a/backlog/tasks/task-31 - Test-transformer-ref-nested-inside-a-CASE-branch-native-codegen-parity.md b/backlog/tasks/task-31 - Test-transformer-ref-nested-inside-a-CASE-branch-native-codegen-parity.md new file mode 100644 index 0000000..1bfe5c6 --- /dev/null +++ b/backlog/tasks/task-31 - Test-transformer-ref-nested-inside-a-CASE-branch-native-codegen-parity.md @@ -0,0 +1,53 @@ +--- +id: TASK-31 +title: Test transformer ref nested inside a CASE branch (native + codegen parity) +status: Done +assignee: + - Ritchie +created_date: '2026-07-19 15:35' +updated_date: '2026-07-19 15:53' +labels: + - test + - sql-surface +milestone: m-1 +dependencies: [] +references: + - src/expr_build.rs +priority: medium +type: task +ordinal: 31000 +--- + +## Description + + +Follow-up flagged by the TASK-27 final review (Minor, non-blocking). The resolve_transformers CASE arm — a transformer call nested inside a CASE branch (e.g. `CASE WHEN ... THEN {t}(col) ... END`) — is correct by inspection but has NO test, and its failure mode is silent: if the arm stopped recursing, the call would just never be rewritten to Expr::Transform (no error, wrong result). Its own small unit of work because it needs the transformer-callout fixtures rather than the plain CASE decision-table. Depends on nothing beyond the shipped CASE + transformer-ref machinery (TASK-27, TASK-30). + + +## Acceptance Criteria + +- [x] #1 A transformer ref inside a CASE THEN (and at least one non-first branch) is exercised and asserts transform == infer_batch parity (DataFusion oracle, decision-1) +- [x] #2 Covers both engines (native + codegen), since resolve_transformers recursion is the shared concern + + +## Comments + + +author: Iris (PM) +created: 2026-07-19 15:41 +--- +Dispatched to Ritchie now (bumped Low→Medium). AmirHossein escalated: testing is important, no test = not done — the silent-failure arm can't ship untested. Bar: the test must genuinely FAIL if resolve_transformers stopped recursing, i.e. place the transformer ref in a NON-first CASE branch (not just THEN of the first WHEN), asserting transform == infer_batch. Worktree-isolated, verified green before merge. +--- + +author: Iris (PM) +created: 2026-07-19 15:53 +--- +Done per Ritchie's merge cb8c350. Mutation-verified (broke the recursion, saw the test go red on non-first branches, restored) — meets the 'must catch the regression' bar exactly. Answering Ritchie's codegen question: your call is correct — transformers are native-only, so a native parity test + a codegen defers-loudly (UnsupportedInCodegen) assertion is the right shape of 'both engines'. Nothing to change; don't fabricate codegen computation for a native-only feature. AC #2 accepted under that corrected premise. +--- + + +## Final Summary + + +Merged to master (cb8c350). tests/test_transformer_case.py: a transformer ref (__tfm_0__) in a NON-first WHEN arm AND the ELSE of a CASE, asserting transform == infer_batch vs the DataFusion oracle, with rows hitting each branch (g=1 plain arm, g=2 transformer arm, g=3 transformer ELSE). Regression-catch VERIFIED by mutation: temporarily broke resolve_transformers to recurse only the first arm → test failed with 'Unknown function: __tfm_0__' on the g=2/g=3 rows → restored + reverified green, so it genuinely fails if the arm stops recursing into every branch. Full suite 486 passed / 16 skipped / 1 xfailed. Scope correction on 'both engines': transformers are a NATIVE-ONLY feature (codegen has no transformer support; SQLTransform runs on the native InferFn), so resolve_transformers is native-only — the AC's 'shared concern' premise was off. Correct resolution: native gets the real parity test; codegen is covered by asserting the same construct DEFERS loudly (UnsupportedInCodegen) rather than silently mishandling it. PM-accepted as the right call — don't pretend codegen computes a native-only feature. + diff --git a/backlog/tasks/task-32 - native-probe-model_fields-on-the-class-not-the-instance-Pydantic-v3-readiness.md b/backlog/tasks/task-32 - native-probe-model_fields-on-the-class-not-the-instance-Pydantic-v3-readiness.md new file mode 100644 index 0000000..cd06306 --- /dev/null +++ b/backlog/tasks/task-32 - native-probe-model_fields-on-the-class-not-the-instance-Pydantic-v3-readiness.md @@ -0,0 +1,57 @@ +--- +id: TASK-32 +title: >- + native: probe model_fields on the class, not the instance (Pydantic v3 + readiness) +status: Done +assignee: + - Wren +created_date: '2026-07-19 15:43' +updated_date: '2026-07-19 15:52' +labels: + - rust + - parity + - pydantic +milestone: m-1 +dependencies: [] +references: + - src/expr.rs + - tests/test_diff_types.py +priority: medium +type: bug +ordinal: 32000 +--- + +## Description + + +At infer time the native engine classifies "is this value a struct/model?" via `obj.hasattr("model_fields")` on the INSTANCE (src/expr.rs:187). Pydantic 2.11 deprecated instance access to model_fields (PydanticDeprecatedSince211 — the root cause of all ~14 current suite warnings) and v3.0 REMOVES it: at that point the probe returns false and native misclassifies struct-typed values. So this is a latent CORRECTNESS bug on the v3 bump, not just log noise. Fix is small + local: probe the class instead (`obj.get_type().hasattr("model_fields")`, or an isinstance-vs-BaseModel check). The other model_fields reads (src/schema.rs:30, _codegen/plan.py:54) are already on the class and are fine. Only surfaces in tests/test_diff_types.py (struct-valued cases). Found by QA on current master (484 passed / 16 skipped / 1 xfailed, ~14 warnings all this one root cause). DataFusion is the oracle (decision-1). + + +## Acceptance Criteria + +- [x] #1 native struct/model detection probes the CLASS, not the instance; the ~14 PydanticDeprecatedSince211 warnings clear +- [x] #2 transform == infer parity holds across the tests/test_diff_types.py struct cases after the change (guards the v3 misclassification regression) + + +## Comments + + +author: Iris (PM) +created: 2026-07-19 15:44 +--- +Dispatched to Wren (2026-07-19). Testing note that matters here: under Pydantic 2.x the instance probe STILL WORKS (just deprecated), so a plain transform==infer parity test passes both before AND after the fix — it can't catch the v3 regression. The test with teeth asserts the deprecation is gone / detection is class-level (e.g. no PydanticDeprecatedSince211 emitted for the struct cases). No test = not done (AmirHossein's standing bar). +--- + +author: Iris (PM) +created: 2026-07-19 15:52 +--- +Done per Wren's merge bae07d9. The test meets the bar I set: it asserts the deprecation is gone (RED→GREEN), not just transform==infer parity, so it actually catches the v3-removal regression. Warnings cleared verified under -W error::DeprecationWarning. +--- + + +## Final Summary + + +Merged to master (bae07d9, fix commit abbfe68). src/expr.rs from_pyobject_typed now probes obj.get_type().hasattr("model_fields") (the CLASS) instead of the instance — mirrors schema.rs's already class-level read; one logic line (the larger expr.rs diff is cargo-fmt whitespace normalization in the same file, tests unaffected). schema.rs:30 and _codegen/plan.py:54 left untouched. Test with teeth: tests/test_native_model_fields_probe.py runs a struct value (arrives at infer as a validated nested-model INSTANCE) through native infer and asserts NO PydanticDeprecatedSince211 warning — confirmed RED before ('Accessing the model_fields attribute on the instance is deprecated'), GREEN after; this is what would break on the v3 removal, so it guards the latent misclassification. Verified: full suite 485 passed / 16 skipped / 1 xfailed; clean under -W error::DeprecationWarning (proves the ~14 warnings gone); cargo test 2 passed. + diff --git a/backlog/tasks/task-33 - chore-rebuild-native-_interpreter.pyd-when-src-.rs-is-stale-before-tests.md b/backlog/tasks/task-33 - chore-rebuild-native-_interpreter.pyd-when-src-.rs-is-stale-before-tests.md new file mode 100644 index 0000000..75699d8 --- /dev/null +++ b/backlog/tasks/task-33 - chore-rebuild-native-_interpreter.pyd-when-src-.rs-is-stale-before-tests.md @@ -0,0 +1,29 @@ +--- +id: TASK-33 +title: 'chore: rebuild native _interpreter.pyd when src/*.rs is stale before tests' +status: To Do +assignee: [] +created_date: '2026-07-19 15:43' +labels: + - dev-ex + - native +dependencies: [] +references: + - src/ + - mise.toml +priority: medium +type: chore +ordinal: 33000 +--- + +## Description + + +The native _interpreter.pyd can silently run STALE: if any src/*.rs is newer than the built .pyd (after a checkout, rebase, or history rewrite), the suite runs against OLD native code with no signal. This just bit QA — identifier folding (TASK-28) landed 07-19 in src/ but the .pyd was built 07-18, so native ran without folding and produced 14 PHANTOM identifier-test failures until a manual maturin rebuild. Timely: the ongoing master history rewrite makes stale builds likely across sessions. Fix: a pre-test guard (mise pretest hook / pytest conftest / build dep) that rebuilds via maturin when any src/*.rs mtime > _interpreter.pyd mtime, else no-op. Keep it cheap — an mtime compare, NOT an unconditional rebuild on every run. + + +## Acceptance Criteria + +- [ ] #1 Running the suite auto-rebuilds native when any src/*.rs is newer than the built _interpreter.pyd; an up-to-date build is a no-op (no forced rebuild cost) +- [ ] #2 A deliberately-stale .pyd no longer yields phantom failures — the guard rebuilds first + diff --git "a/backlog/tasks/task-34 - codegen-transformer-support-resolve_transformers-\342\206\222-Expr-Transform-native-only-today.md" "b/backlog/tasks/task-34 - codegen-transformer-support-resolve_transformers-\342\206\222-Expr-Transform-native-only-today.md" new file mode 100644 index 0000000..930b166 --- /dev/null +++ "b/backlog/tasks/task-34 - codegen-transformer-support-resolve_transformers-\342\206\222-Expr-Transform-native-only-today.md" @@ -0,0 +1,52 @@ +--- +id: TASK-34 +title: >- + codegen: transformer support (resolve_transformers → Expr::Transform, + native-only today) +status: To Do +assignee: + - Ritchie +created_date: '2026-07-19 16:08' +updated_date: '2026-07-19 16:18' +labels: + - codegen + - transformer +dependencies: + - TASK-29 +references: + - sql_transform/_codegen/ + - decision-7 + - TASK-29 +priority: low +type: feature +ordinal: 34000 +--- + +## Description + + +Codegen has NO transformer support — transformer refs are a native-only feature. SQLTransform.infer/infer_batch runs on the native InferFn, and the resolve_transformers rewrite (SQL call → Expr::Transform) lives only in the native path; codegen raises UnsupportedInCodegen for it (verified by TASK-31: the CASE-branch transformer test asserts codegen DEFERS loudly rather than computing). This is a whole feature class codegen lacks, distinct from TASK-29's deferred expr/container surface (which explicitly excludes transformers). Placeholder ticket so the gap is tracked and this 'do we have a ticket?' question stops recurring — NOT active work. BLOCKED-ON-FRAMING: only worth building if codegen becomes a maintained/default serving path (decision-7, the two-engine framing question, still open/parked). Bigger lift than TASK-29 — it's the transformer-resolution machinery, not a few expr shapes. Until decision-7 lands, native carries transformers and codegen defers them loudly (correct, not a bug). + + +## Acceptance Criteria + +- [x] #1 PRECONDITION: codegen-engine framing decision made (decision-7, default vs opt-in) before this is actively prioritized — do not build while codegen is the parked/experimental engine +- [ ] #2 codegen resolves transformer refs to its own Expr::Transform equivalent and computes them (not UnsupportedInCodegen), with transform == infer parity vs the DataFusion/native oracle +- [ ] #3 the TASK-31 codegen 'defers loudly' assertion is flipped to a real parity case once codegen computes transformers + + +## Comments + + +author: Iris (PM) +created: 2026-07-19 16:14 +--- +decision-7 RULED (2026-07-19): native default, codegen opt-in. AC #1's precondition is now satisfied (decided → opt-in), so this is decided-LOW rather than blocked-on-open. Stays a tracked placeholder assigned to Ritchie; not active work unless codegen is promoted to default. Dropped the 'blocked' label to match. +--- + +author: Iris (PM) +created: 2026-07-19 16:18 +--- +Queued for Ritchie AFTER TASK-29 (dependency). Precondition AC #1 checked — decision-7 ruled opt-in, so cleared to work once TASK-29 lands. Not dispatched yet; Ritchie is on TASK-29 first. +--- + diff --git a/backlog/tasks/task-4 - Codegen-only-parity-bugs.md b/backlog/tasks/task-4 - Codegen-only-parity-bugs.md index 207fd8f..0d570bf 100644 --- a/backlog/tasks/task-4 - Codegen-only-parity-bugs.md +++ b/backlog/tasks/task-4 - Codegen-only-parity-bugs.md @@ -16,7 +16,7 @@ ordinal: 4000 ## Description -Two divergences on the codegen path only (Rust already matches the oracle here). NOTE: whether codegen is a maintained/default engine is a pending framing call by AmirHossein -- decision-7 (two-engine framing, OPEN); these are recorded regardless. +Two divergences on the codegen path only (Rust already matches the oracle here). NOTE: whether codegen is a maintained/default engine is a pending framing call by AmirHossein -- decision-7 (two-engine framing — ruled 2026-07-19: native default, codegen opt-in); these are recorded regardless. ## Acceptance Criteria diff --git a/backlog/tasks/task-5 - Native-swap-native-per-transformer-Tier-0.md b/backlog/tasks/task-5 - Native-swap-native-per-transformer-Tier-0.md index 41c1c82..e9d5514 100644 --- a/backlog/tasks/task-5 - Native-swap-native-per-transformer-Tier-0.md +++ b/backlog/tasks/task-5 - Native-swap-native-per-transformer-Tier-0.md @@ -4,12 +4,13 @@ title: 'Native-swap: native per-transformer (Tier 0)' status: To Do assignee: [] created_date: '2026-07-18 13:44' -updated_date: '2026-07-19 01:23' +updated_date: '2026-07-19 15:50' labels: - sklearn - native-swap milestone: m-1 dependencies: [] +priority: high ordinal: 5000 --- diff --git a/backlog/tasks/task-6 - ColumnTransformer-assembly-surface.md b/backlog/tasks/task-6 - ColumnTransformer-assembly-surface.md index 4218540..0bbd1fc 100644 --- a/backlog/tasks/task-6 - ColumnTransformer-assembly-surface.md +++ b/backlog/tasks/task-6 - ColumnTransformer-assembly-surface.md @@ -4,12 +4,13 @@ title: ColumnTransformer assembly surface status: To Do assignee: [] created_date: '2026-07-18 13:44' -updated_date: '2026-07-19 01:23' +updated_date: '2026-07-19 15:50' labels: - sklearn - fallback milestone: m-1 dependencies: [] +priority: high ordinal: 6000 --- diff --git a/backlog/tasks/task-7 - Native-float-display-1e-51e-4-decimal-band-vs-DataFusion.md b/backlog/tasks/task-7 - Native-float-display-1e-51e-4-decimal-band-vs-DataFusion.md index de239c2..c862d8d 100644 --- a/backlog/tasks/task-7 - Native-float-display-1e-51e-4-decimal-band-vs-DataFusion.md +++ b/backlog/tasks/task-7 - Native-float-display-1e-51e-4-decimal-band-vs-DataFusion.md @@ -1,15 +1,17 @@ --- id: TASK-7 title: 'Native float display: [1e-5,1e-4) decimal band vs DataFusion' -status: To Do -assignee: [] +status: Done +assignee: + - Wren created_date: '2026-07-18 14:28' -updated_date: '2026-07-19 01:15' +updated_date: '2026-07-19 16:03' labels: - rust - parity milestone: m-1 dependencies: [] +priority: low ordinal: 7000 --- @@ -21,5 +23,27 @@ Residual of TASK-1 #1: the rust-parity fix handled 1.0/1e300/CONCAT/|| but MISSE ## Acceptance Criteria -- [ ] #1 CAST across [1e-5,1e-4) renders decimal matching DF (1e-5->'0.00001'); the pinned xfail flips to pass +- [x] #1 CAST across [1e-5,1e-4) renders decimal matching DF (1e-5->'0.00001'); the pinned xfail flips to pass + +## Comments + + +author: Iris (PM) +created: 2026-07-19 15:54 +--- +Dispatched to Wren (2026-07-19) — fresh src/expr.rs context from TASK-32. Kept Low (narrow band, real-world-negligible); dispatched now because it's an annoying xfail residual AmirHossein wants gone. Self-verifying: the fix flips the pinned strict xfail, and removing the marker IS the test. +--- + +author: Iris (PM) +created: 2026-07-19 16:03 +--- +Done per Wren's merge 42b9f12. Suite now at 0 xfailed — no pinned native parity residuals left in the differential suite. Oracle-probed before coding (validate-don't-assume), targeted one-line fix, boundary parity added. The annoyance is gone. +--- + + +## Final Summary + + +Merged to master (42b9f12, fix commit 96ff89b). src/expr.rs display_value now renders |x| in [1e-5, 1e-4) as fixed-point shortest-round-trip (`{}`) and keeps `{:?}` elsewhere. Wren probed the oracle first: DataFusion uses fixed decimal for base-10 exponent E in [-5, 15] and scientific outside; `{:?}` already matched all of that except the single E=-5 band — this residual. One-line branch; codegen untouched (already matched). Test: removed the xfail_on_native guard on test_float_display_small_decimal_band (now a normal passing differential case, 1e-5 -> '0.00001') + added band and just-outside-boundary parity checks (9.99e-6 stays scientific, 1e-4 stays fixed; parity-only, no hand-computed digit strings). Verified: full suite 488 passed / 16 skipped / 0 xfailed — the strict xfail is gone and nothing else flipped to XPASS. Closes the last of the TASK-1 rust float-parity residual family. + From 722a7d64674428aae4f481544f50f4ebf6242311 Mon Sep 17 00:00:00 2001 From: AmirHossein Roozbahani Date: Tue, 21 Jul 2026 19:59:47 +0200 Subject: [PATCH 3/4] docs: add serving-path benchmark brief (doc-6) Benchmarks the serving-path options -- ONNX vs the native Rust interpreter vs Python codegen vs WASM. Authored in the benchmarking/QA session (Fermi); committed here at AmirHossein's request. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...t-interpreter-vs-Python-codegen-vs-WASM.md | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 backlog/docs/doc-6 - Serving-path-benchmarks-ONNX-vs-Rust-interpreter-vs-Python-codegen-vs-WASM.md diff --git a/backlog/docs/doc-6 - Serving-path-benchmarks-ONNX-vs-Rust-interpreter-vs-Python-codegen-vs-WASM.md b/backlog/docs/doc-6 - Serving-path-benchmarks-ONNX-vs-Rust-interpreter-vs-Python-codegen-vs-WASM.md new file mode 100644 index 0000000..3311a9e --- /dev/null +++ b/backlog/docs/doc-6 - Serving-path-benchmarks-ONNX-vs-Rust-interpreter-vs-Python-codegen-vs-WASM.md @@ -0,0 +1,193 @@ +--- +id: doc-6 +title: 'Serving-path benchmarks: ONNX vs Rust interpreter vs Python codegen vs WASM' +type: guide +created_date: '2026-07-19 00:48' +tags: + - benchmark + - performance + - wasm + - onnx + - codegen + - serving +--- +## Context — why these benchmarks exist + +Two project goals: ergonomic SQL to transformer authoring, and **fast inference**. Open questions this +session set out to answer with hard numbers instead of intuition: + +1. Where does inference cost actually live — compute, or the boundaries (pydantic/FFI)? +2. How do the candidate serving paths compare: the **Rust interpreter** (`InferFn`, pyo3), the + **Python codegen** (`CodegenFn`), **ONNX Runtime**, and a **WASM** kernel? +3. Is WASM a legit *multi-language serving output* — one artifact across Python/Go/Java, no per-language FFI? + +**Status: exploratory spike, not production measurement.** Everything below is a microbenchmark of one +tiny numeric transform on a laptop-class box with background noise. Treat the *order-of-magnitude gaps* +as the finding; do not quote the exact nanoseconds as SLAs. Run-to-run variance is real (see Caveats). + +--- + +## TL;DR — the implications + +- **Inference is boundary-bound, confirmed.** The pydantic in/out boundary dominates both the Rust + interpreter and the Python codegen: **~4,000–4,500 ns/row batched**, vs **single-digit ns/row** for + paths that take raw columnar arrays (ONNX/WASM/numpy). ~1,000x gap, and it is *not* the arithmetic. +- **Python codegen is about equal to the Rust interpreter** for the pydantic contract (codegen slightly + ahead). Both are pinned by the same pydantic boundary, so swapping the interpreter for codegen barely + moves batched throughput. The lever is owning the columnar path, not the engine internals. +- **ONNX and WASM both hit single-digit ns/row batched** when fed raw columns. ONNX wins in Python + (numpy zero-copy); WASM matches or beats it in Go/Java with a **309-byte artifact and no FFI**. +- **n=1 is where they split hard.** No-FFI WASM runtimes: **Go ~48 ns, Java-AOT ~24 ns**. ONNX at n=1: + **14–29 microseconds** — a ~300-800x gap, entirely ONNX's per-Run() session/binding tax. +- **Two traps, measured:** chicory *interpreter* is ~57x slower than its own AOT (always enable the AOT + compiler); and the pydantic boundary (~4,000+ ns/row) does not amortize with batch size. + +--- + +## The shared workload (every benchmark ran this) + +SQL (post-fit, scaler/feature-math shaped — the smallest realistic numeric transform): + +``` +SELECT age / 271.0 AS age_r, amount / 100.0 AS amt_b, + amount - age AS d, amount + age AS s, + age * 2.0 AS a2, amount * 0.5 AS h +FROM t +``` + +2 f64 inputs (amount, age) to 6 f64 outputs. Inputs: 10,000 deterministic rows. **Parity gate:** every +engine's output is `allclose(rtol=1e-9)` to the hand-computed arithmetic *before* any timing is taken. + +**WASM kernel** (`kernel.wat`, hand-written, 6 f64 ops over linear memory; assembled to a **309-byte** +`.wasm` via `wasmtime.wat2wasm`; the SAME .wasm is loaded by all three host runtimes): + +``` +(func (export "infer") (param $amount i32)(param $age i32)(param $n i32)(param $out i32) + loop i in 0..n: + a = f64.load amount+8i ; g = f64.load age+8i + out[0n+i]=g/271 ; out[1n+i]=a/100 ; out[2n+i]=a-g + out[3n+i]=a+g ; out[4n+i]=g*2 ; out[5n+i]=a*0.5 ) +``` + +NOTE: the WAT was **hand-written**, not emitted by codegen. It proves the runtime/serving layer, NOT a +SQL to wasm compiler. + +**ONNX model** (`model.onnx`, 367 bytes, same 6 ops as a graph): inputs amount,age (DOUBLE,[None]); +Div/Div/Sub/Add/Mul/Mul with constant initializers 271,100,2,0.5; 6 DOUBLE outputs. Same file loaded by +Python, Go, and Java ORT. + +--- + +## Settings (common) + +- **Machine:** Intel i5-12400F (6 P-cores / 12 threads), Windows 11 Pro build 26200. Not core-pinned, + not isolated; laptop-class background load. +- **Sizes:** n=1 (online latency) and N=10,000 (batch). `batch/row` = per-call batch time / 10,000. +- **ONNX:** 1 intra-op and 1 inter-op thread (single-threaded, to compare kernels not thread pools). +- **Timing methods (they differ — this matters for n=1):** + - Python `bench()`: warmup 50, then **median of single `perf_counter()` samples** (n=1: 3000 samples; + batch: 300 samples). + - Go/Java `bench()`: warmup 100–200, then **best-of-7 of amortized loops** (total wall / iters; n=1 + iters 200k, batch iters 3k). + - Consequence: Python n=1 includes per-call timer overhead; Go/Java n=1 is amortized best. **n=1 is not + strictly cross-host comparable; batch/row is** (both amortize over 10k rows). +- **Versions:** Python 3.14.0; onnxruntime 1.27.0; wasmtime 46.0.1; numpy 2.5.1; pydantic 2.13.4; + sqlglot 30.12.0. Go 1.26.5; wazero 1.12.0; onnxruntime_go 1.31.0 (cgo, compiled with **zig 0.16.0 as + `zig cc`** — no MSVC/mingw); onnxruntime.dll 1.27.x reused from the Python venv. Java Temurin 21.0.11; + chicory 1.4.0; onnxruntime(java) 1.22.0; maven 3.9.16. JVM/Maven/zig installed user-local via **mise**. + +--- + +## What each benchmark actually did (sketches) + +**Python** (`bench_mech.py`): rows are 10k pydantic `Row(amount, age)`; a,g are 10k float64 numpy arrays. +- rust-interp: `fn.infer({"t": rows})` — `InferFn`, pydantic in, validated pydantic out +- codegen: `cg.infer({"t": rows})` — `CodegenFn`, identical contract +- onnx: `sess.run(cols, {"amount": a, "age": g})` — numpy arrays in/out, 1 thread +- wasm: write a,g into wasmtime linear memory, `infer(...)`, read 6 cols back (includes the copy) +- numpy: `np.stack([g/271, a/100, a-g, a+g, g*2, a*0.5])` — pure-kernel reference + +**Go** (`gobench/`, `goonnx/`): +- wazero: `NewRuntimeConfigCompiler()`; write cols via `mem.WriteFloat64Le`; `infer.Call(...)` (incl copy). + A hand-written pure-Go loop is the native ceiling. +- onnxruntime_go: `ort.SetSharedLibraryPath(venv onnxruntime.dll)`; `DynamicAdvancedSession(model.onnx)`; + `NewTensor` per call; `sess.Run`. cgo, built with `zig cc`. + +**Java** (`javabench/`, `javaonnx/`): +- chicory interpreter: `Instance.builder(module).build()` +- chicory AOT: `Instance.builder(module).withMachineFactory(MachineFactoryCompiler::compile).build()` + (both write cols to `Memory` via little-endian `ByteBuffer`, then `infer.apply(...)`) +- onnxruntime(java): `OrtEnvironment`; `createSession(model.onnx)`; `OnnxTensor.createTensor` per call; + `session.run`. Native libs bundled in the Maven jar (no DLL wrangling). 1 intra/inter-op thread. + +--- + +## Results — batch (10k rows), per-row (cross-host comparable) + +| Host | Rust interp (InferFn) | Python codegen | ONNX | WASM | native ceiling | +|---|---|---|---|---|---| +| Python | 4,525 ns (pydantic) | 3,997 ns (pydantic) | **3.8 ns** | 8.7 ns (wasmtime) | 3.9 ns (numpy) | +| Go | — | — | 10.3 ns | **8.6 ns** (wazero) | 2.3 ns (pure-Go) | +| Java | — | — | 14.7 ns | **8.3 ns** (chicory AOT) / 472.8 (interp) | — | + +## Results — n=1 (see method caveat; not strictly cross-host) + +| Host | Rust interp | codegen | ONNX | WASM | +|---|---|---|---|---| +| Python | 4.4 us | 3.8 us | 14.3 us | 39.7 us* | +| Go | — | — | 20.1 us | ~0.048 us | +| Java | — | — | 29.4 us | ~0.024 us (AOT) | + +\* Python wasm n=1 is dominated by the per-call numpy to linear-memory copy in the harness, not the wasm kernel. + +--- + +## Caveats — what these numbers are NOT + +1. **Asymmetric boundaries by design.** Rust-interp and codegen include the pydantic in/out boundary; + ONNX/WASM/numpy take raw arrays. That asymmetry IS the finding ("native is slow" = the boundary, not + the kernel), but it means these are not same-boundary comparisons. +2. **n=1 methodology differs across hosts** (Python median-of-singles vs Go/Java amortized best-of-7). + Batch/row is comparable; n=1 cross-host is not. +3. **Run-to-run variance ~2.6x on the pydantic-bound paths** (an earlier, more-loaded run measured + native/codegen batch at ~11,700 ns/row vs ~4,000–4,500 here). Raw-array paths were stable (~3–10 ns/row). + Order-of-magnitude conclusions hold; exact ns do not. +4. **Go pure-kernel n=1 was dead-code-eliminated** by the compiler (reads 0); only its batch/row (2.3 ns) + is meaningful. +5. **This is 6 arithmetic ops on non-null f64.** It does NOT exercise strings, NULLs, casts, joins, + lookups, or real fitted transforms — the WASM/columnar fast path only covers the typed numeric subset. + +--- + +## Implications for engine strategy + +- **Native/Rust `InferFn` stays as the parity oracle** (DataFusion is the oracle) regardless of serving + path. It is not the thing to optimize for throughput — its batched cost is the pydantic boundary, + shared with codegen. +- **Python codegen does not beat native meaningfully under the pydantic contract.** The real lever is + owning the columnar path (eliminate the per-row pydantic boundary), consistent with the boundary-bound + thesis. +- **WASM is a credible multi-language serving OUTPUT for the typed numeric subset:** one 309-byte + artifact ran at 8.3–8.7 ns/row across Python/Go/Java with no FFI, and crushed ONNX at n=1 (no + session/binding tax). The open risk is the compiler — a SQL to wasm codegen for the numeric subset, + reusing the existing codegen front-end (IR to WAT string emit + `wat2wasm`). The full DataFusion + surface in wasm (strings/nulls/joins) is a separate, larger bet, not required for the numeric win. +- **ONNX only wins batched-and-Python-hosted** (numpy zero-copy); its per-Run() floor makes it a poor + fit for n=1 online serving. + +--- + +## Reproduction + +All sources live in the session scratchpad (not committed): +- `bench_mech.py` — Python: InferFn / CodegenFn / onnxruntime / wasmtime / numpy +- `kernel.wat`, `kernel.wasm` — hand-written 6-op WASM kernel (wat2wasm) +- `model.onnx` — identical 6-op ONNX graph +- `gobench/` — Go wazero + pure-Go reference +- `goonnx/` — Go onnxruntime_go (cgo via zig cc; loads the venv onnxruntime.dll) +- `javabench/` — Java chicory interpreter + AOT +- `javaonnx/` — Java onnxruntime + +Tooling installed user-local via **mise**: java (Temurin 21.0.11), maven 3.9.16, zig 0.16.0. Python deps +(onnxruntime, wasmtime, onnx, numpy) added to the project `.venv`. The ONNX native lib for Go was the +`onnxruntime.dll` already bundled in the venv — no separate system install. From 80bf6e70b00013b63b42b902b7f9cff7e4248dd0 Mon Sep 17 00:00:00 2001 From: AmirHossein Roozbahani Date: Wed, 22 Jul 2026 02:14:25 +0200 Subject: [PATCH 4/4] ci: grant id-token:write for Claude review OIDC + bump checkout to v5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit claude-code-action needs id-token:write to fetch its OIDC token — without it the run failed 'Could not fetch an OIDC token'. Also bump actions/checkout v4->v5 (Node 20 deprecation). Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/claude-review.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index ada3d22..0e124bd 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -12,12 +12,13 @@ concurrency: permissions: contents: read pull-requests: write + id-token: write # claude-code-action fetches an OIDC token for auth jobs: review: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: fetch-depth: 0 # review skill diffs against the merge-base