Skip to content

Fix wire type matching to only allow conversions the compiler actually makes - #4688

Merged
Keavon merged 4 commits into
masterfrom
input-adapter-conversion-table
Oct 21, 2026
Merged

Keavon merged 4 commits into
masterfrom
input-adapter-conversion-table

Conversation

@Keavon

@Keavon Keavon commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Follow-up to #4670. When you hover a node with a wire, it reveals hidden inputs based on a type check that didn't match what the compiler does, so it got things wrong in both directions:

  • It assumed every ranked input converts through its element's input_adapter. The preprocessor only puts one in front of inputs registered with a single ranked type. So a Color wire was treated as fitting inputs like Fill's Content, and that fails to compile once connected. Now the editor asks the preprocessor (input_adapter_identifier, the same function it uses itself) which adapter each input gets, and allows only that adapter's conversions.
  • It ignored rank promotions, so an item wire going into a list input was rejected even though type resolution handles that case. The promotion rules moved into Promotion::between, and both the compiler and the editor use it now.
  • A subgraph import that feeds a Monitor first (like in Path) accepted nearly any type, because Monitor is generic. Nodes that pass their input type through unchanged are now looked through, and what they feed decides what the import accepts.

One example of something this fixes from #4670 is the case of dragging a Vector wire over a Fill or Stroke node which should expand the node to show the Paint parameter. It should work equally for both, as they're both valid. But as of #4670, it only the Fill node works and the Stroke node doesn't expand to offer it, which this followup PR fixes. It also fixes a number of misleading and broken type details across the tooltips, such as the Path node's primary input showing the entire set of all inputs accepted by the Monitor node (essentially all wire types, which the tooltip can't hope to fit on screen).

@Keavon
Keavon requested a review from TrueDoctor as a code owner October 13, 2026 09:33
@TrueDoctor

Copy link
Copy Markdown
Member

I deliberatly choose not to implement this in my pr because dealing with rank promotions is very ugly, I will need time to review this in detail since it touches the compiler and type resolution so you may not merge this on your own

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@TrueDoctor

Copy link
Copy Markdown
Member

I don't think I'll be able to impartialy judge the code quality at the moment or have time to work on improvements. So if you need merge this to reach some goal, you can do that without risking me holding this against you

@Keavon
Keavon force-pushed the input-adapter-conversion-table branch from 46b6616 to 832f5aa Compare October 21, 2026 04:53
@Keavon
Keavon merged commit c95588e into master Oct 21, 2026
11 checks passed
@Keavon
Keavon deleted the input-adapter-conversion-table branch October 21, 2026 05:07

This branch was successfully deployed

1 active deployment
graphite-dev (Preview) — 832f5aa0 Deployed Oct 21, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants