refactor(ir): split L2 relational + converter into asap-l2, slim asap-ir to L3 - #65
Merged
Conversation
…-ir to L3 Per @milind's #57 feedback ("a lot fits into asap-ir"). asap-ir was 3.9k LOC holding L2 relational + L3 canonical IR + the L2→L3 converter + binder + resolution. Split along the real (non-cyclic) seam: - asap-ir (2.3k, was 3.9k): the L3 canonical IR only — query_expr, agg_intent, expr_ir, schema, names (+ types, workload). - asap-l2 (1.6k, new): the L2 relational algebra + the L2→L3 converter — relational, lower (convert_root), binder, column_resolution. Depends on asap-ir. The apparent L2↔L3 module cycles were doc-comment links, not code deps; the real graph layers cleanly. Isolation wins: - asap-sketch and asap-plan now depend on asap-ir alone — they never pull the converter/binder machinery (cargo tree: 0 asap-l2 in either). - Front ends depend on asap-ir (L3 types) + asap-l2 (relational + convert_root). - Parser quarantine unchanged (promql frontend: 0 datafusion). Docs (README, design.md §5.1/§6.0/P1, migration-plan) updated: dependency table, directory tree, and the "why L2 and L3 are separate crates" rationale. No behavior change; full workspace suite green; clippy --all-targets clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses @milindsrivastava1997's #57 feedback ("a lot fits into
asap-ir").asap-irwas 3.9k LOC holding L2 relational + L3 IR + the L2→L3 converter + binder + resolution.Split (along the real, non-cyclic seam)
asap-ir— now the L3 canonical IR only:query_expr,agg_intent,expr_ir,schema,names(+types,workload). 3,872 → 2,288 LOC (−41%).asap-l2(new) — the L2 relational algebra + the L2→L3 converter:relational,lower(convert_root),binder,column_resolution. ~1,600 LOC. Depends onasap-ir.The apparent L2↔L3 module cycles turned out to be doc-comment links, not code deps — the real
use-graph layers cleanly.Isolation wins
asap-sketchandasap-plannow depend onasap-iralone — they never pull the converter/binder machinery.cargo tree: 0asap-l2in either.asap-ir(L3 types) +asap-l2(relational +convert_root).Docs
README,
design.md(§5.1 table + tree, §6.0 map, P1), andmigration-plan.mdupdated with the dependency table, the new directory tree, and a "why L2 and L3 are separate crates" rationale.No behavior change; full suite green (29 binaries), clippy clean. Follow-up to the reorg (#57).
🤖 Generated with Claude Code