Repository navigation
feat(python): walking skeleton for the Python SDK - #1405
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Add the fourth Cargo workspace at languages/python with two crates: stackencrypt-core (no pyo3, depends on stack-encrypt by path) and stackencrypt-engine (the `_engine` cdylib, pyo3 0.29, abi3-py39). The pure-Python package `stackencrypt` exposes `__version__` (0.1.0, from the engine crate's manifest through maturin) and `__engine__` (the checkout's commit, `-dirty` when packages/stack-encrypt has local edits). `extension-module` is set only by maturin, so `cargo test --locked` links libpython. Tasks live in languages/python/mise.toml: build, test, test:cargo. The Cargo.lock is seeded from the root one because stack-encrypt requires a yanked cllw-ore pre-release a fresh resolve cannot select. Exclude the workspace from the root Cargo.toml; add it to the Dependabot cargo entry and the publish opt-out and lock freshness guards; update AGENTS.md and SECURITY.md. Mark ADR-0001 accepted, add the Python row to ADR-0002's host-language table, and start the Python section of docs/sdk-design-principles.md, the doc edits the spec says ride with the first PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… comments `-dirty` only watched packages/stack-encrypt, but the engine also takes stack-kms and stack-encrypt-derive by path, so an edit there shipped under a clean SHA. The check now covers every packages/stack-* crate. The comments listing core's future responsibilities now point at the spec instead, so they cannot drift into a second one. Refs #1382 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ADR-0001 is recorded as accepted, with its open items listed and its kind rule amended to the spec's. ADR-0002 cites the spec, not the ADR, for the value rules. The glossary records the `_engine` naming. The Dependabot guard now reads [workspace.dependencies], which hid the Python edge. The dirty check skips fuzz corpora. The unasked release profile and rlib are gone. Refs #1382 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
30e4646 to
a0ae3fc
Compare
Mutation testing (cargo-mutants,
|
The lockfile added with the walking skeleton pinned vitaminc 0.6.1, but the stack-encrypt it builds by path already required 0.8 (8f9c9fb), so any `cargo --locked` in this workspace refused to run. Regenerated with the smallest change cargo allows: the vitaminc family moves to 0.8.1, ore-rs to 0.10, and clww-ore and base85-simd are added, matching the root workspace's move. Refs #1382 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation
Reviewed 98be072 · posted from Claude Code · claude-sonnet-5-5 · effort medium
Sources: code-review (inline read of the diff) · claude-sonnet-5-5
Skills: code-review, review-cross-check, review-post
No findings.
Scope note: the diff was read in sections. The generated languages/python/Cargo.lock (lines ~228–3828 of the diff), the ADR/docs changes and the SECURITY.md/AGENTS.md edits were not read in full. The build.rs -dirty pathspec (absolute paths containing .., plus :(exclude)) was checked with git status against an existing directory and behaves correctly. Existing PR feedback consisted only of bot comments (changeset-bot, CodeRabbit skipped, cargo-mutants); nothing to cross-check.
Generated by Claude Code
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🟡 merge after changes
Before merging, add a test for the -dirty and engine-commit logic in build.rs (finding below). The other findings can wait for a follow-up.
Other findings not posted as comments
- Optional (
languages/python/tests/test_smoke.py:17): the test compares__version__to the literal"0.1.0", so the first version bump fails it. Check the version form instead. - Optional (
languages/python/crates/core/build.rs:77):rerun-if-changedwatches each wholepackages/stack-*directory, includingfuzz/output. Fuzz runs trigger rebuilds that cannot change the-dirtyresult. Watchsrc/andCargo.tomlinstead.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 2 found, 2 posted |
| claude | claude-opus-5-5 | rust | 3 found, 2 posted |
| claude | claude-opus-5-5 | python | 1 found, 0 posted |
| codex | gpt-5.6-terra | test-gap | 1 found, 1 posted |
| codex | gpt-5.6-terra | rust | 0 found, 0 posted |
| codex | gpt-5.6-terra | python | 0 found, 0 posted |
Synthesis: claude-sonnet-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 1 posted finding(s) were raised by two or more models.
Plain language: the reader test did not run; only the word-and-shape check was applied.
Stack: position 1 of 15 (:asterisk: #1405, #1406, #1414, #1415, #1433, #1437, #1453, #1499, #1509, #1510, #1511, #1513, #1516, #1517, #1582). *️⃣ marks this pull request.
Context loaded: the description, 3 linked issue(s) and 4 discussion entries.
Summary
The first piece of the Python SDK,
stackencrypt. It installs into avirtualenv and exposes two facts: its version (0.1.0) and the commit of
the Rust encryption engine it was built from. It encrypts nothing yet.
The point is the build path: a Rust extension built by maturin, over
the
stack-encryptcrate in this repository, importable from Python.Changes
languages/python: a fourth Cargo workspace with two crates.corehas no Python dependency, so its tests run without an interpreter.
engineis the pyo3 extension module. A pure-Python package wraps it.languages/python/mise.toml:build,test,test:cargo.Cargo.toml, Dependabot, and two repo guards know the new workspace.The Dependabot guard now also reads path dependencies declared at a
workspace root, which it missed before.
in ADR-0002, and a first Python section in the SDK design principles.
Verification
Run locally on macOS, Python 3.14:
mise run test(four pytest tests) andmise run test:cargo(two Rust tests) inlanguages/python.cargo metadata --lockedpasses in all four workspaces. The repo guardsuite and the supply-chain e2e test pass.
Not verified: Python 3.9, and any CI run. Both are #1383.
Related
Closes #1382. Refs #1341 (the spec). Stacked on #1284; retarget to
nextafter that merges.
Review notes
No Python lockfile yet, and no CI workflow: both are deferred to #1383.
The engine commit ends in
-dirtywhen apackages/stack-*crate hasuncommitted changes.
🤖 Generated with Claude Code