Repository navigation
v0.8.0 CAM integration: yank Direct Carve, bundle cam-engine sidecar, harden runtimes - #8
Merged
Merged
Conversation
Sessions 01-18 of the Project-Centered Workshop milestone: authoritative ProjectSession boundary, contextual Design Library picker, Project Plan navigation, pinned Prepare Carve flow, protected Run boundary, layout and viewport decomposition, and the 72-state responsive visual matrix with Linux packaging smoke evidence. Guided Workshop remains opt-in until the five-person novice study passes. Excludes the cambridge/ CAM sidecar (vendored nested git repo needs a vendoring decision). Adds _CPack_Packages/ and node_modules/ to gitignore. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
dw-cambridge is the isolated CAM sidecar: the dw-bridge JSON/HTTP API plus PureCutCNC vendored as a proper git submodule pinned at v0.3.0 (9011ee0). Update the pin with git -C cambridge/vendor/purecutcnc checkout <tag> and commit the new gitlink. Adapts bridge/machines.ts to the v0.3.0 machine-library refactor: the removed getBundledDefinition(id) is replaced by a lookup over the bridge's own listMachines() list. Bridge smoke and verify-camj pass against v0.3.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
heightmap_preview, analysis_overlay, and tool_recommender have no production consumers (only exercised by their own tests); the tool_recommendation_widget that displayed tool_recommender output is never instantiated. Part of the v0.8.0 Direct Carve removal.
StartStream now reads the fingerprint-verified persisted G-code file directly and streams its runnable lines through CncController, instead of fingerprint-checking the file and then ignoring it in favor of CarveJob's in-memory toolpath. Removes the adapter's last dependency on CarveJob/CarveStreamer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
CamPlaceholderPanel kept its own bool m_visible instead of the inherited Panel::m_open, so the registry's syncClose convention and setOpen() silently did nothing for the CAM window. Switch render() to the standard ImGui::Begin(title, &m_open) idiom used by every other panel, flip the registry row's syncClose to true, and delete the bespoke pre/post visibility mirror in renderPanels() that worked around the mismatch.
Removes the 26 direct_carve_* UI panel files and their dedicated
architecture tests now that CamPlaceholderPanel owns the direct_carve
window ID. Restores src/app/carve_preparation_adapter.{h,cpp} and
application_carve_preparation.cpp after the initial deletion broke the
build: resolvePrepareCarvePin/beginPrepareCarve are still live
production dependencies of the Operation-resume path and are exercised
by test_project_centered_workshop_end_to_end.cpp, which was not on the
deletion list.
Follow-up cleanup: dead uxCaptureSnapshot() accessor, dead
makeLiveProjectPlanOperationFacts producer, inlined CncStreamOrigin
local, resume-pin comment, stale build-ux-capture dir, and a dead
GCodePanel carve-streaming cluster (onCarveStream*/renderCarveProgress)
left orphaned by the panel removal.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
The adapter's production code survived Task 6 (resolvePrepareCarvePin and beginPrepareCarve are still live in resume and workshop wiring), so its edge-case coverage (AmbiguousOperation/InvalidHierarchy/PolicyRejected) comes back with it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Removes the Direct Carve pipeline sources from src/core/carve/ (job, streamer, toolpath generation/preview/advisor, gcode export, heightmap, surface analysis, island detection, roughing tool selection, material blank defaults, and the direct_carve_* policy files) and their dedicated unit tests, now superseded by the CAM integration. Keeps model_fitter and alignment_validator (still used by the viewport). Updates src/CMakeLists.txt and tests/CMakeLists.txt accordingly, including removing a duplicate heightmap.cpp entry in DW_TEST_DEPS. Also fixes two surprise includers: - tests/test_gcode_document.cpp included core/carve/gcode_export.h and had one test case exercising the deleted carve::generateGcode API; removed the include and that test case. - src/ui/panels/gcode_panel.cpp had a large-program warning that named Direct Carve stepover presets by name; reworded to be generic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Removes requestPinnedProjectDirectory (its only caller died with the preview flow), the stale carve wording in CNC settings, a stale wiring comment, and orphaned includes; documents the run-effect adapter's dormant execute() seam at its construction site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Bun PoC outcome: bundle with --target=bun and manifold-3d external plus the Bun runtime and node_modules/manifold-3d beside it (103MB, full pipeline verified). Single-file --compile is not viable: the bundler breaks manifold's emscripten glue and compiled binaries cannot resolve external packages. machines.ts imports fluidnc.json statically so the definition survives bundling. Design doc updated to the proven shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
The preflight checks themselves were removed with the study machinery in Phase 1; the summary must not claim evidence for checks that no longer run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
startOwnedProcess trusted a stale pid: after an engine crash the early return blocked every restart attempt behind a 10s unreachable wait and left a zombie. A WNOHANG reap resets the pid and falls through to a fresh fork. Port string is now built before fork() to avoid allocating between fork and exec in a multithreaded parent. Verified live: SIGKILL the child, ensureReady respawns and reports ready. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPLmsAw8QkgFDMx5SW2sV7
Track cambridge/package.json (was swallowed by a global ignore), build the cam-engine sidecar in CI before packaging, and repoint the packaging smoke at a committed pyramid.stl fixture with cambridge-relative paths so it runs anywhere and stops rewriting tracked files. Harden the engine runtimes: pre-fork env construction (no setenv/malloc between fork and exec), wedged-child kill-and-refork, dead-child fast-fail, access(X_OK) + manifold-3d payload checks, platform verdict before payload inspection, DW_BRIDGE_PORT honored app-side, quiet 1s health probes keeping the Start-engine wait inside its ~10s budget, and dead-child status reflection. Mirror the reap/fork fixes into OllamaRuntime, whose copy had diverged. Serial port opens O_CLOEXEC so sidecars stop inheriting the tty. Shared curl helpers gain NOSIGNAL, loopback NOPROXY, a POST timeout param (600s for CAM jobs, no longer misreported as unreachable), and quiet mode. The bridge server validates JobSpec shape (400), delivers 413 instead of resetting oversize uploads, validates DW_BRIDGE_PORT, handles listen errors, and drops the body cap to 8MB; the CLI gets a promise catch. Run-effect adapter: roll back the Streaming snapshot when startStream fails, report comment-only files as GCodeFileEmpty instead of missing, and trim leading whitespace in the runnable-line filter. Cleanups: delete dead river-sign/UX-capture scripts and their CTest registrations, the generated 12k-line dome-example.nc, a dead forward declaration, and the Linux-only exe-dir copy (paths::getExeDir now exported); install.sh preserves modes; build-cam-engine.sh guards its arguments and rm -rf target. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
log::warningf is the real API — the warnf calls broke every CI compiler while the local check was masked by a piped exit code. Update the payload completeness test for the manifold-3d requirement and the failed-stream adapter test for the Aborting rollback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f43612e2cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment out the four unused UIManager::init parameter names that break the Debug/coverage leg under -Werror=unused-parameter. Resume validation now resolves the project record's durable path before comparing against the mounted directory root, so network-location projects (smb:// et al.) no longer fail storageMatches and lose their resume bookmark. Import flows fire a new selection-abandoned callback when the native picker is cancelled, the selection collects no supported files, or a folder scan is cancelled; the app uses it to clear the pending Start-Project library purpose so a later unrelated import isn't routed into the wrong flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 8f6b02cf38164ff708bbf57cd0ccf9ab48869576)
setenv/unsetenv do not exist on Windows; route the app-paths and CAM runtime tests through a shared _putenv_s-backed helper, matching the guards the other suites already had. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 75b69015baed95691fc67a30d3ed55adbb15cf88)
KIO-FUSE runtime paths, exec-permission semantics, and '?' in filenames have no Windows equivalent; guard those cases so the rest of each suite still runs there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 0f362f3bda28b30b25b73752c9eae2c9489e0ad6)
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
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.
What changed
This branch lands the first two phases of the v0.8.0 CAM integration milestone plus a max-effort review pass over the whole range:
Phase 1 — Direct Carve yank. The in-process Direct Carve CAM core (
src/core/carve/, 26 UI panel files, app glue) is deleted. Thedirect_carvewindow now hosts a CAM placeholder panel; the Run adapter streams verified persisted G-code instead of CarveJob; the ProjectPlan carve stages and PrepareCarveFlow policy are retained for the Phase 3 rebuild.Phase 2 — cam-engine sidecar. A new
cambridge/environment vendors the PureCutCNC engine (submodule) behind a zero-dependency Bun/node:http JSON bridge (/api/health,/api/machines,/api/job). The C++ side gainsCamEngineClient(pure parsers + libcurl transport) andCamEngineRuntime(fork/exec lifecycle, health probing, payload discovery). Linux packaging bundles the sidecar payload (bun+ bundled bridge + external manifold-3d) into the .run installer with a dedicated smoke.Review hardening (final commit). A 10-angle review of the range surfaced 35 confirmed findings; the fixes include:
cambridge/package.jsonwas gitignored by a global pattern — the sidecar was unbuildable off-machinebuild-cam-engine.sh/data/DWabsolute paths; now uses a committed fixture with paths resolved at run timeOllamaRuntimecopyaccess(X_OK)); install.sh preserves modes under restrictive umasksDW_BRIDGE_PORT, handles listen errorsO_CLOEXECso sidecar children stop holding the CNC ttyNOSIGNAL, loopbackNOPROXY, configurable POST timeout (CAM jobs no longer misreported as "engine unreachable" after 120s)Why
Direct Carve reimplemented CAM in-process and had reached its ceiling; the PureCutCNC engine is developed and tested independently, so DW consumes it as a local sidecar over loopback HTTP. This PR gets the app to a state where the Phase 3 CAM experience (features tree, operations, toolpath preview, export to project G-code) can be built against a packaged, supervised engine.
Reviewer notes
.planning/STATE.md; only the .run installer bundles it.DirectCarveRunEffectAdapteris constructed but no production dispatcher callsexecute()until Phase 3 restores a protected-run initiator (enforced by an architecture test); onlysnapshot()is read.🤖 Generated with Claude Code