Skip to content

Commit dff8f1b

Browse files
authored
refactor(memory): rename synthesis Skill dataclass to DistilledProcedure (#2684)
Two different frozen dataclasses were both named `Skill` and both reachable from `gaia.agents.base`: the synthesis pipeline's four-field LLM-output schema and the on-disk agentskills.io file contract landing in `src/gaia/skills/` (#2669). The split is a deliberate security boundary — the model may only derive `name`/`when_to_use`/`body`/`tools_required`, never `license`, `version`, or a security tier — so both classes stay; the synthesis one becomes `DistilledProcedure`, matching its own docstring and the `procedures` corpus it feeds. Anyone importing both no longer needs an alias, and nobody can pass the wrong one. Also adds the test `to_skill_md`'s docstring has been asserting all along: the rendered document is now round-tripped through the real `gaia.skills.format` parser + validator instead of a hand-rolled YAML check. Verified against #2669's branch — **it validates, no drift** (`tools_required` is a first-class `metadata.gaia` field there). The test skips until #2669 merges. Part of #2671. ## Test plan - [ ] `python -m pytest tests/unit/test_skill_synthesis.py tests/unit/test_memory_mixin.py tests/unit/test_chat_dynamic_tools.py -q` — 310+ pass, the round-trip test skips while `gaia.skills` is unmerged - [ ] After #2669 merges, rebase and confirm `test_renders_a_document_the_format_validator_accepts` runs and passes rather than skipping - [ ] `python -m pytest tests/unit/ -q` — no new failures vs. `origin/main` (verified: identical failure set, all pre-existing environment issues) - [ ] `python util/lint.py --all` clean - [ ] `grep -rn '\bSkill\b' src/gaia/agents/base/` returns only prose section headers, no class references
1 parent d2832fd commit dff8f1b

8 files changed

Lines changed: 117 additions & 78 deletions

File tree

docs/plans/skill-synthesis.mdx

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ new model, no new store engine.
103103
of ≥ MIN_OCCURRENCES
104104
105105
106-
3. DISTILL distill_cluster(cluster, llm) → Skill | None PROPOSED
106+
3. DISTILL distill_cluster(cluster, llm) → DistilledProcedure|None PROPOSED
107107
one low-temp LLM call per cluster (DISTILL_SYSTEM_PROMPT);
108-
Skill.parse() validates; literal "SKIP" → None
108+
DistilledProcedure.parse() validates; literal "SKIP" → None
109109
110110
111111
4. STORE reconcile_and_store(candidate, store) PROPOSED
@@ -242,20 +242,20 @@ The pipeline *emits* a `SKILL.md`; it does **not** define the format. The format
242242
**references** that schema and never redefines a field.
243243

244244
The subtlety — and the single most important correctness point — is the split
245-
between **what the LLM emits** and **what `Skill.parse()` injects**. The guiding
245+
between **what the LLM emits** and **what `DistilledProcedure.parse()` injects**. The guiding
246246
rule: **the LLM emits only the fields it *derives* from the learned procedure;
247247
fixed constants are injected, never emitted.** So `DISTILL_SYSTEM_PROMPT` asks the
248248
model for an **intermediate** frontmatter of four learned fields, and a
249-
deterministic `Skill.parse()` maps them to the canonical #691 document and adds the
249+
deterministic `DistilledProcedure.parse()` maps them to the canonical #691 document and adds the
250250
two constants:
251251

252252
| Field | Origin | → Canonical `SKILL.md` (#691) | Why |
253253
|---|---|---|---|
254254
| `name` | **LLM (derived)** | `name` | Identity, derived from the cluster. Matches the rule at [`skill-format.mdx:150`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx). |
255255
| `when_to_use` | **LLM (derived)** | `description` | **`when_to_use` is the LLM-emitted field on purpose** — it forces the model to *conclude the trigger boundaries* of the skill; `description` is too vague a label to constrain it. Its embedding is what `recall_skill(goal)` matches against. Maps to the required `description` ([`skill-format.mdx:151`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx)). |
256256
| `tools_required` (top-level) | **LLM (derived)** | `metadata.gaia.tools_required` | The tool set the procedure actually used. #691 **locks** `tools_required` as the cross-spec recipe contract and nests it under `metadata.gaia` ([`skill-format.mdx:143`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx), `:158`). |
257-
| `markdown_body` | **LLM (derived)** | body | The LLM writes the **full procedure — numbered steps *and* a `## Edge cases` section — inline**. `Skill.parse()` does not synthesize a separate edge-case section; the distilled body already carries it. |
258-
| `license: MIT`, `version: 1.0.0` | **`Skill.parse()` (fixed)** | `license`, `version` | **Fixed constants, not learned content — the LLM does *not* emit them.** `license` is always the repository license ([`skill-format.mdx:152`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx)); a freshly synthesised skill is always `version: 1.0.0` (SemVer — [`skill-format.mdx:153`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx)). Injecting them keeps the prompt's output surface purely dynamic and removes any chance the model emits a wrong version. |
257+
| `markdown_body` | **LLM (derived)** | body | The LLM writes the **full procedure — numbered steps *and* a `## Edge cases` section — inline**. `DistilledProcedure.parse()` does not synthesize a separate edge-case section; the distilled body already carries it. |
258+
| `license: MIT`, `version: 1.0.0` | **`DistilledProcedure.parse()` (fixed)** | `license`, `version` | **Fixed constants, not learned content — the LLM does *not* emit them.** `license` is always the repository license ([`skill-format.mdx:152`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx)); a freshly synthesised skill is always `version: 1.0.0` (SemVer — [`skill-format.mdx:153`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx)). Injecting them keeps the prompt's output surface purely dynamic and removes any chance the model emits a wrong version. |
259259

260260
A synthesised recipe therefore emits a **bounded field set**`name`,
261261
`description`, `license`, `version`, `metadata.gaia.tools_required`, and the body.
@@ -269,10 +269,10 @@ most-restrictive `experimental` at load time.
269269
This is the **one deliberate refinement** of the maintainer's pseudocode: his
270270
`DISTILL_SYSTEM_PROMPT` listed `version: 1.0.0` *inside* the LLM's required output
271271
([#887 technical-spec comment](https://github.com/amd/gaia/issues/887#issuecomment-4321407541));
272-
this spec moves `version` (and `license`) to `Skill.parse()` injection because both
272+
this spec moves `version` (and `license`) to `DistilledProcedure.parse()` injection because both
273273
are fixed, not derived. The pipeline otherwise keeps the maintainer's prompt as
274274
written, and **(a)** treats the four learned fields as the intermediate shape,
275-
**(b)** specifies `Skill.parse()` as the translator to the locked schema, and
275+
**(b)** specifies `DistilledProcedure.parse()` as the translator to the locked schema, and
276276
**(c)** guarantees the emitted file validates under #691 — an explicit acceptance
277277
criterion (see [Acceptance criteria traceability](#acceptance-criteria-traceability)).
278278

@@ -448,7 +448,7 @@ current code; these are the corrections an implementer must internalize.
448448

449449
| Original framing | Reality on `main` | Consequence |
450450
|---|---|---|
451-
| `DISTILL_SYSTEM_PROMPT` emits `when_to_use` + top-level `tools_required` as the on-disk shape | #691 locks `description` + `metadata.gaia.tools_required` ([`skill-format.mdx:151`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx), `:158`) | The prompt fields are the **intermediate** shape; `Skill.parse()` maps them. See [The format contract](#the-format-contract). |
451+
| `DISTILL_SYSTEM_PROMPT` emits `when_to_use` + top-level `tools_required` as the on-disk shape | #691 locks `description` + `metadata.gaia.tools_required` ([`skill-format.mdx:151`](https://github.com/amd/gaia/blob/main/docs/plans/skill-format.mdx), `:158`) | The prompt fields are the **intermediate** shape; `DistilledProcedure.parse()` maps them. See [The format contract](#the-format-contract). |
452452
| Scope C: *"New **tool**: `recall_skill`"* | The memory registry is exactly five tools ([`memory.py:1984`](https://github.com/amd/gaia/blob/main/src/gaia/agents/base/memory.py)+) and the planner calls recall programmatically | `recall_skill` is an **internal method**, not a sixth `@tool`. See [Decided design](#decided-design). |
453453
| Hook is `on_consolidation_pass``_consolidate_old_sessions / _reconcile_contradictions / _prune_stale` | Those names don't exist; the real pass is `_run_memory_post_init``reconcile_memory` + `consolidate_old_sessions` ([`memory.py:1206`](https://github.com/amd/gaia/blob/main/src/gaia/agents/base/memory.py)) | Bind `_synthesize_skills` to the **real** maintenance method. |
454454
| Tunables in `~/.gaia/config.toml [memory.skill_synthesis]` | Memory is configured via `~/.gaia/memory_settings.json` ([`memory.py:57`](https://github.com/amd/gaia/blob/main/src/gaia/agents/base/memory.py)) + `GAIA_MEMORY_DISABLED`; there is **no** `config.toml` `[memory]` section | Surface the thresholds on the **existing** memory-settings surface, not a new TOML section — see [Open questions](#open-questions). |
@@ -460,7 +460,7 @@ current code; these are the corrections an implementer must internalize.
460460
- **The intermediate frontmatter is not the on-disk schema.** A reader of the
461461
maintainer's `DISTILL_SYSTEM_PROMPT` must not write `when_to_use` /
462462
top-level `tools_required` to disk. The on-disk file is the #691 schema
463-
(`description`, `metadata.gaia.tools_required`); `Skill.parse()` is the only
463+
(`description`, `metadata.gaia.tools_required`); `DistilledProcedure.parse()` is the only
464464
bridge.
465465
- **`recall_skill` is not an LLM tool.** Do not register a sixth memory `@tool`;
466466
it would change the five-tool registry the rest of the system assumes.
@@ -510,7 +510,7 @@ mirroring the v1→v2 `ADD COLUMN` pattern
510510
### Phase 1 — Detect → cluster → distill → reconcile/store (PROPOSED)
511511

512512
`skill_synthesis.py` with `extract_sequences` / `cluster_by_goal` /
513-
`distill_cluster` / `Skill.parse` / `reconcile_and_store`, hooked into the
513+
`distill_cluster` / `DistilledProcedure.parse` / `reconcile_and_store`, hooked into the
514514
maintenance pass as `_synthesize_skills` after `consolidate_old_sessions`
515515
([`memory.py:1233`](https://github.com/amd/gaia/blob/main/src/gaia/agents/base/memory.py)).
516516
Reuses the nomic-768 embedder ([`memory.py:148`](https://github.com/amd/gaia/blob/main/src/gaia/agents/base/memory.py)).
@@ -569,9 +569,9 @@ The recipe reuses `triage-support-ticket`
569569
so its `tools_required` resolve to real registry tools.
570570

571571
<AccordionGroup>
572-
<Accordion title="A. The synthesised SKILL.md (Skill.parse output — the on-disk #691 document)">
572+
<Accordion title="A. The synthesised SKILL.md (DistilledProcedure.parse output — the on-disk #691 document)">
573573

574-
What `Skill.parse()` writes after distillation — a valid #691 document. `name`,
574+
What `DistilledProcedure.parse()` writes after distillation — a valid #691 document. `name`,
575575
`description`, `license`, `version`, and `metadata.gaia.tools_required` are all the
576576
locked schema. Each `tools_required` name is a real registry tool (`query_documents`
577577
from `RAGToolsMixin`, `read_file` from `FileIOToolsMixin`, `remember` from the
@@ -606,12 +606,12 @@ metadata:
606606
```
607607
608608
</Accordion>
609-
<Accordion title="B. The intermediate DISTILL_SYSTEM_PROMPT output (before Skill.parse)">
609+
<Accordion title="B. The intermediate DISTILL_SYSTEM_PROMPT output (before DistilledProcedure.parse)">
610610
611611
What the LLM emits — **only the four learned fields**: top-level `name`,
612612
`when_to_use`, `tools_required`, and a body that already contains the
613613
`## Edge cases` section. No `version`, no `license` — those are fixed constants.
614-
`Skill.parse()` maps `when_to_use → description`, `tools_required →
614+
`DistilledProcedure.parse()` maps `when_to_use → description`, `tools_required →
615615
metadata.gaia.tools_required`, and **injects the fixed `license: MIT` and
616616
`version: 1.0.0`** to produce example A.
617617

@@ -672,7 +672,7 @@ tool-loader's `CORE ∪ SKILL ∪ SEMANTIC` union ahead of semantic results.
672672
# PROPOSED — at turn start, before tool selection (internal method, not an @tool)
673673
matches = self.recall_skill(user_goal, top_k=2)
674674
# matches → [
675-
# Skill(
675+
# DistilledProcedure(
676676
# name="triage-support-ticket",
677677
# when_to_use="Triage an inbound support ticket end to end. ...",
678678
# body="# Triage a Support Ticket\n1. ...\n## Edge cases\n- ...",
@@ -746,7 +746,7 @@ The memory layer it builds on is real and merged; the synthesis pipeline is
746746
| Prompt composition | `_compose_system_prompt`, `_select_tools_for_turn` | `agent.py:591`, `:786` | **Exists** — injection seam |
747747
| Memory REST + dashboard | `/api/memory/*`, `X-Gaia-UI` guard, three-tab dashboard | `ui/routers/memory.py:26`, `:29`; `MemoryDashboard.tsx:267` | **Exists** — exposes procedural data |
748748
| `category='skill'` (a fact, not a procedure) | `VALID_CATEGORIES` includes `"skill"` | `memory_store.py:97` | **Exists***different concept* |
749-
| **`skill_synthesis.py` / `procedures` table / `extract_sequences` / `cluster_by_goal` / `distill_cluster` / `Skill.parse` / `SkillProvenance` / `reconcile_and_store` / `recall_skill` / `_synthesize_skills` / `put_skill` / `search_skills` / `supersede_skill` / `iter_sessions`** || **NOT FOUND** (verified absent on `main`) | **Greenfield — PROPOSED** |
749+
| **`skill_synthesis.py` / `procedures` table / `extract_sequences` / `cluster_by_goal` / `distill_cluster` / `DistilledProcedure.parse` / `SkillProvenance` / `reconcile_and_store` / `recall_skill` / `_synthesize_skills` / `put_skill` / `search_skills` / `supersede_skill` / `iter_sessions`** || **NOT FOUND** (verified absent on `main`) | **Greenfield — PROPOSED** |
750750

751751
---
752752

@@ -779,7 +779,7 @@ guarantees a consumer. Deferrals are explicit, with the track that owns them.
779779
| **Scope E** — Dashboard "Procedures" tab + provenance + promote | [Phase 2 deferral](#phased-build) | **Tab build DEFERRED to #606's dashboard track**; #887 stores provenance + exposes the data. |
780780
| **AC** — after 3 successful similar sequences, auto-create a `SKILL.md` discoverable via `recall_skill()` | [KPIs](#kpis), [Phase 1](#phased-build) + [Phase 2](#phased-build) | The headline KPI. |
781781
| **AC** — recall measurably reduces tool-step count on the 4th attempt | [KPIs](#kpis), [Phase 2](#phased-build) | Measured vs the tool-loader baseline. |
782-
| **AC** — synthesised skills are valid agentskills.io documents (Hermes parser) | [The format contract](#the-format-contract), [Examples A–B](#examples) | `Skill.parse()` round-trip; 100% validity KPI. |
782+
| **AC** — synthesised skills are valid agentskills.io documents (Hermes parser) | [The format contract](#the-format-contract), [Examples A–B](#examples) | `DistilledProcedure.parse()` round-trip; 100% validity KPI. |
783783
| **AC** — Dashboard shows count / last-used / provenance | [Phase 2 deferral](#phased-build) | **Data exposed by #887**; *rendering* is #606's tab. |
784784
| **AC** — disabling a skill prevents recall | [Off-states](#off-states-as-safe-floors), [Phase 2](#phased-build) | The recall path honours `enabled = 0` (this **is** #887); the toggle UI is #606's tab. |
785785
| **Out of scope** — marketplace / tiers / OpenClaw shim | [Dependencies](#dependencies) |#647 / #692. |

docs/plans/tool-loader.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ stored procedure declares the exact `tools_required` for that recipe.
431431

432432
#### How Part 3 shipped (implementation reference)
433433

434-
**The loader stays memory-agnostic.** `recall_skill` and `Skill` are *not*
434+
**The loader stays memory-agnostic.** `recall_skill` and `DistilledProcedure` are *not*
435435
imported into `tool_loader.py` (that would be an upward dependency). Instead
436436
`ChatAgent` — the composition layer — flattens the recalled procedures'
437437
`tools_required` into a `List[str]` and passes it to `select()` via a new
@@ -442,7 +442,7 @@ never learns what a skill is.
442442
cost).** `recall_skill` already runs once per turn in
443443
`MemoryMixin._refresh_recalled_skills` (before tool selection, via the
444444
`process_query``super()` ordering). Part 3 refactors that method to cache the
445-
matched `Skill` objects in `self._recalled_skills` alongside the rendered
445+
matched `DistilledProcedure` objects in `self._recalled_skills` alongside the rendered
446446
prompt; `_recalled_skill_tools()` reads that cache. So the SKILL signal adds
447447
**no** second embed/FAISS call — TTFT is the whole point of the loader.
448448

src/gaia/agents/base/memory.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,9 +483,10 @@ def init_memory(
483483
# the composed system prompt. Empty string = no recall = the system
484484
# prompt stays byte-identical to a build without procedural memory.
485485
self._recalled_skill_prompt = ""
486-
# The matched Skill objects from the same per-turn recall (#1451): the
487-
# tool loader reads their tools_required via _recalled_skill_tools as the
488-
# SKILL signal. Empty list = no recall = no SKILL signal this turn.
486+
# The matched DistilledProcedure objects from the same per-turn recall
487+
# (#1451): the tool loader reads their tools_required via
488+
# _recalled_skill_tools as the SKILL signal. Empty list = no recall =
489+
# no SKILL signal this turn.
489490
self._recalled_skills = []
490491

491492
# Step 2: Validate Lemonade embedding service connectivity.

src/gaia/agents/base/procedural_memory.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import numpy as np
2121

2222
from gaia.agents.base.skill_synthesis import (
23-
Skill,
23+
DistilledProcedure,
2424
SynthesisConfig,
2525
cluster_by_goal,
2626
distill_cluster,
@@ -180,7 +180,7 @@ def _proc_faiss_search(self, query_vec: np.ndarray, top_k: int) -> List[tuple]:
180180

181181
def recall_skill(
182182
self, goal: str, top_k: int = 2, similarity_tau: Optional[float] = None
183-
) -> List[Skill]:
183+
) -> List[DistilledProcedure]:
184184
"""Recall stored procedures whose trigger matches ``goal`` (vector search).
185185
186186
The RECALL half of the procedural loop and the consumer the tool-loader
@@ -215,8 +215,9 @@ def recall_skill(
215215
so a recalling turn reads the settings file only once.
216216
217217
Returns:
218-
Matched ``Skill`` objects (full bodies; injection truncates, the row
219-
keeps the full body), best match first; ``[]`` on any off-state.
218+
Matched ``DistilledProcedure`` objects (full bodies; injection
219+
truncates, the row keeps the full body), best match first; ``[]``
220+
on any off-state.
220221
"""
221222
from gaia.agents.base.memory import (
222223
_load_memory_settings, # deferred (cycle break)
@@ -250,7 +251,7 @@ def recall_skill(
250251
if similarity_tau is not None
251252
else load_synthesis_config(_load_memory_settings()).similarity_tau
252253
)
253-
skills: List[Skill] = []
254+
skills: List[DistilledProcedure] = []
254255
recalled_ids: List[str] = []
255256
for procedure_id, score in matches:
256257
if score < tau:
@@ -267,7 +268,7 @@ def recall_skill(
267268
continue
268269
row = rows[0]
269270
skills.append(
270-
Skill(
271+
DistilledProcedure(
271272
name=row["name"],
272273
when_to_use=row["when_to_use"],
273274
body=row["markdown_body"],
@@ -291,7 +292,7 @@ def recall_skill(
291292

292293
def _recall_skills_for_turn(
293294
self, goal: str
294-
) -> Tuple[List[Skill], Optional[SynthesisConfig]]:
295+
) -> Tuple[List[DistilledProcedure], Optional[SynthesisConfig]]:
295296
"""Recall the procedures matching ``goal`` once, with the resolved config.
296297
297298
The single per-turn recall pass shared by both consumers:
@@ -332,7 +333,7 @@ def _recall_skills_for_turn(
332333
return skills, config
333334

334335
def _build_recalled_skills_prompt(
335-
self, skills: List[Skill], config: Optional[SynthesisConfig]
336+
self, skills: List[DistilledProcedure], config: Optional[SynthesisConfig]
336337
) -> str:
337338
"""Render the recalled-procedure system-prompt section from ``skills``.
338339
@@ -400,8 +401,9 @@ def _refresh_recalled_skills(self, goal: str) -> None:
400401
"""Recompute the per-turn recalled-skill state for ``goal``.
401402
402403
Recalls the matching procedures **once** and caches both consumers'
403-
inputs: ``self._recalled_skills`` (the matched ``Skill`` objects, read by
404-
the tool loader through ``_recalled_skill_tools`` — #1451) and the
404+
inputs: ``self._recalled_skills`` (the matched ``DistilledProcedure``
405+
objects, read by the tool loader through ``_recalled_skill_tools`` —
406+
#1451) and the
405407
rendered ``self._recalled_skill_prompt`` (the system-prompt block, read by
406408
``get_recalled_skills_system_prompt`` — #887). The single recall keeps
407409
the loader's SKILL signal free (no second ``recall_skill``).

0 commit comments

Comments
 (0)