agent-guides/plan-sm-rewrite-v3.md

ACE SkillManager Rewrite — Plan v3

Semi-temporary planning doc. Delete once PR 3 lands and is merged.

PR 1 — Substrate

ace/core/context.py:95 — add field to ACEStepContext:

• injected_skill_ids: tuple[str, ...] = ()

ace/core/skillbook.py:226 — add counters to Skill:

• used_count: int = 0
• helpful_count: int = 0
• harmful_count: int = 0
• neutral_count: int = 0

ace/core/skillbook.py — add Skillbook.tag_skill(skill_id, delta: Literal[+1, -1, 0]) method. Keep the TAG branch in _apply_operation as its implementation so serialized UpdateOperation(TAG) still works.

ace/core/skillbook.py:535 — as_prompt() stays unchanged (no counter rendering).

Agent step (ace/implementations/agent.py:96) — after rendering the skillbook into the prompt, write injected_skill_ids onto the context and bump used_count on each. This is the only upstream counter touched.

Remove citation plumbing:

• ace/core/outputs.py:52 — delete SkillTag.
• ace/core/outputs.py:79 — delete ReflectorOutput.skill_tags.
• ace/implementations/skill_manager.py:109-112 — remove the skill_tags consumer block.

Docs:

• docs/design/ACE_ARCHITECTURE.md — update Skill section (counters, injection-based attribution).
• docs/design/ACE_DECISIONS.md — add "Injection is ground truth; citation dropped."

PR 2 — Reflector & RR prompts

ReflectorOutput stays pure analysis — reasoning, error_identification, root_cause_analysis, correct_approach, key_insight. No tagging fields, no harmful_ids/helpful_ids.

Reflector prompt (ace/implementations/prompts.py:419-432) — delete REFLECTOR_SKILL_EVAL_SECTION entirely.

RR prompt (ace/implementations/rr/prompts.py:115-136) — delete the re.findall citation recipe. No replacement. RR may still be told "inspect the skillbook (covered / contradicted / gap) and narrate what you find" as analysis guidance — never as a decision.

Two read-only RR tools (so RR can enrich its narrative without citations):

• search_skillbook(query, top_k) → wraps retrieve_top_k.
• read_skill(id) → Skillbook.get_skill. Return value includes counters.

ace/steps/rr_step.py:

• _build_traces_dict (line 267) — drop skill_ids; add injected_skill_ids from context.
• Line 399 — remove the skill_tags=[] remnant in the timeout fallback.

RR batch input preserved at the caller level — contract unchanged.

PR 3 — Rewrite SkillManager (same class)

Rewrite SkillManager on top of RecursiveAgent. Same class name, same SkillManagerLike protocol, same UpdateStep wrapper — nothing upstream changes.

Mutation tools (operate on the real Skillbook, no staging):

• add_skill(section, content, justification, evidence) → Skillbook.add_skill.
• update_skill(skill_id, content?, justification?, evidence?) → Skillbook.update_skill.
• remove_skill(skill_id, reason) → Skillbook.remove_skill.
• tag_skill(skill_id, delta: +1 | -1 | 0) → Skillbook.tag_skill.

Read-only tools:

• search_skills(query, top_k) → returns skills with counters.
• read_skill(id) → returns skill with counters.

Sandbox:

• sandbox_eval(code) — reuse register_execute_code; opt-in per runner.

Termination:

• finalize(reasoning) → terminates the loop; returns SkillManagerOutput as an audit log of what was done, not a plan to be applied.

Delete ApplyStep (ace/steps/apply.py:10) and remove it from the ACE pipeline composition. UpdateStep is the sole SM invocation and the skillbook is already mutated when it returns. UpdateStep.max_workers=1 stays.

SkillManagerOutput shape — reasoning: str + operations: list[UpdateOperation]. Operations become a post-hoc audit trail. Same dataclass, same serialization; semantic shift only.

UpdateBatch / apply_update / _apply_operation remain for offline reconstruction and tests, but the online path no longer flows through them.

SM prompt (ace/implementations/prompts.py:439+):

• Drop <skill_effectiveness> section (lines 505-513).
• Instruct the SM that it decides helpful/harmful/neutral from injected_skill_ids + outcome + reflection.
• Instruct the SM to REMOVE skills whose harmful_count ≥ N when it encounters them during investigation. Counters are surfaced via read_skill / search_skills, not the rendered skillbook.

AgenticConfig(max_requests=…) — max_requests=1 degrades to a single-tool-call pass.

Docs:

• docs/design/ACE_ARCHITECTURE.md — SM section (tools, direct mutation, no ApplyStep).
• docs/design/ACE_REFERENCE.md — tool surface.
• docs/design/ACE_DECISIONS.md — "SM mutates directly; Reflector is analysis-only."

Out of scope

• RetrieveStep / production top-k injection.
• Sandbox-gated commit gating (Voyager-style verify-before-ADD).
• Counter decay / windowing.
• Global "sweep all skills with harmful_count ≥ N" tool.

Ship order

PR 1 → PR 2 → PR 3.