Skip to content

Add building the artifact graph in sketch mode, take 2 #7557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 23, 2025

Conversation

jtran
Copy link
Contributor

@jtran jtran commented Jun 20, 2025

Resolves #5166. Replaces #5093.

Thanks to #5068, we can build the majority of the artifact graph without the engine. I verified that the graph gets built without exiting sketch mode.

Note: Mock UUIDs don't match real UUIDs. From past conversations, we think this is desirable. But I'm not actually sure why it is different. Unlike #5093, I didn't make a new, separate IdGenerator.

I made a Rust-side test, but not a TS-side test. It isn't user-facing. Presumably, whatever user-facing feature that relies on this will get tested.

We remove skip_serializing_if because empty Vecs are getting used for the first time in this PR, and that's causing a TS runtime error. Aleph-Alpha/ts-rs#393 fixes the TS types to reflect that. We need to upgrade to ts-rs v11.0. Until then, just don't skip.

@jtran jtran requested a review from Irev-Dev June 20, 2025 23:51
@jtran jtran requested review from a team as code owners June 20, 2025 23:51
Copy link

vercel bot commented Jun 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 23, 2025 3:24pm

@jtran jtran requested a review from andrewvarga June 20, 2025 23:54
Copy link

codspeed-hq bot commented Jun 21, 2025

CodSpeed Instrumentation Performance Report

Merging #7557 will not alter performance

Comparing jtran/sketch-mode-artifact-graph-take2 (3df43c4) with main (5f2a10e)

Summary

✅ 85 untouched benchmarks

@andrewvarga
Copy link
Contributor

Great to see this!

I did check the issue mentioned here which is still occurring but I think that's because we still pass an outdated sketchDetails.sketchNodePaths to setupSketch as discussed here.
So probably that can be fixed by removing that and using the - now available - artifact graph in sketch mode.

@jtran jtran enabled auto-merge (squash) June 23, 2025 15:18
@jtran jtran merged commit eabcf86 into main Jun 23, 2025
76 checks passed
@jtran jtran deleted the jtran/sketch-mode-artifact-graph-take2 branch June 23, 2025 15:34
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.

Generate the artifact graph in sketch mode
2 participants