Repository navigation
feat(contract): mode_type and primary_key contract fields - #74
Conversation
MODE_TYPES and DEFAULT_MODE_TYPE (replace) join the model; Node carries mode_type and primary_key as written. KindRules.mode_types allows every mode for python-node and python-api and only replace for python-csv. Signed-off-by: Simone Carolini <simonecarolini.sc@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137E9XmyUXhwPTRrtNfivDw
mode_type is one of append, upsert, replace (default replace) and must be allowed for the node's kind. primary_key is required for upsert and rejected otherwise; it is a non-empty list of distinct names (compared case-insensitively), each naming a nullable: false output column exactly as spelled there. Signed-off-by: Simone Carolini <simonecarolini.sc@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137E9XmyUXhwPTRrtNfivDw
mode_type appears only when it is not replace and primary_key only for upsert, in authored order, so every existing entry and its config_hash and content_hash stay byte-identical (pinned against a 0.8.0 known vector). Signed-off-by: Simone Carolini <simonecarolini.sc@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137E9XmyUXhwPTRrtNfivDw
Canonical wire entries for a replace, an append and an upsert node, without hash fields. The real loader and node_entry reproduce each entry exactly; the replace entry carries neither mode_type nor primary_key. continuo keeps a byte-identical twin under topology-controller/tests/fixtures. Signed-off-by: Simone Carolini <simonecarolini.sc@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137E9XmyUXhwPTRrtNfivDw
carolsimone
left a comment
There was a problem hiding this comment.
Reviewed all 12 changed files, including the supporting tests, documentation, and shared fixture. Validation: 767 tests passed, 33 image/integration tests deselected; Ruff and mypy passed. Live warehouse behavior and the coordinated cross-repository rollout were not tested.
The PR description clarifies that this is R1, writer support is deferred to R6, and nothing is released before the series' single tag. With that context, the comments below are integration requirements before that release. The replace-only write path has high data-loss severity if these new modes are shipped or executed before R6; its current presence is consistent with the stated staging plan.
| mode_type = raw.get("mode_type", DEFAULT_MODE_TYPE) | ||
| if not isinstance(mode_type, str) or mode_type not in MODE_TYPES: | ||
| raise ContractError( | ||
| f"{label}: 'mode_type' must be one of {sorted(MODE_TYPES)}, got {mode_type!r}" | ||
| ) | ||
| if mode_type not in rules.mode_types: |
There was a problem hiding this comment.
Series release requirement: honor append/upsert before executing these contracts.
At this commit, run_node() accepts a valid append/upsert contract through load_contract_dir(..., check_reads=False), then unconditionally calls active_adapter.load(...). The port and all three shipped adapters define that operation as replacement: Postgres truncates, DuckDB deletes all rows, and Trino swaps in a replacement table. For example, existing id=1 plus a run producing only id=2 leaves only id=2 and reports success; an empty batch removes all existing rows.
The PR explicitly defers writer support to R6 and holds release until the series is complete. Please ensure R6 implements the mode dispatch and regression coverage for preserving existing rows under append/upsert before the single tag. If these contracts can be run before the new writers are available, reject unsupported modes before any writes.
There was a problem hiding this comment.
Addressed in c587b88 (this branch). run_node now rejects any non-replace mode_type with a LoadError immediately after node selection — before the script runs or ensure_table/load is called — so an accepted append/upsert contract can no longer silently replace a table. A test asserts both modes return the error sentinel with no adapter write and no script side effect. This is an interim guard; R6 replaces it with the per-mode _WRITERS dispatch + capability check and adds the row-preservation end-to-end coverage, all before the single v0.9.0 tag.
| if node.mode_type != DEFAULT_MODE_TYPE: | ||
| entry["mode_type"] = node.mode_type | ||
| if node.mode_type == "upsert": | ||
| entry["primary_key"] = list(node.primary_key) |
There was a problem hiding this comment.
Cross-repository rollout dependency: land the consumer parser update before using the new wire entries.
Continuo's current main parser (topology-controller/service/python_contract_parser.py) omits mode_type and primary_key from _OPTIONAL_ENTRY_KEYS and rejects unknown fields. Consequently, the append/upsert entries emitted here are rejected by that consumer, even though contract_version remains 1.
The coordinated series needs the companion parser update before these entries reach an install. The shared fixture should be exercised through that consumer as well as this producer to verify the completed integration. This is a series rollout check given the PR's stated R1 scope and single-tag release plan.
There was a problem hiding this comment.
Noted — this is a cross-repository dependency outside continuo-python-runtime (this repo is scoped to the runtime only). The companion update to continuo's topology-controller/service/python_contract_parser.py (adding mode_type/primary_key to _OPTIONAL_ENTRY_KEYS) and the byte-identical fixture twin are tracked on the continuo side. Nothing in this series is released before the single v0.9.0 tag, which gates when these wire entries can reach an install. The runtime fixture's sha256 for the twin is 0021cfab2f3c509805c0b64b6f3fa0cfb97f2c3b703303a66bad00abc946eb3a.
run_node() still writes every node with load() (replace). An accepted append or upsert contract would therefore silently delete existing rows. Reject any non-replace mode_type before the script runs or anything is written; the per-mode dispatch in a later PR removes this guard. Signed-off-by: Simone Carolini <simonecarolini.sc@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137E9XmyUXhwPTRrtNfivDw
First runtime PR of the python node write-modes series (spec: python-node-write-modes-design, R1).
mode_type(replacedefault,append,upsert) andprimary_keyon the contract: loader rules from spec section 3,Node.mode_type/Node.primary_key,KindRules.mode_types(python-csv= replace only).node_entry()emitsmode_typeonly when it is notreplaceandprimary_keyonly forupsert, in authored order. Existing entries and hashes are byte-identical (pinned against a 0.8.0 known vector);contract_versionstays 1.tests/fixtures/wire_contract_write_modes.yml(sha256: 0021cfab2f3c509805c0b64b6f3fa0cfb97f2c3b703303a66bad00abc946eb3a), byte-identical twin in continuo's topology-controller.The harness still writes every node with
loaduntil R6; nothing is released before the series' single tag.🤖 Generated with Claude Code
https://claude.ai/code/session_0137E9XmyUXhwPTRrtNfivDw