[malibio]

Closes #3442.

What changed

SchemaNode is now one wire type, defined in nodespace-types and filled by the store (ADR-086 §1). Core's second struct is gone.

• One type. nodespace_types::SchemaNode carries the envelope, fields, relationships, extends, abstract, the structural rules and the two templates. description is removed (it was always empty; a schema's description is its child subtree).
• The store fills it. packages/core/src/models/schema_node.rs now holds no struct, only the storage mapping: from_storage(node, declarations), to_node, to_properties, to_declarations. A schema can only be built from a row together with its declaration edges, so there is no unfilled SchemaNode. The extends edge becomes SchemaNode::extends and is never an entry in relationships.
• Nothing rides in properties. into_wire_node is deleted. The schema RPCs return the typed SchemaNode (SchemaResponse / SchemaListResponse, JSON-encoded like the other typed payloads on this proto).
• Typed operation parameters. CreateSchemaParams, UpdateSchemaParams, FieldRename, FieldValueAddition, CreateSchemaOutput and SchemaUpdateOutput are wire types in nodespace-types, with generated TypeScript. Core exposes typed create_schema / update_schema; the daemon decodes the RPC payload into the typed params and answers with the typed output. handle_create_schema / handle_update_schema remain as the agent's JSON tool-call adapters over them.
• Surfaces. Tauri commands and the dev-proxy pass the daemon's SchemaNode through unchanged (the dev-proxy previously dropped relationships and extends). The CLI prints a schema under the keys the parameters take.

Node-type sequence: schema is a concrete core type (the meta-type), category Structured. No field, relationship or rule is added. Removed: SchemaNode.description. Docs: the schema sheet in components/node-types.md, plus components/schema-management.md and development/standards/naming-conventions.md in nodespace-docs.

Behaviour changes to review

• A schema node read as a plain node keeps the generic shape. node_to_typed_value no longer builds a SchemaNode from a row, because it has no store to fill it from; that conversion used to return a schema with empty relationships. The frontend only ever read typed schemas through getSchema / getAllSchemas, so nothing there changes. The agent's get_node on a schema id and relationship get now return the stored definition under properties.

• extends is no longer a traversable relationship name. With the parent out of relationships, get_related_nodes(…, "extends") on an instance is refused as undeclared instead of returning an empty result, and a subtype's extends edge no longer shows up as an inbound relationship on its base type.

• CLI output. schema get --json prints the schema with snake_case top-level keys (title_template, is_core, schema_version, …) and no properties; schema list --json is {"count", "schemas"} instead of {"count", "nodes"}. packages/skill/references/cli.md is updated.

• create_schema result carries extends as its own key; it used to appear as an extends entry in relationships.

• Workspace context fetches the schema corpus before semantic retrieval (same one read per turn with a query) and resolves hits against it, so a hit is never a bare row. If that read fails, schema retrieval is skipped for the turn rather than returning hits without relationships.

• Tauri get_all_schemas fails on a schema it cannot decode instead of skipping it. The daemon serializes a typed struct, so a decode failure is a contract break, not a bad row.

• A declaration write that drops the parent is refused. set_schema_relationships is a full replace, and a schema's relationships no longer holds the extends edge, so a set without it would have unlinked the type from its base. A parent is re-targeted, never cleared.

• A schema row is read whole on a generic read. Its structural rules (children, parent) are objects and were dropped by the flat-properties rule; they now come back under properties with the rest of the stored definition.

Tests

• nodespace-types: wire round trip of SchemaNode; params decode, unknown-key refusal and round trip.
• core: storage mapping (models/schema_node.rs), search-hit resolution, re-target keeps one extends edge.
• daemon: get_all_schemas and get_schema_definition return typed schemas with relationships and extends; the write RPCs carry typed params and outputs.
• Tauri: schema_read_tauri_seam_test against a real daemon; decode_schema unit tests.
• dev-proxy / TypeScript: schema-seeding.e2e.ts against a real daemon; adapter unit tests.
• CLI: schema get reads a schema under the keys schema create takes.

bun run test:changed passes (frontend, browser, scripts, skill, generated-TypeScript drift, Rust workspace). The e2e suite passes locally against a daemon built from this branch (50 tests).

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review: PR #3490

Review Type: Initial review (no prior reviews or comments on the PR). Reviewed at 4fce6e38.

Summary: The consolidation does what the issue asks. There is one SchemaNode, the store is the only production reader that fills it, extends is a field rather than a relationship row on every surface, and the schema RPCs carry typed payloads. I traced every write path and found none that loses a schema's extends edge today. No blockers. Two things are worth fixing in this PR: generic reads of a schema node drop its structural rules, which contradicts what the PR documents, and the one path where the extends edge is rewritten during a relationship edit has no assertion.

How I checked: I read the substantive files in full (not just the diff), read ADR-086 §1, the node type sequence and the schema sheet, and ran the three lint checkers (literal node-type comparisons, lifecycle_status reads, Pro boundary), all clean. I did not re-run the Rust or frontend suites, and I did not compile doctests.

Requirements Check

#	Acceptance criterion	Status	Notes
1	One SchemaNode, in nodespace-types; models/schema_node.rs holds no struct	✅	One struct SchemaNode in the workspace (nodespace-types/src/schema.rs:573). schema_node.rs is four functions. The TypeScript type is the generated one.
2	Reads return relationships and extends on core, daemon, Tauri, dev-proxy, TypeScript (tests)	✅	Core: schema_node.rs unit tests, extends_subtype_identity_test, core_schemas parent test. Daemon: two RPC tests. Tauri: schema_read_tauri_seam_test against a real daemon. Dev-proxy and TypeScript: schema-seeding.e2e.ts through HttpAdapter. The adapter unit tests only pass a mock through, so the e2e is the real coverage there.
3	No schema relationship written to or read from properties	✅	to_properties writes neither key, from_storage ignores both (pinned by a test), into_wire_node is gone, and no remaining reader takes them from properties.
4	SchemaNode.description is gone	✅	Removed from Rust, generated TypeScript, dev-proxy and fixtures. CreateSchemaOutput.description remains, correctly: it is the operation's echo of the subtree text.
5	Schema operation parameters typed end to end	✅	Wire types in nodespace-types with generated TypeScript; core takes typed params; the daemon decodes into them and refuses unknown keys. The RPC field is still a JSON string, and the CLI passes the user's JSON through, which matches the other typed payloads on this proto.
6	Follows the node type sequence; node-types.md schema sheet updated	✅	Sheet updated in nodespace-docs (65240d4). One statement in it is not true as written; see the first Important finding.
7	bun run test:changed passes; quality:fix clean	✅	Author-reported; not re-run.
8	schema get and schema update agree on where title_template lives	✅	schema_to_json prints title_template at the top level, pinned by schema_json_uses_the_keys_the_schema_params_take and by the daemon write/read test.

The questions you asked

• Can a write path lose the extends edge? Not today. The only production callers of set_schema_relationships / set_schema_declarations are create_schema, update_schema and core seeding, and all three build the list with to_declarations, which re-adds the edge. The rename and relabel paths write properties only. Every other caller is test code. The risk is latent; see the second Important finding.
• Did update_schema keep its semantics? Yes. The re-target counts as one added declaration only when the parent changes, the declarations are still written only when something was added or removed, the version check still uses the Phase 2 read, and absent abstract / children / parent are still sent as null to clear them. Dropping the type-system filters is correct, since the list can no longer hold an extends row.
• Does the generic shape for schema nodes break a consumer? No consumer breaks. The frontend reads schemas only through getSchema / getAllSchemas; the sync listeners only look at nodeType and re-fetch. No agent or daemon code reads schema keys off a generic node. But the generic shape is lossy; first Important finding.
• Anything still reading relationships or extends through properties? No.
• Leftovers? Yes, minor; listed under Suggestions and Nits.

Code Review Findings

🟡 Important

1. A schema node read as a plain node silently drops its children and parent rules
📁 packages/nodespace-types/src/convert.rs:72-80, :176-185, :199-207; test at :1101; packages/skill/references/cli.md:26-29

• What is wrong: node_to_typed_value runs flatten_properties_for_api before choosing the shape. A schema row has flat properties and no schema bucket, so it takes the already-flat fallback, which drops every object-valued key. The structural rules are stored as objects ({"rule": "none"}), so they are dropped; fields (an array) and abstract (a bool) survive. The CLI's node get uses the same rule.
• Why it matters here: this PR makes the generic shape the contract for get_node on a schema id and for relationship get, and both the schema sheet and cli.md say the node carries "its stored definition under properties". For any type with a non-any rule, including seeded core types, that is not what comes back, and an agent reading it would conclude the type has no structural rule. The drop itself predates the PR (the old schema arm read the same flattened properties and reported any), but the old shape made no such claim.
• Verified by: reading the code path. I did not execute it. The new test does not catch it because its fixture has no object-valued key.
• Recommendation: pass a schema row's properties through unflattened (its registry entry already says it has no bucket), and add children and parent to the fixture in schema_read_as_a_plain_node_keeps_the_generic_shape. If you would rather not touch the flatten rule, correct the sheet and cli.md to say structural rules are omitted from a generic read.
• Principle: a documented contract should match behaviour; silent partial reads are worse than refused ones.

2. The path that rewrites the extends edge during a relationship edit has no assertion, and the write API invites dropping it
📁 packages/core/src/schema/schema_test.rs:5358-5395, :4664-4682; packages/core/src/schema/mod.rs:3181, :3330-3334; packages/core/src/services/node_service/schema.rs:973-997

• What is wrong: when update_schema adds or removes an ordinary relationship on an extending schema, it replaces the whole declaration set, and the parent edge survives only because to_declarations puts it back. The test that exercises this (test_add_relationships_only_call_still_allows_a_genuinely_new_relationship) asserts only is_ok(). The test whose comment says "relationships is a full replace, so the existing declaration has to survive" uses add_fields, which never reaches the replace.
• Why it matters: before this PR schema.relationships contained the extends row, so passing it back to set_schema_relationships was safe. Now the same call deletes the parent edge, and the live-edge guard does not stop it (no instance carries an extends edge). The ancestry triggers would then unlink the subtype from its base with no error. Nothing does this today, but the method is pub, takes the same type as SchemaNode.relationships, and is called with hand-built lists across the tests.
• Recommendation: assert persisted_extends_target(&svc, "bug") == Some("ticket") in the test above, and add a remove_relationships twin. Optionally close the trap in the signature: have set_schema_relationships take &SchemaNode, or the relationships and the parent as separate arguments, so a caller cannot omit the edge by accident.
• Principle: test the failure mode the design depends on; make the wrong call hard to write.

🟢 Suggestions

3. "Only the store fills a SchemaNode" is a convention, not something the code enforces
📁 packages/core/src/models/mod.rs:22; packages/core/src/behaviors/mod.rs:1352; packages/nodespace-types/src/schema.rs:558-568
schema_node became pub mod, but nothing outside nodespace-core's own src uses it. SchemaNodeBehavior::validate calls from_storage(node, Vec::new()), which is an unfilled SchemaNode built outside the store, while the type's doc comment (exported to TypeScript) says nothing does that. Make the module pub(crate), and either validate the row without building a SchemaNode or name the exception in the doc comment.

4. Guards made dead by the consolidation
📁 packages/core/src/ops/path_ops.rs:116, :288
Both is_type_system_relationship checks filter lists that come from resolve_relationships and get_inbound_relationships, which can no longer contain a type-system name. Remove them and the comment at :114-115.

5. get_schema_with_relationships is now a pure alias
📁 packages/core/src/services/node_service/schema.rs:1266-1272
It only calls get_schema_node. It has two callers in playbook/workflow_state.rs (:206, :732) and a stale mention at schema/mod.rs:3193. Fold it in while this is the PR that settles the one read.

6. schema_to_json will silently miss a new SchemaNode field
📁 packages/cli/src/output.rs:204-238
It is a hand-written key list. Destructure SchemaNode exhaustively (let SchemaNode { envelope, is_core, .. } = schema without the ..) so adding a field fails to compile until the CLI decides how to print it.

7. Doc example no longer matches the type
📁 packages/core/src/services/node_service/schema.rs:1207
The no_run example on get_all_schemas prints schema.id; the field is now schema.envelope.id. The gate does not run doctests, so nothing caught it. I did not compile it to confirm it fails.

8. Undisclosed behaviour change in the Tauri list command
📁 packages/desktop-app/app-lib/src/commands/schemas.rs:59-63
get_all_schemas used to skip a schema it could not parse; it now fails the whole list on one decode error. That is the better behaviour given the daemon serializes a typed struct, but it belongs in the PR body's list of behaviour changes.

Nits

• Nit: packages/core/src/schema/mod.rs:686-713: the doc block for reserved relationship names is now glued onto extend_inherited_field (:731). It belongs on reject_reserved_relationship_names (:886), which has none.
• Nit: six comments still name parse_and_filter_non_core_schemas, renamed to non_core_schema_hits: core/src/ops/skill_ops.rs:298, :1296; agent/src/skill_rules.rs:384, :386; agent/src/local_agent/routing.rs:600; agent/src/local_agent/tools.rs:839. parse_schema_search_hits in skill_ops.rs no longer parses anything either.
• Nit: packages/cli/src/commands/schema.rs:3-9 still says the params are defined in packages/core/src/schema/mod.rs.
• Nit: stale wording about hydration and from_node: core/src/behaviors/mod.rs:3643, core/src/ops/context_ops.rs:68-71, core/src/playbook/workflow_state.rs:1476-1481, and the "full replace" comment at core/src/schema/schema_test.rs:4664-4665.

Recommendation

APPROVE. The change is a clear net improvement and meets every acceptance criterion. Findings 1 and 2 are worth addressing before merge: both are small, and both concern the exact contract this PR establishes.

[malibio]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[malibio]

Review feedback addressed (commits 4fce6e38..04d77c08)

Addressed: 7 of 8 findings, plus all four nits.

1. 🟡 Generic read of a schema row dropped children / parent. Fixed at the one flatten rule (flatten_namespaced_properties_at_scope), so node_to_typed_value and the CLI's node get both read a schema row whole. A schema's properties are flat, so an object among them is a structural rule, not another type's bucket. The test fixture now carries both rules and a _-prefixed key.
2. 🟡 The extends edge on a relationship edit had no assertion. The add-relationship test now asserts the parent survives, with a remove_relationships twin. I did not change the signature of set_schema_relationships (about twenty test call sites pass hand-built lists). Instead the write itself refuses a declaration set that leaves out an existing extends edge: a parent is re-targeted, never cleared. New test: test_a_declaration_write_that_drops_the_parent_is_refused.
3. 🟢 models::schema_node is pub(crate). The SchemaNode doc comment now states what holds (every schema read returns one built from the row and the edges), and the behaviour's row-only validation says why it reads with no declarations.
4. 🟢 Removed the two dead is_type_system_relationship guards in path_ops.rs.
5. 🟢 schema_to_json destructures SchemaNode in full, so a new field fails to compile until the CLI prints it.
6. 🟢 Fixed the get_all_schemas doc example (schema.envelope.id).
7. 🟢 The PR description now lists the Tauri list command's fail-on-decode-error change, the new write refusal, and the whole-row schema read.

Nits: the reserved-names doc block is back on reject_reserved_relationship_names; the renamed helper is referenced by its new name everywhere (parse_schema_search_hits is now non_core_schema_hits_with_scores); the CLI module doc points at nodespace-types; stale hydration wording is rewritten.

Skipped: 1.

5. 🟢 Fold get_schema_with_relationships into get_schema_node. The alias predates this PR, and the schema-chain-blindness guard test is built around its name: the tracked-call pattern, two allowlist entries and the scanner's own fixtures. Removing it means rewriting that guard, which is unrelated to the wire type. Worth doing when that guard is next touched.

Verification: bun run test:changed passes (frontend, browser, scripts, skill, generated-TypeScript drift, Rust workspace). Clippy with -D warnings is clean. The Tauri seam tests and the e2e suite (50 tests) pass against a daemon built from 04d77c08.

Re-Review Decision

Decision: RE-REVIEW NEEDED

Rationale: Two of the fixes add logic rather than tidy it: the flatten rule now has a schema branch that every generic read goes through, and set_schema_relationships has a new refusal on the declaration write path.

[malibio]

Pragmatic Code Review: PR #3490

Review Type: Re-Review. Scope: 4fce6e38..04d77c08 (two commits: the review fixes and the regenerated TypeScript doc comment).

Previous Review Summary

	Count
Previous recommendations	8 findings (2 🟡, 6 🟢) plus 4 nits
Addressed	7 findings and all 4 nits
Not addressed	1 (finding 5, skipped with a reason I accept)
New issues found	0 blocking; 3 nits

How I checked: I read the full diff and the code around each fix, and traced every caller of the two functions that gained logic. I ran three tests at 04d77c08, all passing: schema_read_as_a_plain_node_keeps_the_generic_shape (nodespace-types), test_a_declaration_write_that_drops_the_parent_is_refused and test_add_relationships_only_call_still_allows_a_genuinely_new_relationship (nodespace-core). I did not re-run the full suites, clippy, the Tauri seam tests or e2e; those are author-reported.

Verification of previous findings

1. 🟡 Generic read of a schema row dropped children / parent: fixed, correct.
📁 packages/nodespace-types/src/convert.rs:156-170, test at :1118-1146

• Every caller reaches the branch the same way. flatten_properties_for_api passes the registry chain, which for schema is ["schema"]. The CLI's properties_to_json (packages/cli/src/output.rs:366) passes the single scope "schema". Both hit the new branch, so get_node, relationship get and node get agree.
• It cannot fire for another type. The branch tests only the first scope, with CoreNodeType::Schema.is_exactly. The only other production caller (packages/nodespace-types/src/ai_chat.rs:516) passes an AI-chat chain. Nothing can extend schema (stated on is_exactly, core_type.rs:451-455), so schema is never a later entry in someone else's chain, and checking only the first scope loses nothing.
• No other type reads differently. The branch returns before the bucket and already-flat logic, which are unchanged.
• _-prefixed filtering is consistent with both other branches, and the test fixture pins it with _seed.
• The fixture now carries both structural rules, so the test fails without the fix.
• Not directly tested: the CLI's single-scope entry point with "schema". It shares the branch, so I consider this covered.

2. 🟡 extends edge unasserted on a relationship edit, and a write API that invites dropping it: fixed, correct.
📁 packages/core/src/services/node_service/schema.rs:1021, :1081-1109, :1147; packages/core/src/schema/schema_test.rs:5394-5454

• It does not refuse a legitimate write. I traced each path:
  • create_schema: the schema is new, so there are no existing declarations and the guard passes.
  • update_schema: the list comes from to_declarations(&updated_schema), and extends there is the stored parent or the re-target, never None when a parent exists. UpdateSchemaParams.extends absent means unchanged, so the API has no "clear" to refuse.
  • Re-target: the new set contains an extends entry, so it passes; the test covers it.
  • Core seeding: writes through SqliteStore::set_schema_declarations directly, only for missing schemas, built with to_declarations. The guard is not on that path.
  • Methodology installer: goes through handle_create_schema / handle_update_schema only (packages/core/src/methodology/install.rs).
  • Schema deletion: no production code calls set_schema_relationships outside schema/mod.rs; every other caller is under #[cfg(test)].
• Remaining ways to drop the edge: only the store-level SqliteStore::set_schema_declarations, whose one production caller is seeding. The generic delete_relationship already refuses any edge whose source is a schema node (relationship.rs:1869-1876). Keeping the guard at the service layer matches where live-edge protection already sits, so I would not move it.
• The guard reads existing from the pooled reader inside the _in_tx twin. That is the same read the live-edge check beside it already uses, and both callers pass declarations that predate the transaction. Not a new hazard.
• The tests are meaningful.
  • The add-relationship test now asserts the parent after the full replace, and its remove_relationships twin asserts it again through get_schema_node. Either would fail if update_schema stopped writing the parent back.
  • The refusal test passes bug.relationships (the exact trap from the first review), checks the error and that the edge is still stored, then shows the full set and a re-target are accepted.

3. 🟢 "Only the store fills a SchemaNode": addressed. models::schema_node is pub(crate) (packages/core/src/models/mod.rs:22); nothing outside packages/core/src references it. The doc comment now states what actually holds, the behaviour's row-only validation says why it reads with no declarations (behaviors/mod.rs:1349-1352), and the generated schema-node.ts matches the Rust comment.

4. 🟢 Dead guards: addressed. Both is_type_system_relationship checks are gone from path_ops.rs. I confirmed they were dead: resolve_relationships and get_inbound_relationships (relationship.rs:2408-2449) both read SchemaNode.relationships, which from_storage partitions the type-system rows out of.

5. 🟢 Fold the get_schema_with_relationships alias: skipped, justified. packages/core/tests/it/schema_chain_blindness_guard_test.rs is built around the name: TRACKED_CALLS at :572, a fixture at :905, and about ten doc references. Removing the alias means rewriting that guard, which is unrelated to the wire type. The stale mention in schema/mod.rs was reworded. The cost of leaving it is one two-caller alias.

6. 🟢 schema_to_json: addressed. packages/cli/src/output.rs:205-219 destructures SchemaNode with no .., so a new field fails to compile. Output keys are unchanged.

7. 🟢 Doc example: addressed (schema.envelope.id). Not compiled; the example is no_run and the gate does not build doctests.

8. 🟢 Undisclosed behaviour change: addressed. The PR body lists the Tauri fail-on-decode change, the new write refusal and the whole-row schema read.

Nits: all four addressed. The reserved-names doc block is on reject_reserved_relationship_names; the six comments use non_core_schema_hits; parse_schema_search_hits is non_core_schema_hits_with_scores; the CLI module doc points at nodespace-types; the hydration wording is rewritten. The new semantic_schema_count doc matches the code at context_ops.rs:795-817.

New findings

No blocking or important issues. Three nits, none of which need a re-review.

• 🟢 Nit: 📁 packages/nodespace-types/src/convert.rs:117-120, :145-147. The two doc comments describe the flatten rule as two branches and say a single scope "preserves the pre-extends behavior exactly". With the schema branch that is no longer the whole rule. One sentence naming the schema exception would keep the "single definition of the rule" comment true. Principle: a comment that claims to be the full rule should be.
• 🟢 Nit: 📁 scripts/eval/fixtures/decisions.ts:483. Still names parse_and_filter_non_core_schemas. It was not on the first review's list, which covered only Rust files.
• 🟢 Nit: 📁 packages/core/src/services/node_service/relationship.rs, the doc comment on the relationship graph summary just after get_inbound_relationships (:2459-2462). It says resolve_relationships "filters" the extends edge. Nothing filters it now; a SchemaNode holds it as extends, as the rewritten doc on resolve_relationships says. Not touched by this diff.

Recommendation

APPROVE. Both fixes that added logic are correct for every caller I traced, the new tests fail without them, and the fix commits introduce no new problem. The three nits are optional comment touch-ups.

[malibio]

Automated pragmatic-code-review (re-review): APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[malibio]

Re-review feedback addressed (fb04e4e0)

All three nits from the re-review are fixed, comment-only:

• convert.rs: the flatten rule's doc now names the schema exception, and the single-scope sentence says what it does today.
• scripts/eval/fixtures/decisions.ts: the renamed helper is referenced as non_core_schema_hits.
• relationship.rs: the graph-summary doc says a schema holds its parent as extends rather than that something filters it.

Nothing skipped. cargo fmt --check, the scripts lint and types:check are clean; no code path changed since the run that passed test:changed.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: The re-review approved the logic changes, and this commit touches comments only.

[malibio]

Merge queue: #3490 was taken out of the queue — it conflicts with main in: packages/core/src/models/mod.rs, packages/daemon/src/services/node_service.rs, packages/desktop-app/src/lib/types/generated/index.ts, packages/nodespace-types/src/convert.rs, packages/nodespace-types/src/lib.rs. Rebase onto origin/main, push, and re-run bun run merge 3490.