FE-1270: Extract Python bindings for the Petrinaut CLI into @local/petrinaut-python - #9228
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
c1e9f74 to
a2a4545
Compare
a2a4545 to
cfbbb60
Compare
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit 7f897d7. Bugbot is set up for automated code reviews on this repo. Configure here. |
14efab7 to
4fe418d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (1)
libs/@hashintel/petrinaut-cli/README.md:15
- The architecture check described by this PR does not currently cover this new relationship.
libs/@local/petrinaut-arch-docs/architecture.config.ts:34-57registers only core, editor, and CLI;content/index.mdx:78-80still saysapps/petrinaut-optis outside the generated model; andlibs/@hashintel/petrinaut-cli/src/README.md:9-12still says the optimizer directly spawns the CLI. Consequently the generated docs remain stale and nooptimizer → python-bindingsedge is enforced. Register the optimizer and bindings with layer declarations and update the stale architecture prose as part of this extraction.
The [`@local/petrinaut-python`](../../@local/petrinaut-python/README.md)
bindings drive this CLI from Python: sessions, run requests, and optimization
studies.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (1)
libs/@local/petrinaut-python/src/petrinaut/optimization.py:14
- The stated architecture check cannot currently cover this new boundary:
libs/@local/petrinaut-arch-docs/architecture.config.ts:35-57registers only the core, editor, and CLI packages, whilesrc/extract.ts:158-165explicitly rejects Python packages because no extractor exists. As a result, the claimedoptimizer → python-bindingsedge is never generated or enforced, so a direct optimizer-to-CLI dependency would pass the architecture check. Add Python extraction/registration for both this package andapps/petrinaut-opt(with their layer declarations and graph wiring), or remove the claim and track that enforcement separately.
class OptimizationSession(PetrinautSession):
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (2)
libs/@local/petrinaut-python/src/petrinaut/session.py:328
PetrinautClientErrormeans the process/transport is unusable (errors.py:6-7), andrequest()explicitly promises that this exception closes the session (lines 204-207). This branch deliberately leaves the session healthy, so callers cannot use the documented exception type to decide whether the session can be reused. Treat non-serializable params as caller input—consistent with the constructor’sValueErrors—rather than as a transport failure.
libs/@local/petrinaut-python/README.md:79- The uppercase constants are defaults, not constructor option names. A caller following this text and passing
BOOTSTRAP_TIMEOUT_SECONDS=will get an unexpected-keywordTypeError; namebootstrap_timeout_secondsandrequest_timeout_secondsas the options and identify the constants as their defaults.
- Bootstrap (spawn to readiness) and each protocol response have deadlines,
`BOOTSTRAP_TIMEOUT_SECONDS` and `PROTOCOL_READ_TIMEOUT_SECONDS`. Both are
constructor options.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9228 +/- ##
=======================================
Coverage 59.63% 59.63%
=======================================
Files 1420 1420
Lines 138775 138775
Branches 6557 6557
=======================================
Hits 82753 82753
Misses 54958 54958
Partials 1064 1064
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2009728. Configure here.
|
I think its worth addressing the cursor issue here too. Sorry forgot to mention it earlier! |
Adds libs/@local/petrinaut-python: a stdlib-only, POSIX-only package wrapping the CLI's JSON-lines protocol. PetrinautSession serves a model (from a file or over stdin) with healthz/metadata/run plus a generic request method; OptimizationSession adds describe/evaluate/objective over an opaque manifest, and evaluate returns per-seed replicates when the manifest asks for more than one seed. The code comes from petrinaut-opt's client: spawn scrubbing, bounded reads, and process-group shutdown are unchanged. petrinaut-opt consumes the package as an editable uv path dependency, mirrored as a workspace:* edge so Turbo pruning and change detection see it; its codegen inputs, the Docker python-deps/runner stages, and the deploy path filter follow. The old src/petrinaut_client.py, the PYTHON_INTEGRATION.md walkthrough, and examples/python_stdio.py are retired, and MODEL_EXAMPLES.md now uses the bindings. Two end-to-end pytests drive the real built CLI bundle, one per session type.
Yes,
|
This naming is a bit confusing. To add to these, why do we need a |
The method sat on a class already named OptimizationSession, while its siblings evaluate() and objective() carry no prefix. describe() maps 1:1 to the protocol's optimization.describe. The previous name remains as a class-level alias. Raised in review on #9228.
Encoding failures happen before anything is written: params or a bootstrap payload that will not serialize to JSON raise TypeError, and a payload over the line cap raises ValueError. Both previously raised PetrinautClientError, whose contract says the process or transport is no longer usable — a caller treating it as terminal would discard a healthy session. The request id is now consumed only after a successful encode, so a rejected request leaves the session exactly as it was. Raised by Cursor and in review on #9228.
This seems to be a pretty established convention, and is what |
Yes, I'm gonna rename that, and they are still binding/wrappers for |
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |

🌟 What is the purpose of this PR?
apps/petrinaut-optcarried its own Petrinaut CLI client, private to that service and covering two of the CLI's five protocol methods. This PR extracts the client into a package any Python code can use, and removes the service's knowledge of the CLI.Bottom of stack #9269; FE-1467 (#9261) sits above.
Most of the diff is a move:
petrinaut_client.py→session.py.🔗 Related links
🔍 What does this change?
New package
libs/@local/petrinaut-python(import namepetrinaut), internal to the monorepo:PetrinautSessionruns one model per CLI process and exposeshealthz(),metadata(),run(), andrequest().OptimizationSessiondrives a study over a manifest it treats as opaque JSON:describe(),evaluate(),objective().PetrinautRunError(recoverable) andPetrinautClientError/PetrinautProtocolError(session closes).apps/petrinaut-optdepends on the bindings and nothing else. It no longer names the CLI in code, tests, docs, configuration, or the image:@hashintel/petrinaut-clias a runtime dependency, so the Docker image provisions the bindings' closure (turbo prune @local/petrinaut-python) and links the closure'spetrinautexecutable into/usr/local/bin. The image ships built output only.uv sync --no-editablebuilds the bindings into the virtualenv, so the runner copies the virtualenv alone.PETRINAUT_CHILD_NODE_OPTIONS; the image's ownNODE_OPTIONSnever reached the child and is removed.optimizer → python-bindingswith no edge tocli.Review fixes.
from_model_fileandfrom_modelare staticmethods rather than classmethods: they always build a basePetrinautSession, and a classmethod read as a subclass constructor onOptimizationSessionwhile returning the base class.PetrinautRunError's docstring now also names the non-finite-objective case, and a doubled sentence in the README's development instructions is fixed.Two follow-up commits address the remaining review threads:
describe_optimization()is renamed todescribe(): the old name repeated the class name, its siblingsevaluate()/objective()carry no prefix, anddescribe()maps 1:1 to the protocol'soptimization.describe. The previous name remains as an alias.PetrinautClientError, whose contract says the session's process or transport is gone:paramsor a bootstrap payload that will not serialize to JSON raiseTypeError, and a payload over the 8 MiB line cap raisesValueError— all before anything is written, leaving the session untouched (the request id is only consumed after a successful encode). Raised by Cursor and in review.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
@local/petrinaut-pythonis private.📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
turbo.json's have been updated to reflect this: the bindings'test:unitdepends on the CLI'sbuild, andpetrinaut-opt's codegen inputs include the bindings'*.pysources.🛡 What tests cover this?
test_petrinaut_client.py, plus end-to-end tests that spawn the built CLI for a run and an optimization study.apps/petrinaut-opt: 75 tests pass against the package.Build petrinaut-opt (arm64)in CI builds the rewired image.❓ How to test this?
turbo run test:unit --filter @local/petrinaut-python --filter @apps/petrinaut-optuv run python -c "from petrinaut import OptimizationSession, PetrinautSession"🤖 Generated with Claude Code