[malibio]

Implements ADR-100 §6 for saved queries.

• Deterministic describer (describe_query) renders a query's type, filters, sorting, limit and layout; tests per filter kind and for every seeded query.
• The query takes part in the bound chat: opens on the description, options with "Explain this view" first, pins the new Building and Changing a View skill.
• New seeded skill (procedure lane, so it takes no place from tool skills; confusion matrix re-recorded on the locked model).
• Saving a query refuses a filter or sort on a field no schema declares, listing the fields the type has. Runs and search_nodes are unchanged.
• Query viewer gets an Ask button beside its existing controls.
• Prompt goldens regenerated (new capability name only).

🤖 Generated with Claude Code

[malibio]

Code Review Summary

Verdict: APPROVE (net positive, no blockers). The change is cohesive and follows the established play-chat pattern: a second BoundChatType entry (QUERY) in bound_chat.rs, a deterministic describer in core, a Procedure-lane skill pinned by the chat, a saved-query-only unknown-field refusal, and a button in the viewer. No node_type literal comparisons, no lifecycle_status reads, no raw console output, no compatibility shims, no new wire type or schema field (so the node-type sequence is not triggered).

Acceptance criteria:

• Chat offered from the view, opens on the description with "Explain this view" first: met (mode === 'saved' button; a_saved_query_opens_on_what_it_lists pins the exact opening and option order).
• Description for every filter kind/operator incl. related, relative dates, permitted, and seeded queries: met in coverage; two wording concerns below.
• Filterable-on answer from the graph, writes nothing: skill text covers it (reads schema node); behaviour is model-dependent and not evidenced in the diff.
• Why a node is/isn't in the view: skill text covers it.
• Unknown field refused with the existing fields: met for top-level names (see Improvement 2 on scope).
• Direct controls still work: met (List/Table/Kanban asserted in the viewer test).
• Pinned, not searched: met (skill_id in BoundChatType, Procedure role).
• Prompt dump on the built-in model / eval scenarios: not visible in the diff (goldens regenerated, but no dump or eval scenario added). Please confirm in the PR body that the dump was checked.

Findings

Critical Issues

None.

Suggested Improvements

1. description.rs, path_phrase / filter_clause (readability of seeded queries). The Ready queue reads "that have a blocks where status is set" and "that have no blocked_by where ...", and Needs Shaping "that have no blocks where status is set". The acceptance criterion says the description "reads correctly for each seeded query"; the article plus a raw relationship name is not grammatical, and a person is the reader (the model never sees this sentence). Either map the common names to nouns ("blocker", "blocked item", "plan") and keep the raw name as with a "blocks" relationship, or phrase it as through blocks: "that have no blocked_by node where status is ...". The test fixtures pin these strings, so the fix is a one-place change.
2. query_ops.rs, refuse_unknown scope. A name is refused only when no schema anywhere declares it (declared_anywhere). So priority filtered on a person view, or a sort by another type's field, is accepted and the view is silently empty, which is exactly the failure the criterion targets. The comment explains why subtype fields must not be refused; narrowing to "declared by this type, its ancestors, or its subtypes (or derived)" would keep that and catch the cross-type case. Also the segments.len() >= 2 (path into a structured field) branch is not covered by refuse_unknown; worth either handling or saying so in a comment.
3. Tests, write path. The refusal is tested only by calling resolve_saved_filters / resolve_saved_sorting directly. The behaviour that matters is validate_query_paths on create/update of a query node (the chat's update_node goes through it). Add one test that updating a query with an unknown property is rejected with the field list, and that a nested related filter with an unknown field is too (the flag is threaded through DeclaredFields, but nothing asserts it).
4. DRY in description.rs. filter_clause and condition duplicate the Related and Permitted arms almost verbatim (path/nested/verdict computation). Extract related_phrase(filter, lead) and permitted_phrase(filter) helpers so the two cannot drift.

Nitpicks

• Nit: building-a-view.md lists title among sort fields and operators; confirmed title is in NODE_COLUMNS, so correct. view_config keys match lastView / kanban.groupBy in core_queries.rs.
• Nit: query-node-viewer.svelte passes class="query-ask" but no .query-ask rule exists; confirm in the app that the button matches its neighbours (the viewer test asserts presence, not appearance).
• Nit: skill_retrieval/current.tsv shows "Building and Changing a View" as the leader on the "keeping the calls we make..." row (margin -0.013 to -0.026). Owner and routed leader are unchanged, so fine, but it is worth a line in the PR that Procedure skills still score in the recorded leaders while taking no window place.
• Nit: describe_query_node's fallback text ("Its definition can't be read as it is stored.") hides the decode error; fine for a person, but a log::debug of the error would help diagnose.

Positives

• Reusing the pinned bound-chat mechanism (one table entry) rather than a query-specific path keeps the surface small.
• Scoping the refusal to saved-query writes keeps search_nodes behaviour unchanged, as intended.
• Describer is deterministic, has per-kind tests plus every seeded query as a fixture, and the skill ends with an explicit confirm-before-write and repair-on-rejection step.

[malibio]

Automated pragmatic-code-review: APPROVE (no blockers). Full report in the comment above: four improvements (seeded-query wording for relationship names, unknown-field refusal scope and write-path test, describer DRY) and minor nits.

[malibio]

Review addressed

Addressed

• Related filters read grammatically: "that have no node through blocked_by where status is none of done, cancelled" (fixtures updated).
• Related and permitted phrases are one helper each, shared by the clause and the nested condition.
• Test through the save path: a query node with an undeclared field is refused at the top level, in sorting and inside a related filter, with the fields the type has.

Skipped

• Narrowing the unknown-field check to the type and its relatives: a base-type view of a subtype's field and derived attributes deliberately still run and match nothing (existing, documented behavior); the check targets names no schema declares.
• .query-ask rule and decode-error log: the shared Button styles itself, and the fallback message already says the definition cannot be read.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: Wording, a behavior-preserving helper extraction, and one added test; no logic or behavior change beyond the describer's phrasing.