Repository navigation
fix: model and export STEP from a script in a read-only, unattended run (#89) - #91
Merged
Merged
Conversation
…only, unattended run (#89) DECISION: three changes for the three obstacles of the issue, each the smallest that removes the obstacle. No new API name, no relocatable state directory, no rewrite of rule R3. 1. A run without the cache touches nothing next to the source (the crash). PROBLEM: `run_notebook(path, cache=False)` failed on a read-only source directory (`Errno 30` on Linux, `WinError 5` here): `_run` ended with `write_dependencies`, which writes `__marimo__/.../deps.json` beside the notebook, and `exclusive()` made the state directory for its lock. REPRODUCED on Windows with an ACL that denies creating entries in the directory, on the issue's own notebook: PermissionError at runner.py `write_dependencies`. After the change the same run prints `ok` and creates nothing. WHY THIS FIX: the dependency record exists for the cache (cache entries are keyed on the files it lists); a run that keeps no entries has no reason to move it, and moving it left the record and the entries out of step. The lock exists to protect pruning and that record, neither of which a run without the cache does. A run WITH the cache still writes its state and still fails loudly when it cannot: asking for persistence and silently not getting it would be worse. REJECTED: an environment variable or argument that moves the state directory (more surface, and the problem is that there should be no state, not that it is in the wrong place); swallowing the OSError (hides real cache failures). 2. The exporters take a build result (one step, no `.scadpkg` on disk). PROBLEM: STEP from a freshly built Part needed `capture(result, path)` and then the exporter on that path, and the default capture tessellates a scene nobody reads: 1.85 s against 0.26 s for the issue's 36-hole plate. WHY THIS FIX: `build_product_package` already accepts a build result or a definition and can leave the scene out; `read_product_package_translation_units`, which every exporter and translator goes through, now builds that package in memory when it is given neither a package nor bytes nor a path. So STEP, STL, OBJ, MJCF and FreeCAD all gain the one-step form with the same validation, under the existing names. A value that is no product still raises TypeError. REJECTED: `exporter.step.export_part(part_or_solid, path)` as the issue suggests: a bare Part or solid has no definition (feature graph, topology snapshot, hashes), those come from `@scad.part`; and a second family of names beside `export_product_package_to_*` (the tests pin that these live only in `scad.exporter`). 3. The skill lists the plain script as an entry for a single-script task. PROBLEM: R3 says a model is a marimo notebook; an agent given "one script, `--output part.step`" read the skill and chose another library. WHY THIS FIX: R3 keeps its force for everything a person opens or reuses; it gains one exception, a task that itself asks for a single script, and the template (which I ran in a read-only directory: STEP written, source untouched) lives in the export domain page next to the rule it relaxes. THIS IS A POLICY DECISION OF THE OWNER, made here provisionally: R3 was a deliberate rule, and the PR says so. Not done: the issue's timings were not rerun on Linux, only on this Windows machine (same direction). Checked: test_notebook_runtime and the cache tests (123 passed), test_product_exporter/packages/capture/assembly pass with the three new exporter tests, test_skill_lint passes. test_skill_build::test_files_without_directives_copied_verbatim fails on the base commit as well. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
… result, ProductInput) The committed API pages must equal what tools/auto_docs_gen.py writes (test_auto_docs_gen); the new parameter type of export_product_package_to_step/stl/obj/mjcf was not in them, which failed Python 3.10-3.13 in CI. The cut and intersect pages the generator also touched belong to #92 (they come from the native module installed here, which has that branch's signatures) and are left out. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
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.
Fixes #89 (all three points). Decisions and rejected alternatives are in the commit message.
run_notebook(..., cache=False)wrote__marimo__/.../deps.json(and made the lock directory) next to the source. A run without the cache now touches nothing there; a run with the cache is unchanged. Reproduced on Windows with an ACL-read-only directory on the issue's notebook; fixed.export_product_package_to_step/stl/obj/mjcfand the FreeCAD translator accept the result of a build (or a definition) and package it in memory without the scene: 1.85 s -> 0.26 s for the issue's plate.domains/export-and-translation.md(run in a read-only directory).Needs the owner's call: (3) relaxes rule R3, which was deliberate.
Not run here: the issue's Linux/container (
--read-only,-u 65534) setup; the POSIX read-only test is skipped on Windows and runs in CI.🤖 Generated with Claude Code