Skip to content

Optimize eligible sketch-instance execution and cancel native work - #13883

Open
ksbharaj wants to merge 6 commits into
codex/sketch-instance-selectionfrom
codex/sketch-execution-performance
Open

ksbharaj wants to merge 6 commits into
codex/sketch-instance-selectionfrom
codex/sketch-execution-performance

Conversation

@ksbharaj

@ksbharaj ksbharaj commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Render eligible first sketch instances without executing unrelated solids and booleans, and stop background native execution when the Python await is cancelled.

  • Add try_render_sketch_instance and its code-string counterpart. Unsupported cases return None so MCP can use ordinary execution.
  • Reuse the existing stop-after-sketch-solve mechanism while retaining function arguments, normal unit/constraint evaluation, source locations, and the selected instance.
  • Keep the existing conservative eligibility scope together: direct first-instance calls, literal inputs/arithmetic on earlier literals, supported sketch operations including sin, and an optional named region feeding the helper's extrusion.
  • Reuse the shared Python/native cancellation implementation now merged in Cancel native KCL tasks with Python callers #13859, retaining in-flight cleanup and subsequent-call regression coverage. There is no new native timer: MCP owns the call's single 120-second budget.

Scope and dependencies

Split from #13734. This PR is stacked on #13882 (codex/sketch-instance-selection); its diff is only performance and cancellation regression coverage. The point-colour change in #13734 is independent. No region/segment visualization is included, and this split adds no new optimization cases or behavioural changes. A subsequent safety fix declines the shortcut if removing a referenced outer binding could change the sketch’s meaning, falling back to ordinary execution.

The previously tested combined commit 70b856e29374d84ca2f0ca5707e0b2b427b551c9 is preserved on codex/sketch-visualization-tested-combined. Existing MCP/TTC pins stay there until the reviewed pieces are available together in a published native release.

Reviewer focus

  • Correct instance, not just a faster image: preserve geometry, constraints and placement. Imports, aliases, face-dependent sketches, later indices and unsupported helper layouts keep ordinary execution. This is not a general dependency slicer or cache.
  • Shared cancellation behaviour: uses main's spawn_py implementation from Cancel native KCL tasks with Python callers #13859; this PR no longer changes that wrapper. Cancellation is cooperative. In-flight integration tests relay a real DEV engine connection, confirm socket closure after direct cancellation or timeout, then check a subsequent call succeeds.
  • Bounded scope: the helper-layout and operation allowlist are deliberate restrictions. A new unsupported spelling should not silently render a different sketch.

The equivalence regressions compare full and optimized PNG bytes, constraint reports and placement. Saved real Flanges checkpoints motivated the optimization: two historical 519/619-second calls rendered byte-identical PNGs in approximately 1.7/1.8 seconds with the shortcut. These are focused tool benchmarks, not a controlled claim about total Zookeeper runtime. On the same newer build, a full-execution comparison hit its 240-second cap, so no precise end-to-end speedup ratio is claimed.

@vercel

vercel Bot commented Sep 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
modeling-app Ready Ready Preview Sep 15, 2026 5:00am UTC

Request Review

@codspeed

codspeed Bot commented Sep 14, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 150 untouched benchmarks
⏩ 128 skipped benchmarks1


Comparing codex/sketch-execution-performance (b91eaeb) with codex/sketch-instance-selection (5276a3f)

Open in CodSpeed

Footnotes

  1. 128 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@ksbharaj
ksbharaj force-pushed the codex/sketch-execution-performance branch from 3753dff to b91eaeb Compare September 15, 2026 04:49
@ksbharaj
ksbharaj marked this pull request as ready for review September 15, 2026 12:58
@ksbharaj
ksbharaj requested a review from a team as a code owner September 15, 2026 12:58
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.

1 participant