Make the data model use Item and List types universally, with nodes authored as rank-polymorphic kernels#4335
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a major refactoring of the node graph's type system, moving from a list-based rank system to a more robust Item<T>/List<T> rank-0/rank-1 structural type system. It includes significant changes to node definitions, input adapters, and migration logic to support this new structure, alongside performance optimizations for raster image processing in nodes like dehaze, sample_image, and combine_channels by avoiding unnecessary data cloning.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 120 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Not reviewed (too large): node-graph/nodes/vector/src/vector_nodes.rs (~2,115 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
!build (Run ID 29391783747) |
Wasm: 34.55 MB — JS: 0.44 MB — CSS: 0.09 MB — Fonts: 0.30 MB — Images: 0.09 MB — All Assets: 35.48 MB |
Performance Benchmark Results
|
|
The expected 6.16 MB build size increase from doubling the monomorphizations should be temporary once we can move the dual List-and-Item implementations from the macro to deeper in the compiler in the future. Instruction counting based metrics look worse than expected, however my real-world benchmarks show a frame rate drop from a consistent 28.8 to 27.6 FPS, which is an acceptable 4%. Tested using Chrome and this file in master and this branch. Since this PR must meet its deadline, the above can be recovered through future optimizations subsequent to this landing. |
There was a problem hiding this comment.
All reported issues were addressed across 15 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Performance Benchmark Results
|
Keavon
force-pushed
the
rank-polymorphism-foundation
branch
from
4e3a772 to
a221822
Compare
Performance Benchmark Results
|
…em<T> primary input
…ing trait boilerplate
…h, and Omit Element to Remove at Index
…ing the Sort Key node
…ement TypeIds survive to default-literal parsing
…-paint migration, quantize List rows, and other smaller issues
… speeding up cached evaluation
Keavon
force-pushed
the
rank-polymorphism-foundation
branch
from
3157d02 to
ff7714d
Compare
Performance Benchmark Results
|
0HyperCube
left a comment
0HyperCube
left a comment
There was a problem hiding this comment.
I think that this is in principle a great improvement. Having the rectangle node return a list of vectors has baffled me ever since you first changed it.
I have left a couple of considerations here, however I was unable to look at most of the files in this PR (GitHub apparently can't render long diffs and I don't have the attention span).
Since this PR must meet its deadline, the above can be recovered through future optimisations subsequent to this landing.
+8,774 -5,043
I'm a bit confused as to why you are rushing this large change
Instructions: 59,385,391 (master) → 93,004,444 (HEAD) : + 56.61 %
6.16 MB build size increase
This is pretty dramatic +25% binary size for a web file. I guess you did add a lot of types for the monomorphisation in the dynamic executor and we enumerate all combinations. I feel there are some easy wins in this regard which I have pointed out in my comments.
…age of the ranked monitor row types
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…d scalar and choice enum types
Performance Benchmark Results
|
…specting them resolves, with matching data panel arms
Performance Benchmark Results
|
3 participants
Closes #3779.
Bonus: Closes #2047. Closes #4239.