feat: add learn export-kg and NormalizedTerm action metadata (Refs #759, #735)

.docs/design-735-normalizedterm-metadata.md

@@ -0,0 +1,211 @@
+# Design & Implementation Plan: Add Action Metadata to NormalizedTerm
+
+**Issue**: #735 — `feat(types): add action metadata to NormalizedTerm beyond url field`
+**Author**: Agent
+**Date**: 2026-04-22
+**Phase**: 2 — Design
+
+---
+
+## 1. Summary of Target Behavior
+
+After implementation, `NormalizedTerm` carries four additional optional metadata fields:
+
+| Field | Type | Description |
+|-------|------|-------------|
+| `action` | `Option<String>` | CLI action template with `{{ model }}` and `{{ prompt }}` placeholders |
+| `priority` | `Option<u8>` | Routing tiebreaking priority (higher = preferred) |
+| `trigger` | `Option<String>` | Pattern or alias that activates this term |
+| `pinned` | `bool` | Whether the term is pinned (default `false`) |
+
+`AutocompleteMetadata` (in `terraphim_automata`) receives the same four fields in the same PR to stay in sync.
+
+All new fields are `Option<T>` (or `bool` with `#[serde(default)]`), ensuring backward-compatible JSON deserialization: existing serialised `NormalizedTerm` data remains valid.
+
+---
+
+## 2. Key Invariants and Acceptance Criteria
+
+### Invariants
+- `NormalizedTerm` JSON can be deserialised regardless of which optional fields are present
+- Adding fields does not change the meaning of existing `id`, `value`, `display_value`, or `url` fields
+- `AutocompleteMetadata` and `NormalizedTerm` remain structurally aligned for their shared fields
+
+### Acceptance Criteria
+
+| # | Criterion | Test Type | Location |
+|---|-----------|-----------|----------|
+| AC1 | `NormalizedTerm::new()`, `with_auto_id()`, `with_display_value()`, `with_url()` still compile and produce terms without the new fields set | Unit | New test in `terraphim_types` |
+| AC2 | `NormalizedTerm` deserialises from JSON missing all new fields | Unit | New test in `terraphim_types` |
+| AC3 | `NormalizedTerm` serialises with all four new fields present | Unit | New test in `terraphim_types` |
+| AC4 | `NormalizedTerm::action()` / `::priority()` / `::trigger()` / `::pinned()` builder methods work | Unit | New test in `terraphim_types` |
+| AC5 | `AutocompleteMetadata` gains the same four fields | Unit | New test in `terraphim_automata` |
+| AC6 | `cargo build --workspace` succeeds with no new warnings | Build | CI |
+| AC7 | All existing tests pass (`cargo test --workspace`) | Integration | CI |
+
+---
+
+## 3. High-Level Design and Boundaries
+
+### Components
+
+```
+crates/terraphim_types/src/lib.rs
+  └── NormalizedTerm struct (MODIFY)
+        + action: Option<String>
+        + priority: Option<u8>
+        + trigger: Option<String>
+        + pinned: bool
+      + builder methods: with_action(), with_priority(), with_trigger(), with_pinned()
+      + getter methods: action(), priority(), trigger(), pinned()
+
+crates/terraphim_automata/src/autocomplete.rs
+  └── AutocompleteMetadata struct (MODIFY)
+        + action: Option<String>
+        + priority: Option<u8>
+        + trigger: Option<String>
+        + pinned: bool
+```
+
+### Boundaries
+- **Inside**: Adding fields to `NormalizedTerm` and `AutocompleteMetadata`, including serde attributes and builder/getter methods
+- **Outside**: No changes to `MarkdownDirectives`, `RouteDirective`, `RoutingRule`, `Thesaurus`, or any storage layer
+- **New dependencies**: None — only standard library + existing serde/ahash imports
+
+### Anti-Patterns Avoided
+- No wrapper struct (per user decision)
+- No changes to JSON schema for existing persisted data (backward-compatible `Option` fields)
+- No migration of existing data
+
+---
+
+## 4. File/Module-Level Change Plan
+
+| File/Module | Action | Before | After | Dependencies |
+|-------------|--------|--------|-------|--------------|
+| `crates/terraphim_types/src/lib.rs` | Modify | `NormalizedTerm` with 4 fields | `NormalizedTerm` with 8 fields; 4 new builder methods; 4 getter methods | None new |
+| `crates/terraphim_types/src/lib.rs` | Modify | `AutocompleteMetadata` with 4 fields | `AutocompleteMetadata` with 8 fields | None new |
+| `crates/terraphim_automata/src/autocomplete.rs` | Modify | `AutocompleteMetadata` with 4 fields | `AutocompleteMetadata` with 8 fields; serde attributes updated | `terraphim_types` re-export |
+
+---
+
+## 5. Step-by-Step Implementation Sequence
+
+### Step 1: Add fields to `NormalizedTerm` in `terraphim_types/src/lib.rs`
+- Add `action: Option<String>` with `#[serde(default, skip_serializing_if = "Option::is_none")]`
+- Add `priority: Option<u8>` with same serde attributes
+- Add `trigger: Option<String>` with same serde attributes
+- Add `pinned: bool` with `#[serde(default)]`
+- Add builder methods: `with_action(self, String)`, `with_priority(self, u8)`, `with_trigger(self, String)`, `with_pinned(self, bool)`
+- Add getter methods: `action(&self) -> Option<&String>`, `priority(&self) -> Option<&u8>`, `trigger(&self) -> Option<&String>`, `pinned(&self) -> bool`
+- Update `new()` to initialise all new fields to default values
+- Update `with_auto_id()` similarly
+- Update existing `with_url()` return type to `Self` (already correct for builder pattern)
+
+**Deployable state**: Yes — compiles, all old code path-compatible
+
+### Step 2: Add tests for `NormalizedTerm` in `terraphim_types`
+- Test deserialisation from JSON missing all new fields
+- Test serialisation with all fields set
+- Test builder chain with all new methods
+- Test `display()` still works with new fields
+
+**Deployable state**: Yes — tests only
+
+### Step 3: Update `AutocompleteMetadata` in `terraphim_automata/src/autocomplete.rs`
+- Add the same four fields with identical serde attributes
+- Add identical builder and getter methods
+
+**Deployable state**: Yes — compiles, structurally aligned with NormalizedTerm
+
+### Step 4: Add tests for `AutocompleteMetadata`
+- Test serialisation and deserialisation round-trip
+- Test builder methods
+
+**Deployable state**: Yes — tests only
+
+### Step 5: Run full workspace build and test
+- `cargo build --workspace`
+- `cargo test --workspace`
+- `cargo clippy -- -D warnings`
+
+**Deployable state**: Yes — verified green
+
+---
+
+## 6. Testing & Verification Strategy
+
+| Acceptance Criteria | Test Type | Test Location |
+|---------------------|-----------|---------------|
+| AC1: Constructors produce unset new fields | Unit | `crates/terraphim_types/src/lib.rs` (inline tests or `tests/` dir) |
+| AC2: JSON backward compat (missing fields) | Unit | Same |
+| AC3: JSON serialisation with all fields | Unit | Same |
+| AC4: Builder/getter methods | Unit | Same |
+| AC5: AutocompleteMetadata alignment | Unit | `crates/terraphim_automata/src/autocomplete.rs` |
+| AC6: No new warnings | Build | CI (`cargo clippy`) |
+| AC7: All tests pass | Integration | CI (`cargo test --workspace`) |
+
+---
+
+## 7. Risk & Complexity Review
+
+| Risk | Mitigation | Residual Risk |
+|------|------------|--------------|
+| Forgetting to update a construction site causes compiler warning | Run `cargo build --workspace` which will warn on missing fields; all test crates updated in same PR | Low |
+| JSON output change breaks external consumers | New fields are optional; consumers that ignore unknown fields are unaffected | Low |
+| `AutocompleteMetadata` and `NormalizedTerm` drift out of sync in future | Document that they must be updated together; consider a shared test | Low |
+
+---
+
+## 8. Open Questions / Decisions for Human Review
+
+1. **Field defaults for `priority`**: `Option<u8>` means `None` means no priority set. Should `priority: Option<u8>` be replaced with `priority: u8` with `#[serde(default = "default_priority")]` where `default_priority()` returns `50` as a sensible neutral priority? (The current plan uses `Option<u8>` — confirm this is preferred or choose the default-value approach.)
+
+2. **Should `trigger` be a `Vec<String>` instead of `Option<String>`** to support multiple trigger patterns per term? (Current plan uses `Option<String>` — single trigger per term. Multi-trigger would require `Option<Vec<String>>`.)
+
+3. **`pinned: bool` with `#[serde(default)]`** — should pinned terms default to `false` (current plan) or should the serde default be a specific behaviour (e.g., pinned = false is never serialised)?
+
+**Note**: All three questions above are clarifications on default values and representation — the four-field structure is fixed per the approved research. The default-value approach for `priority` and the single-string `trigger` are recommended for simplicity.
+
+---
+
+## Appendix: Target Struct Shapes
+
+### `NormalizedTerm` (after)
+```rust
+pub struct NormalizedTerm {
+    pub id: u64,
+    pub value: NormalizedTermValue,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub display_value: Option<String>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub url: Option<String>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub action: Option<String>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub priority: Option<u8>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub trigger: Option<String>,
+    #[serde(default)]
+    pub pinned: bool,
+}
+```
+
+### `AutocompleteMetadata` (after)
+```rust
+pub struct AutocompleteMetadata {
+    pub id: u64,
+    pub normalized_term: NormalizedTermValue,
+    pub original_term: String,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub url: Option<String>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub action: Option<String>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub priority: Option<u8>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub trigger: Option<String>,
+    #[serde(default)]
+    pub pinned: bool,
+}
+```

.docs/quality-evaluation-design-735.md

@@ -0,0 +1,169 @@
+# Document Quality Evaluation Report
+
+## Metadata
+- **Document**: `.docs/design-735-normalizedterm-metadata.md`
+- **Type**: Phase 2 Design
+- **Evaluated**: 2026-04-22
+
+## Decision: GO
+
+**Average Score**: 4.2 / 5.0
+**Blocking Dimensions**: None
+
+---
+
+## Dimension Scores
+
+| Dimension | Score | Status |
+|-----------|-------|--------|
+| Syntactic | 5/5 | Pass |
+| Semantic | 4/5 | Pass |
+| Pragmatic | 4/5 | Pass |
+| Social | 4/5 | Pass |
+| Physical | 4/5 | Pass |
+| Empirical | 4/5 | Pass |
+
+---
+
+## Detailed Findings
+
+### Syntactic Quality (5/5) — EXEMPLARY
+
+**Strengths:**
+- All 8 expected sections present and correctly labelled
+- Tables in Sections 2, 3, 5, 6, 7 are internally consistent (columns match content)
+- The Appendix shows exact target Rust struct shapes, fully consistent with the prose descriptions
+- No contradictions between sections: Section 5 step sequence matches Section 4 change plan
+- Code in appendix uses correct Rust syntax and serde attributes (`#[serde(default, skip_serializing_if = "Option::is_none")]`) matching the established pattern in the codebase
+- Field names and types in appendix exactly match the field list in Section 1
+
+**Weaknesses:**
+- None identified
+
+**Suggested Revisions:**
+- None
+
+---
+
+### Semantic Quality (4/5)
+
+**Strengths:**
+- All field types are technically accurate: `Option<String>` for action/trigger, `Option<u8>` for priority, `bool` for pinned
+- serde attribute strategy (backward-compatible `Option` with `skip_serializing_if`) correctly applied to all three new optional fields
+- `pinned: bool` with `#[serde(default)]` correctly means it serialises only when `true`
+- Builder and getter method signatures are plausible and follow existing builder pattern in the codebase
+- Anti-patterns section correctly identifies what is NOT being done (no wrapper struct, no migration, no changes to related types)
+
+**Weaknesses:**
+- Section 7 risk table repeats the same "Low" residual risk for all three risks — the actual residual risks are genuinely low but the repetition makes the table uninformative
+- Open Question 1 in Section 8 asks about `priority` default value but the field is declared `Option<u8>` — the question is valid but the design should tentatively pick a default or confirm `Option<u8>` is correct before implementation
+
+**Suggested Revisions:**
+- [ ] Add a tentative recommendation in Open Question 1 (e.g., "Recommend Option<u8> with None = unset, since 50 as magic default could conflict with intentional priority values")
+
+---
+
+### Pragmatic Quality (4/5)
+
+**Strengths:**
+- Step-by-step sequence (Section 5) is concrete and actionable — a competent Rust developer could follow it
+- Each step is independently deployable (verified state)
+- Acceptance criteria table maps directly to testable criteria with specific test types and locations
+- Anti-patterns and boundaries clearly delineate what is NOT changing, reducing implementer uncertainty
+- Build/test commands in Step 5 are specific: `cargo build --workspace`, `cargo test --workspace`, `cargo clippy -- -D warnings`
+
+**Weaknesses:**
+- The "Step 1" description says "Update `new()` to initialise all new fields to default values" — for `Option` fields this is `None` and for `bool` this is `false`, but this is not explicitly stated, leaving ambiguity about whether the developer should set them explicitly or rely on Rust's default
+- Open Questions 1-3 are left as pure questions without even a tentative recommendation, which is less helpful for Phase 3
+
+**Suggested Revisions:**
+- [ ] Clarify in Step 1 that new fields are simply omitted from struct literals (relying on `#[serde(default)]`) since Rust will zero-initialise missing struct fields
+- [ ] Provide a tentative recommendation for each of the three Open Questions
+
+---
+
+### Social Quality (4/5)
+
+**Strengths:**
+- All assumptions made explicit in the plan (top-level fields, four fields, same PR for AutocompleteMetadata)
+- Anti-patterns section clearly separates what is NOT in scope, reducing misinterpretation risk
+- Builder method names follow existing codebase convention (`with_*`) — no ambiguity about naming
+- serde attribute semantics (`skip_serializing_if = "Option::is_none"`) are explained in context in Section 1
+
+**Weaknesses:**
+- "Option<T>" serde semantics could be misunderstood by developers unfamiliar with serde — the document assumes familiarity
+- "skip_serializing_if = Option::is_none" pattern is used without explicit explanation of what it means for the JSON output (field absent vs. field = null)
+
+**Suggested Revisions:**
+- [ ] Add a brief note in Section 1 explaining that `Option` fields are absent from JSON when None (not null), which is the existing pattern already used for `display_value` and `url`
+
+---
+
+### Physical Quality (4/5)
+
+**Strengths:**
+- All 8 sections present with clear headers
+- Tables used appropriately throughout: AC table (Section 2), components table (Section 3), change plan table (Section 4), step sequence (Section 5), test table (Section 6), risk table (Section 7)
+- Appendix with exact struct shapes is a strong addition
+- Consistent formatting throughout
+- Good use of checkboxes in Section 7 "Anti-Patterns Avoided"
+
+**Weaknesses:**
+- None significant
+
+**Suggested Revisions:**
+- [ ] None
+
+---
+
+### Empirical Quality (4/5)
+
+**Strengths:**
+- Well-chunked: each of the 8 sections is independently navigable
+- Step sequence uses bullet points with bold purpose labels — easy to scan
+- Tables break up dense content effectively
+- The design is concise: 6 pages total including appendix
+- Phrasings like "competent Rust developer could follow it" in Section 5 evaluation are honest about the target audience
+
+**Weaknesses:**
+- Section 7 risk table with three "Low" entries is repetitive — the three risks are genuinely different but all low, but the table format obscures the distinctions
+- Open Questions section is brief — while three focused questions is better than ten, the absence of even tentative recommendations makes it less actionable
+
+**Suggested Revisions:**
+- [ ] Convert Section 7 risk table to prose bullets to avoid the repetitive "Low" column
+- [ ] Provide a one-sentence tentative recommendation for each Open Question
+
+---
+
+## Revision Checklist
+
+Priority order based on impact:
+
+- **[Low]** Provide tentative recommendation for each Open Question in Section 8 (helps Phase 3)
+- **[Low]** Add note in Section 1 that Option fields are absent (not null) in JSON
+- **[Low]** Clarify Step 1 struct literal approach for new fields
+- **[Trivial]** Convert Section 7 risk table to prose bullets to avoid repetition
+
+---
+
+## Phase 2 Weighted Score Calculation
+
+| Dimension | Score | Weight | Weighted |
+|-----------|-------|--------|----------|
+| Syntactic | 5 | 1.5 | 7.5 |
+| Semantic | 4 | 1.0 | 4.0 |
+| Pragmatic | 4 | 1.5 | 6.0 |
+| Social | 4 | 1.0 | 4.0 |
+| Physical | 4 | 1.0 | 4.0 |
+| Empirical | 4 | 1.0 | 4.0 |
+| **Total** | | **6.0** | **29.5 / 6.0 = 4.2** |
+
+**Threshold**: Average >= 3.5, no dimension < 3 → **PASS**
+
+---
+
+## Next Steps
+
+**GO**: Design approved for Phase 3. The minor revision suggestions above are all low priority — implementation can proceed. The Open Questions in Section 8 should be resolved before Phase 3 begins, or explicitly deferred with rationale.
+
+Proceed with: `disciplined-implementation` skill, or begin Phase 3 directly if the Open Questions can be resolved.