[malibio]

Closes #3421

What changed

A collection's description can now be set from the CLI and the agent, and the agent sees it in its workspace context.

Node type (node type sequence): collection is a concrete core type, category flat, generic wire shape in the registry. Main already declares the field; this PR adds no field, relationship, structural rule or participation rule:

Field	Storage	Type	Required / default
description	collection.description	text	optional / none

Docs: the collection sheet in components/node-types.md already describes the field and where it is shown; no docs change is needed.

• Schema: a test pins that collection declares exactly one field, an optional text description.
• Create op: create_collection writes the description into the collection bucket explicitly.
• Deterministic id on the generic create: node_ops::create_node (what nodespace node create --type collection and the agent's create_node call) now derives a collection's id from its name, as the collection op and imports do. Before, it minted a random UUID, so the CLI could create a second collection with a taken name. The check goes through the registry (type_is_a), so a subtype of collection is covered.
• A taken name is refused, whether it is held by id (including an archived collection) or by a renamed collection, which keeps the id of its first name. The error names the collection in the way and its id: Already exists: collection '<name>' (id <uuid>). rename_collection uses the same text.
• Workspace context: WorkspaceContext.collections carries name and description. With no descriptions the section is unchanged (COLLECTIONS: a, b). Once any collection has one, it is one collection per line, - name — description; a collection without one stays a bare name. Each description is cut to one line of 80 characters, descriptions together take at most a quarter of max_chars (later collections are listed by name), and if the list still doesn't fit the section falls back to the names line. Subtypes of collection are listed too.
• Skill guidance: references/cli.md teaches --property description=… on a collection, on create and update, and says why. SKILL.md carries one sentence: its body had 36 characters left under the 5,000-token limit, so two redundant clauses in the same section were trimmed to make room.
• Prompt golden regenerated; the fixture now has one described and one bare collection. The diff is the collections section only.

Notes for review

• The viewer needed no change. The service stores a collection's flat properties in its bucket on write, and the wire flattener returns the bucket as properties.description, which the viewer header reads. A wire test pins that.
• Wire shape: the registry records collection as a generic wire shape, so its description is written through the generic update. No typed struct is added here.
• A non-text description (the CLI parses --property description=42 as a number, and text fields are not type-checked on write) reads as no description in the workspace context. Before the fix it panicked the listing query in the database driver.
• Clearing through the generic update (--property description=null) stores a JSON null, as it does for every type; it reads as no description.
• Known limits, unchanged by this PR: the name lookup lowercases with SQLite's ASCII-only LOWER, and the first name of a renamed collection stays taken.

Tests

• core: the schema declaration; description stored in the bucket; generic create uses the deterministic id (for a subtype of collection too, which is also listed with its description) and refuses a taken name, including a renamed collection's and a padded spelling; a non-text description is listed as none; generic update sets, changes and clears; context rendering with, without, truncated (including a multi-byte boundary), the description budget, and the fallback to names.
• agent: create_node / update_node set and change a collection description.
• CLI: node create --type collection --property description=… and node update --property description=… end to end.
• wire: the bucket's description reaches properties.description.

eval:decisions before and after: posted on #3421.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review — PR #3464

Review Type: Initial Review (no prior reviews or comments on the PR)

Summary

The change does what the issue asks: description is declared on collection, written into the bucket, settable from the CLI and the agent, and rendered in the workspace context with a sensible budget. The tests are specific (exact-equality assertions on stored properties and rendered output) and would fail without the change. Targeted runs pass: core lib tests matching collection (106) and context_ops / core_schemas (82).

One defect should be fixed before merge: a non-string description makes the new query panic on every agent turn. The rest are improvements.

How I checked the blocking finding: by reading check_field_value, the CLI's parse_property, and libsql 0.6.0's FromValue for String, and by running typeof(json_extract(...)) in sqlite3. I did not reproduce it end to end.

Requirements Check

#	Acceptance criterion	Status	Notes
1	Schema declares description; create_collection stores it in the bucket; nothing writes a flat key (test)	✅	core_schemas.rs field + test_collection_declares_only_an_optional_description; create_collection_stores_the_description_in_the_collection_bucket asserts the whole properties object.
2	Viewer header reads the description from the bucket	✅	No viewer change, as the PR says: the wire flattener returns the bucket as properties.description, which collection-node-viewer.svelte:257 reads. Pinned by collection_description_reaches_the_wire_from_its_bucket.
3	CLI create goes through the deterministic-id path with the description; node update changes it (CLI tests)	✅	node_create_and_update_set_a_collection_description checks the deterministic id, the bucket, no flat key, and the duplicate refusal.
4	Agent create_node / update_node set and change the description (test)	✅	create_and_update_set_a_collection_description.
5	Context renders name — description, truncated, within max_chars; test covers with, without, truncated	✅	Five tests, including a multi-byte boundary and the names fallback. See finding 1 for the non-string case.
6	Skill guidance teaches it; SKILL.md drift check passes	✅	references/cli.md and one sentence in SKILL.md. The two trimmed clauses are still in references/cli.md (lines 372 and 626), so nothing was lost. I did not run the drift check.
7	Golden regenerated and reviewed; eval:decisions before/after posted on the issue	❌ partial	Golden diff is the collections section only. The eval results are not on the issue (its only comment is the baseline). The PR explains why it was skipped; the criterion is still unmet, so either post the run or amend the criterion on the issue.
8	test:changed passes; quality:fix clean	Not verified	I ran only the targeted core tests above.

Node type sequence. No undeclared key, no lifecycle_status read, no compatibility code, no id list, no non-UUID id, and the collection sheet in components/node-types.md already describes the field. Three checklist items name mechanisms that are not in the codebase: the CoreNodeType registry, closed-schema rejection of undeclared keys, and ts-rs generation (no matches for any of them). One statement in the PR description is inaccurate; see finding 4.

Code Review Findings

1. 🔴 Critical — a non-string description panics the workspace-context query

📁 packages/core/src/db/sqlite_store/relationships.rs:1133-1149

"SELECT content, json_extract(properties, '$.collection.description') \
 FROM node WHERE node_type = 'collection' ORDER BY content ASC",
...
collections.push((row.get(0)?, row.get(1)?));

• A text field is not type-checked on write: check_field_value (services/node_service/crud.rs:2727-2743) checks only number, boolean, date and datetime.
• The CLI parses --property values as JSON first (cli/src/commands/node.rs:120-130), so --property description=2026 or description=true stores a number or a boolean.
• json_extract returns those as SQL integer.
• libsql 0.6.0's FromValue for String hits unreachable!("invalid value type") for anything but text or null. That is a panic, not an Err, so the .unwrap_or_default() in build_workspace_context (context_ops.rs:725) does not catch it.

Result: one bad write on one collection panics context assembly on every agent turn for that database, until someone corrects the value.

Recommendation: make the read total. Either select only text values (CASE WHEN json_type(properties, '$.collection.description') = 'text' THEN json_extract(...) END; db/required_extensions.rs:94 uses the same json_type guard), or read with row.get_value(1) and match on Value::Text. Add a test that stores {"description": 42} through node_ops::update_node and asserts the listing still returns. Rejecting non-string values for text fields in check_field_value is the wider fix, but it affects every type and is a separate decision.

Principle: user data must not be able to panic a read path; validate at the boundary or read defensively.

2. 🟡 Important — "a taken name is refused" does not hold after a rename, and the docs now promise it

📁 packages/core/src/ops/collection_ops.rs:310-326, packages/core/src/ops/node_ops.rs:190-203, packages/skill/references/cli.md (the new "identified by its name" paragraph)

new_collection_id checks only whether the id derived from the name exists. rename_collection changes the name and keeps the id. So, from reading the code:

• Rename Clients to Accounts, then node create --type collection --content Accounts: the derived id is free, so a second active collection named Accounts is created.
• node create --type collection --content Clients is then refused with Already exists: Clients, although no collection has that name.

The id-only check predates this PR in create_collection. What is new is that the generic create inherits it and cli.md states the guarantee to agents.

Recommendation: in new_collection_id, also refuse when get_collection_by_name finds an active collection with that name, and add a rename-then-create test. Otherwise, soften the sentence in cli.md.

Principle: documented behaviour should match the invariant the code enforces.

3. 🟡 Important — the Already exists error gives the caller nothing to act on

📁 packages/core/src/ops/collection_ops.rs:321-323

return Err(OpsError::AlreadyExists { id: name.to_string() });

The CLI prints Already exists: clients and the agent gets create_node failed: Already exists: clients. cli.md then says "Update the existing collection instead", but node update needs the id, and the message carries the name. The id is known at that point (deterministic_id). An archived collection at that id is the worst case: it is refused, and name lookups, which match active collections only, will not find it.

Recommendation: include the existing node's id in the error, for example collection 'clients' (id <uuid>), so the next call can be node update <id>. The test that asserts id == "clients" changes with it.

Principle: error messages should make the corrective action possible, especially for a model that cannot ask a follow-up.

4. 🟡 Important — the PR description's reason for skipping the wire type is inaccurate

📁 PR description, "Not in this PR"; packages/nodespace-types/src/

The description says the typed CollectionNode wire struct and typed update depend on a registry and TypeScript generation that do not exist yet. The registry and generation indeed do not exist. But typed wire structs and typed updates do exist today for other flat types (TaskNode/TaskNodeUpdate, ProjectNode/ProjectNodeUpdate, PersonNode/PersonNodeUpdate in nodespace-types), with hand-written TypeScript beside them. So the sequence's "wire type" and "typed update" items are achievable now for collection.

Deferring them is defensible: there is no UI that edits the description (a non-goal), and promoting the field would move the viewer's read from properties.description to a top-level field. I am not asking for it in this PR.

Recommendation: correct the description so the deferral is stated as a choice, not as blocked. Decide whether the typed struct lands with the description-editing UI.

Principle: the node type sequence exists to stop layers drifting; a skipped layer should be recorded accurately.

5. 🟢 Suggestion — the listing query keeps node_type = 'collection' while the create path covers subtypes

📁 packages/core/src/db/sqlite_store/relationships.rs:1138-1139, packages/core/src/ops/node_ops.rs:192-197

node_ops resolves the extends chain so a subtype of collection gets the deterministic id, but the context query matches the literal type, so a subtype collection is never listed for the agent. The literal predates this PR and matches the sibling queries (get_collection_by_name, get_all_collections_with_member_counts), so changing only this one would be inconsistent. I am noting it because the sequence lists literal node_type comparisons as a rejection reason and this statement was rewritten here. The subtype claim in node_ops also has no test.

Recommendation: add a test for a subtype through node_ops::create_node, or drop the subtype sentence from the comment and PR description until subtypes of collection are exercised end to end.

6. 🟢 Suggestion — the description budget counts bytes, the constants say characters

📁 packages/core/src/ops/context_ops.rs:141-191

The per-description cap is in characters (leading_chars), but the shared budget uses description.len(), which is bytes and includes the 3-byte …. An 80-character CJK description costs 243 bytes, so about four collections get a description instead of twelve. This matches how the rest of format_for_prompt treats max_chars, so it is consistent; the doc comments on the two constants could say "bytes" for the budget, or the budget could count chars().

7. 🟢 Suggestion — descriptions are user and agent-written text placed in the system prompt

📁 packages/core/src/ops/context_ops.rs:153-161

Collapsing whitespace to one line and capping at 80 characters is the right containment: a description cannot forge a new section header. What remains is 80 characters of free text per collection that an agent can write (via update_node) and that then appears in every later system prompt. That is the same exposure collection names, entity titles and playbook descriptions already have, so it is not a regression. If the context block gains a "this is data" framing later, this line should be inside it.

Nits

• Nit: get_all_collection_descriptions returns names and descriptions as a tuple; a name such as get_all_collection_summaries returning CollectionSummary would remove the mapping in build_workspace_context.
• Nit: node_ops::create_node now runs resolve_type_chain (one query for all extends edges) on every id-less create to answer a question that only matters for collections.
• Nit: the query reads every collection's full description each turn, and nothing caps description length on write; substr in SQL would bound it.
• Nit: the new SKILL.md sentence writes node update <id> without the nodespace prefix the surrounding examples use, and does not say where the id of a collection auto-created by --collection comes from.

Recommendation

REQUEST CHANGES

Fix finding 1 before merge. Findings 2 and 3 are small and belong in the same change, since both concern behaviour this PR documents to agents. Finding 4 is a description correction, and criterion 7 needs either the eval run posted or the criterion amended on the issue.

[malibio]

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

[malibio]

Address-review summary (commit ee96229)

Addressed

1. 🔴 Non-string description panics the context query — reproduced with a test first (libsql unreachable!), then fixed: the query reads the description only when json_type is text. Test: a_non_text_collection_description_is_listed_as_none (number, boolean, object).
2. 🟡 Taken name after a rename — new_collection_id now also checks the name. Test: create_refuses_the_name_of_a_renamed_collection, through both the generic create and create_collection, for the new name and the first name.
3. 🟡 Unactionable error — AlreadyExists now reads collection '<name>' (id <uuid>); cli.md says to use the id from the error.
4. 🟡 PR description — the typed wire struct is now stated as a deferral by choice, not as blocked.
5. 🟢 Subtype claim — test added: a schema extending collection gets the deterministic id and its description lands in the collection bucket.
6. 🟢 Bytes vs characters — the budget constant's doc says it is counted in bytes.

• Nit: the SKILL.md sentence uses the full nodespace node update form.

Skipped

• 7 (prompt exposure): no change requested; same exposure as names and playbook descriptions.
• Nit, return a struct from the listing query: the tuple matches the sibling get_all_collections_with_counts.
• Nit, chain lookup on every id-less create: it is what makes subtypes take the name-derived id.
• Nit, bound description length in SQL: the render already cuts to 80 characters; descriptions are short text.
• The collection listing's literal node_type = 'collection' predates this PR and matches its sibling queries; changing one alone would be inconsistent.

Still open: acceptance criterion 7's eval:decisions run — not run, for the reason in the PR description.

test:changed passes (rust, scripts, skill). Re-review needed: this round changed logic.

[malibio]

Pragmatic Code Review — PR #3464

Review Type: Re-Review (commit ee96229d, diff f775b2d5..HEAD)

Previous Review Summary

• Previous recommendations: 7 findings and 4 nits
• Addressed: 6 findings and 1 nit (finding 2 with two narrow gaps, below)
• Not addressed: finding 7 and 3 nits, each skipped for a reason that holds
• New issues found: 0 critical, 0 important, 4 suggestions, 2 nits

#	Previous finding	Status	How I checked
1	🔴 Non-string description panics the context query	✅ Addressed	The query now returns the value only when json_type is text. I ran the same expression in sqlite3 against a number, real, boolean, object, array, JSON null, a missing bucket, a string bucket and an array bucket: all read as SQL NULL; a string and an empty string read as text. The new test passes, and it would fail without the fix: libsql 0.6.0's FromValue for String hits unreachable! for an integer, and an object would be listed as its JSON text.
2	🟡 A taken name is not refused after a rename	✅ Addressed, two gaps	new_collection_id falls back to a name lookup when the id is free. The test covers both names through both create paths and passes; without the name check the first unwrap_err panics. Gaps are in findings A and B.
3	🟡 Already exists gives the caller nothing to act on	✅ Addressed	The error carries the stored name and the id. The three tests compare the whole string. cli.md tells the caller to use the id from the error.
4	🟡 PR description misstates why the wire type is skipped	✅ Addressed	The description now says other flat types have typed structs and that this is a deferral by choice.
5	🟢 Subtype claim untested	✅ Addressed	generic_create_of_a_collection_subtype_uses_the_deterministic_id asserts the id and the whole properties object (both buckets). Passes.
6	🟢 Budget counts bytes, constants say characters	✅ Addressed	The divisor's doc comment says the share is counted in bytes.
7	🟢 Descriptions are free text in the system prompt	Skipped, reason holds	No change was requested; the exposure matches names and playbook descriptions.
Nit	Return a struct from the listing query	Skipped, reason holds	The tuple matches the sibling query.
Nit	Chain lookup on every id-less create	Skipped, reason holds	The subtype test now depends on it.
Nit	Bound description length in SQL	Skipped, reason holds	The render cuts to 80 characters.
Nit	SKILL.md sentence lacks the nodespace prefix	✅ Addressed	The second half of that nit (where the id of an auto-created collection comes from) is still open; it stays a nit.

Consumers of the changed error text: none break. The daemon maps the variant to Status::already_exists("Already exists: …"); the Tauri commands and the frontend key on the gRPC code (COLLECTION_EXISTS), not the text; the CLI test asserts only Already exists. No other test matches the old text.

Tests I ran, all passing: core lib collection_ops (13), core lib context_ops (57), integration collection_name_convergence_test (11), bun run test:skill (132), and bun run skill:check ("Skill content is up to date").

Requirements Check

#	Acceptance criterion	Status	Notes
6	Skill guidance teaches it; SKILL.md drift check passes	✅ (was not run before)	I ran the drift check and the skill tests this time.
7	Golden regenerated; eval:decisions before and after posted on the issue	❌ Still unmet	The issue has only the baseline comment and the criterion is not amended. The PR gives a reason for not running it. This is a decision for the issue owner: post the run, or amend the criterion.
8	test:changed passes; quality:fix clean	Not verified	The author reports test:changed passing. I ran only the targeted tests above.

Code Review Findings

No critical or important problems. Nothing in the new commit breaks a CLAUDE.md rule: no compatibility code, no issue numbers in comments, no lint suppression, no new literal node_type comparison.

A. 🟢 Suggestion — the name check does not trim, the id does

📁 packages/core/src/ops/collection_ops.rs:323-330, packages/core/src/db/sqlite_store/relationships.rs:1011-1012

deterministic_collection_id normalizes with trim().to_lowercase(). get_collection_by_name only lowercases, and compares against title, which strip_markdown trims. So after renaming Clients to Accounts, a create of " Accounts " passes both checks: the id of accounts is free and " accounts " does not equal the title accounts. The result is the second active Accounts this commit set out to prevent. I found this by reading; I did not run it, because the review may not add a test.

Recommendation: pass name.trim() (or normalize_collection_name(name)) to the lookup in new_collection_id, and add a padded name to the loop in create_refuses_the_name_of_a_renamed_collection.

Principle: two checks that guard one invariant should normalize the input the same way.

B. 🟢 Suggestion — the name check is ASCII-only for case, so it misses a renamed collection with a non-ASCII capital

📁 packages/core/src/db/sqlite_store/relationships.rs:1012-1022

The lookup lowercases the argument in Rust (Unicode) and the column with SQLite's LOWER (ASCII only). In sqlite3, lower('ÉCOLE') = 'école' is false and lower('École') returns École. So a collection renamed to École is not found by name, even for the identical spelling, and a second École can be created. The mismatch predates this PR and also affects rename_collection; the new check inherits it, and cli.md says names are case-insensitive. I checked the SQL in the sqlite3 CLI, not through libsql.

Recommendation: out of scope to fix here. If it is worth closing, compare a normalized name computed in Rust on both sides (a stored normalized column, or fetch candidates and compare in Rust). Otherwise leave it.

C. 🟢 Suggestion — the first name of a renamed collection can never be used again

📁 packages/core/src/ops/collection_ops.rs:311-314

The comment is accurate: after Clients is renamed to Accounts, creating Clients is refused with Already exists: collection 'Accounts' (id …), and the test pins that. The message now explains itself, which is a real improvement on the old Already exists: Clients. It is still a refusal of a name no collection holds, and the only way out is to rename back. That is a consequence of name-derived ids, not of this commit.

Recommendation: accept it for this PR. The cli.md sentence "a name that is taken" is slightly narrower than the behaviour; the error text covers the difference.

D. 🟢 Suggestion — the name check matches only the literal collection type

📁 packages/core/src/db/sqlite_store/relationships.rs:1021

node_ops::create_node covers subtypes of collection for the id, and the new test proves it. The name lookup keeps node_type = 'collection', so a renamed subtype collection (a shelf renamed to Recipes) does not block a new Recipes. Same root as previous finding 5, whose skip reason (the literal predates the PR and matches its siblings) still holds. Noted so the gap is recorded with the subtype test.

Nitpicks

• Nit: packages/core/src/ops/collection_ops.rs:302-314 — the doc comment says "the check is by id, not by get_collection_by_name" and the next paragraph says the name is checked as well. One paragraph that states the order (id first, covering archived holders; then the active name, covering renames) would read straight.
• Nit: packages/core/src/ops/collection_ops.rs:332-334 and 379-381 — AlreadyExists { id } now holds a phrase in one call site and a bare name in rename_collection. Giving the rename refusal the same collection '<name>' (id <uuid>) text would make the two consistent; existing is in scope there.

Recommendation

APPROVE

The blocking finding is fixed and tested, and the rest of the previous review is addressed or skipped for sound reasons. Findings A to D are optional; A is a two-line change if the author wants the rename guarantee to be complete. Acceptance criterion 7 (eval:decisions results on the issue) is still open and needs a decision from the issue owner before merge.

[malibio]

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

[malibio]

Address-review summary, round 2 (commit 56a44bf)

Addressed

• A: new_collection_id trims the name before the lookup, as the id does. The rename test now includes a padded, differently-cased spelling; it fails without the trim.
• Nit: the doc comment states the two checks and their order in one place.
• Nit: rename_collection's refusal uses the same collection '<name>' (id <uuid>) text as the create's.

Skipped

• B (SQLite's ASCII-only LOWER): predates this PR and also affects rename; fixing it means a normalized-name column or a Rust-side comparison, which is its own change.
• C (a renamed collection's first name stays taken): a consequence of name-derived ids; the error now says which collection holds it.
• D (the name lookup's literal collection type): matches its sibling queries, as before.

test:changed passes. No re-review needed: a one-line trim with a test that fails without it, one shared error constructor, and a comment.

Still open: the eval:decisions run for acceptance criterion 7, which will be run and posted on the issue before merge.

[malibio]

Pragmatic Code Review — PR #3464

Review Type: Re-Review (post-rebase), head 7b1fcce8 on main at 27603545

How this was checked

By reading only. A live-model eval is running on this machine, so I ran no cargo, nextest, bun run test* or build. I used git diff origin/main...HEAD, git range-diff 2391251a..56a44bf5 origin/main..56ac77e4, git show 7b1fcce8, file reads, and one sqlite3 in-memory check of the listing SQL. The author reports bun run test:changed passing on 7b1fcce8 (Rust workspace, scripts, skill tiers). I could not re-run it, so that is a claim, not something this review confirms.

The range-diff shows what the rebase changed relative to the approved commits:

• core_schemas.rs: the branch's field declaration and its edit to test_simple_schemas_have_no_fields dropped out, because main already has both.
• relationships.rs: the branch's SELECT list and json_type guard now sit on main's is_a_sql predicate.
• Commit 4 (56a44bf5 to 56ac77e4) is identical.
• 7b1fcce8 adds two things: type_is_a in node_ops.rs, and SchemaFieldType::Text in the schema test.

Previous Review Summary

Items whose status the rebase changed:

• Finding 5 (round 1), listing query used a literal node_type = 'collection': resolved by main. The listing now uses is_a_sql("node_type", &[CoreNodeType::Collection]), so a subtype of collection is listed.
• Finding D (round 2), name lookup matched only the literal type: resolved by main. get_collection_by_name uses the same is_a_sql predicate, so new_collection_id's name check now also finds a renamed subtype collection.
• Nit (round 1), chain lookup on every id-less create: reduced. type_is_a answers from the registry with no database read when the type is a core type. Only a non-core type costs one indexed read of type_ancestry.
• Finding 4 (round 1), PR description on the wire type: stale again, for a new reason. See finding 1 below.
• Finding 1 (round 1), non-text description panics the listing: still fixed. See check 4 below for why the guard is still needed on current main.
• Acceptance criterion 7 (eval:decisions before and after on the issue): still open. Issue Give collections a declared description that the CLI, agent and workspace context use #3421 has only the baseline comment.

Nothing else from the two earlier reviews changed status.

The six questions, answered from the code

1. core_schemas.rs: coherent. Main declares collection.description once (lines 528-560: text, required: Some(false), no default, core protection). The branch's remaining diff in this file is one test, test_collection_declares_only_an_optional_description. There is no duplicate field. Main's test_simple_schemas_have_no_fields no longer lists collection, so the two tests agree. Nothing main added was lost. The branch's longer field description text gave way to main's ("What the collection is for"), and no test pins that text.

2. get_all_collection_descriptions: valid SQL, nothing dropped, no broken caller.

   • The statement expands to SELECT content, CASE WHEN json_type(properties, '$.collection.description') = 'text' THEN json_extract(...) END FROM node WHERE node_type IN (SELECT node_type FROM type_ancestry WHERE ancestor IN ('collection')) ORDER BY content ASC.
   • It reads one table, so the unqualified node_type is unambiguous. Inside the subquery, node_type binds to type_ancestry, which is the shape main uses in get_collection_by_name.
   • I ran the expanded statement in sqlite3 against a hand-built node and type_ancestry. A collection and a shelf subtype returned their text descriptions. A number, a JSON null and a missing key returned SQL NULL.
   • Main's version of this function had only the is_a_sql predicate and the content column, and both are kept.
   • Main has three references to get_all_collection_names (store, service, context_ops.rs:635) plus one service test. The branch renames all of them. git grep finds no remaining reference in either repo, and no second function with the old name.

3. type_is_a in node_ops::create_node: no new error path.

   • A core type resolves through CoreNodeType::from_id with no database read.
   • An unknown type has no ancestry rows, so chain_or_unextended returns the type as its own chain, CoreNodeType::nearest returns None, and type_is_a returns Ok(false). The create then continues to the service's existing validation with id: None, as before, so the caller sees the same error as on main.
   • A user-defined subtype (shelf extends collection) has the chain ["shelf", "collection"]. nearest gives Collection, so it takes the name-derived id.
   • The only error type_is_a can return is a failed database read.

4. Closed core schemas: nothing on the branch writes a rejected key.

   • reject_undeclared_core_keys (crud.rs:2673) checks the top-level keys of each core bucket in the chain. Every write on the branch puts only description in the collection bucket, which main declares and collection owns. The subtype test's room goes to the shelf bucket, which is user-defined and stays open.
   • collection is WireShape::Generic in the registry, so typed_update_fields is empty and the daemon's ensure_no_typed_core_fields lets description through the generic update. Main pins exactly that case in core_type_registry_test.rs:595-601.
   • Main does not type-check text fields on write. check_field_value (crud.rs:2852) validates enum, object, array of objects, number, boolean, date and datetime. text falls to _ => None. So a_non_text_collection_description_is_listed_as_none and the doc comment on get_all_collection_descriptions ("Nothing type-checks a text field on write") are still accurate, and the json_type guard is still needed.

5. Stale or redundant branch code: none in the code; the PR description is stale (finding 1).

   • CollectionSummary is still the only type of its kind in Rust. Main's only CollectionSummary is a local TypeScript interface in an e2e test.
   • Literal "collection" strings added by the branch are test inputs (node_type, extends, bucket assertions), which scripts/check-node-type-literals.ts excludes, plus one bucket key in product code (finding 3). None is a node_type comparison.
   • convert.rs: main gives collection no typed wire struct and no promoted field. It is in the generic(node) arm, and core_promoted_fields returns &[] for it. So the branch's wire test and the viewer's read of collection.properties.description (collection-node-viewer.svelte:259) are both still right.

6. Docs sheet: matches. The collection sheet in components/node-types.md lists description, storage collection.description, string, optional, set from the CLI and the agent, shown after the name in the workspace context. It has no wire-name column, which fits the generic wire shape. Structure, content and participation lines describe rules main enforces and the branch does not touch. No docs change is needed for this PR.

Code Review Findings

No critical problems. No new problem in the conflict resolution. No rule in the node-type sequence's reviewer list is broken: no undeclared key, no literal node_type comparison, no lifecycle_status read, no second wire type, no id list, no non-UUID id, no compatibility code.

1. 🟡 Important — the PR description describes the codebase before the rebase

📁 PR description ("What changed" → Schema; "Notes for review" → "Not in this PR, by choice")

Three statements are no longer true on current main:

• "The CoreNodeType registry, closed-schema rejection and generated TypeScript are not in the codebase yet." The registry (packages/nodespace-types/src/core_type.rs) and closed-schema rejection (reject_undeclared_core_keys) are on main. I found no ts-rs in the workspace, so the generated-TypeScript part still appears to hold.
• "Schema: collection declares description in core_schemas.rs." Main declares it. This PR now adds only the test that pins it.
• The typed CollectionNode struct is described as this PR's deferral. The registry now records collection as WireShape::Generic, and main has a test that its description is written through the generic update. That is main's recorded shape, not a gap this PR leaves.

Recommendation: edit the description before merge. Say that main declares the field and this PR pins it with a test, drop the "not in the codebase yet" sentence, and say that the registry gives collection the generic wire shape.

Principle: the squash-merge message is the durable record of the change, and the node-type sequence asks the PR description to state the type's layers accurately.

Established by reading, not by running code.

2. 🟢 Suggestion — a subtype collection is now listed, and no test says so

📁 packages/core/src/ops/collection_ops.rs:598-630, packages/core/src/db/sqlite_store/relationships.rs:1151-1176

After the rebase the listing covers subtypes, and a subtype's inherited description is in the collection bucket, so $.collection.description reads it. generic_create_of_a_collection_subtype_uses_the_deterministic_id asserts the id and the stored buckets, but not that the shelf appears in get_all_collection_descriptions with its description. That is the behaviour the agent sees, and it changed in this rebase.

Recommendation: add one assertion to that test: the listing equals [("Cookbooks", Some("Recipes we kept"))].

Principle: test the behaviour a change introduces at the surface that consumes it.

Established by reading, plus a sqlite3 run of the same SQL on a hand-built table. Not run through libsql or the test suite.

3. Nit — the bucket key is a string literal in product code

📁 packages/core/src/ops/collection_ops.rs:355

json!({ "collection": { "description": … } }) is a bucket key, not a type comparison, and the literal checker does not flag it. CoreNodeType::Collection.as_str() would tie it to the registry. Optional.

Established by reading.

Outside this PR

• The listing has no lifecycle filter, so an archived collection is still shown in the workspace context. components/participation.md says archived nodes take no part in it. Main's get_all_collection_names behaves the same way and I found no governance module in packages/core yet, so this belongs to the ADR-087 work, not here.
• Round-2 findings B (SQLite's ASCII-only LOWER) and C (a renamed collection's first name stays taken) are unchanged by the rebase.

Recommendation

APPROVE

The conflict resolutions are correct and lose nothing from main, and 7b1fcce8 uses the registry the way main's own code does. Before merge: update the PR description (finding 1), and settle acceptance criterion 7, which is still open on the issue. Finding 2 is a one-assertion addition if the author wants the new subtype behaviour pinned. Test status rests on the author's reported test:changed run, which this review did not repeat.

[malibio]

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

[malibio]

Address-review summary, round 3 (post-rebase, head 68e6f9f)

Addressed

• 1 (PR description stale after the rebase): rewritten. It now says main declares the field and this PR pins it with a test, that the registry gives collection the generic wire shape, and drops the "not in the codebase yet" sentence.
• 2 (subtype listing untested): the subtype test now asserts the shelf collection is listed with its description.

Skipped

• Nit (bucket key as CoreNodeType::Collection.as_str()): a no-op refactor; the literal is a bucket key, not a type comparison, and the literal checker passes.

Rebased once more onto main (two unrelated commits, no conflicts). test:changed passes on the head.

Acceptance criterion 7 (eval:decisions before/after): dropped with the issue owner's agreement; the reason is recorded on #3421.

No re-review needed: one test assertion and a description edit.