Skip to content

Commit 6478e9a

Browse files
committed
docs(memory): curation outcomes from agent-abstraction migration
0128 -- ReviewPanel.tsx imported react-markdown directly 0129 -- mermaid sequenceDiagram syntax hazards (no \n in note) 0130 -- agent abstraction in koan/agents replaces runner concept 0131 -- new docs files must avoid case collision 0132 -- cancelled initiative recovery via fresh plan workflow 0133 -- negative-presence shell verification uses grep -q 0134 -- lazy import discipline at koan.agents/runners boundary 0135 -- resumption run pattern: exec-review writes 0136 -- boundary translation pattern for retiring legacy types
1 parent 5dd5c78 commit 6478e9a

9 files changed

Lines changed: 127 additions & 0 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
title: ReviewPanel.tsx imported react-markdown directly, bypassing the Md mermaid-routing
3+
wrapper
4+
type: lesson
5+
created: '2026-04-29T09:08:56Z'
6+
modified: '2026-04-29T09:08:56Z'
7+
related:
8+
- 0121-visualization-framework-adopted-c4-l1-l3-mermaid.md
9+
---
10+
11+
This entry records a frontend rendering bug in koan's artifact viewer (`frontend/src/components/organisms/ReviewPanel.tsx`). On 2026-04-29, Leon reported that mermaid fenced blocks in artifacts (e.g. a `sequenceDiagram` in a generated `core-flows.md`) displayed as raw markup instead of inline SVG. Investigation found that ReviewPanel imported `ReactMarkdown` from `react-markdown` directly and rendered with `<ReactMarkdown remarkPlugins={[remarkGfm]}>{content}</ReactMarkdown>`. The project has a `<Md>` wrapper at `frontend/src/components/Md.tsx` that routes `className === 'language-mermaid'` fences to a `<MermaidBlock>` SVG renderer; all other markdown surfaces in the frontend (`MemoryRoutes.tsx`, `CurationTakeover.tsx`, `SteeringBar.tsx`, `KoanToolCard.tsx`) already composed `<Md>`. ReviewPanel was the sole outlier.
12+
13+
Root cause: when `<Md>` was extended with mermaid support during the visualization-framework adoption work in late April 2026, existing direct-`react-markdown` callers were not audited or migrated as part of that change. The new routing reached every `<Md>`-composing surface but silently failed in the un-migrated ones.
14+
15+
Fix applied 2026-04-29: Leon's plan replaced the direct `ReactMarkdown` usage in `ReviewPanel.tsx` with `<Md>{content}</Md>` and updated the file-header JSDoc to describe the new path. A grep audit of `from 'react-markdown'` across `frontend/src/` confirmed the post-fix invariant -- `frontend/src/components/Md.tsx` is the only place `react-markdown` is imported.
16+
17+
Leon endorsed a project rule from this fix: `<Md>` is the single `react-markdown` entry point in the koan frontend. Future markdown-rendering surfaces should compose `<Md>` rather than introduce a competing direct import; doing so bypasses any future renderer extensions.
18+
19+
Generalized lesson: when introducing a wrapper component over a shared library to add cross-cutting behavior, audit and migrate all existing direct-import callers in the same change. One un-migrated caller silently regresses the new behavior on its surface, and the regression stays invisible until a user reports it.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
title: 'Mermaid sequenceDiagram syntax hazards: no `;` in Note bodies; `<br>` for
3+
multi-line Notes'
4+
type: procedure
5+
created: '2026-04-29T09:09:05Z'
6+
modified: '2026-04-29T09:09:05Z'
7+
related:
8+
- 0121-visualization-framework-adopted-c4-l1-l3-mermaid.md
9+
---
10+
11+
This entry documents two parser hazards in mermaid `sequenceDiagram` syntax that affect LLM-generated diagrams in koan's planning artifacts (`core-flows.md`, `tech-plan.md`). On 2026-04-29, Leon surfaced a parse-error reproduction in a generated `core-flows.md`: a line `Note over A, B: Two unrelated entry points; mutually exclusive per agent` failed with `Parse error on line 9 ... Expecting '()', 'SOLID_OPEN_ARROW', 'DOTTED_OPEN_ARROW', 'SOLID_ARROW', ...` pointing at the next line.
12+
13+
Root cause: in mermaid's `sequenceDiagram` grammar, `;` is a statement separator (the alternative to a newline). When `;` appears inside a `Note over` body, it terminates the Note mid-sentence; the remainder of that line parses as a new statement expecting an arrow token, and on failure the parser eats the next line still searching for the arrow -- producing the misleading "Expecting SOLID_ARROW" error pointing at the line below.
14+
15+
Two hazards captured in `docs/visualization-system.md` section 8 ("Mermaid syntax hazards") on 2026-04-29:
16+
17+
- Do not use `;` inside `Note over`, `Note left of`, or `Note right of` bodies, or inside message labels (the text after the `:` in `A->>B: text`). Use `,`, `--`, or split into two separate Notes.
18+
- For multi-line Notes, use the `<br>` HTML break tag rather than embedding a raw newline; mermaid does not parse multi-line Note bodies across raw newlines.
19+
20+
Why this matters for LLM-generated content specifically: LLMs naturally produce prose with semicolons (parenthetical clauses, list separators). Without explicit guidance in the generation prompt, the LLM emits parser-breaking content, producing a frozen artifact that fails to render until a downstream reader notices.
21+
22+
The rule was inlined into the `PHASE_ROLE_CONTEXT` strings of `koan/phases/core_flows.py` and `koan/phases/tech_plan_spec.py` (the two phases that emit `sequenceDiagram` content per the visualization-framework slot mapping), each as a 7-line `## Mermaid syntax hazards` subsection that cross-references `docs/visualization-system.md` section 8. Two presence tests in `tests/test_phase_guidance.py` (`test_core_flows_role_context_includes_mermaid_syntax_hazards`, `test_tech_plan_spec_role_context_includes_mermaid_syntax_hazards`) assert the heading, the semicolon mention, the `<br>` mention, and the doc cross-reference, guarding against silent regression of the inlined guidance.
23+
24+
Procedure for future agents: any new phase module that emits mermaid `sequenceDiagram` content should follow the same pattern -- inline the hazards rule in `PHASE_ROLE_CONTEXT` with a cross-reference to `docs/visualization-system.md` section 8, and add a parallel presence test in `tests/test_phase_guidance.py`.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
title: Agent abstraction in koan/agents/ replaces Runner Protocol; ClaudeSDKAgent
3+
+ CommandLineAgent split with PostToolUse steering hook
4+
type: decision
5+
created: '2026-05-02T07:23:05Z'
6+
modified: '2026-05-02T07:23:05Z'
7+
related:
8+
- 0001-persistent-orchestrator-over-per-phase-cli-spawning.md
9+
- 0016-steering-vs-phase-boundary-message-routing-dual-queue-design.md
10+
- 0004-file-boundary-invariant-llms-write-markdown-driver-writes-json.md
11+
---
12+
13+
The koan agent-spawn layer (`koan/agents/`, `koan/subagent.py:spawn_subagent`, `koan/agents/steering.py`, `koan/web/mcp_endpoint.py:_drain_and_append_steering`) was redesigned between 2026-04-29 and 2026-05-02 to integrate the Claude Agent SDK as the canonical Claude transport. On 2026-04-29, user directed the migration with the brief instruction "migrate our Claude Code agent implementation to use the Claude Agent SDK"; intake and tech-plan-spec produced a 13-decision design captured in the run brief at `~/.koan/runs/1777448300-422e9a02/brief.md`. On 2026-04-29 the agent-abstraction milestone shipped the new package with an interim `CommandLineAgent` wrapper around `ClaudeRunner`; on 2026-04-30 the SDK-adapter milestone shipped `ClaudeSDKAgent` and deleted `koan/runners/claude.py`; on 2026-05-02 the documentation milestone shipped `docs/agent-protocol.md`.
14+
15+
The architectural spine: user approved a new `Agent` Protocol with primitives `run`, `interrupt`, `compact`, `register_process`, `exit_code`, `stderr_output`, `list_models`, replacing the subprocess-shaped `Runner` Protocol as koan's public surface for agent integration. Two implementations satisfy the Protocol -- `ClaudeSDKAgent` (drives `claude_agent_sdk.ClaudeSDKClient`, dependency pinned in `pyproject.toml`) and `CommandLineAgent` (wraps `koan.runners.base.Runner` instances for codex and gemini); `koan/runners/` became an internal detail of `CommandLineAgent`. Configuration flowed through a single `AgentOptions` dataclass passed to `Agent.run()`. Steering migrated to a single `PostToolUse` hook on Claude calling `koan/agents/steering.py:drain_for_primary` and rendering via `render_text` for the SDK's `additionalContext` field; codex and gemini retained MCP-handler injection (`render_blocks` for content-block output). Both paths shared the single `drain_for_primary` helper -- one drain, two formatters. Hooks stayed a Claude-internal implementation detail not exposed on the Protocol.
16+
17+
HTTP MCP at `http://localhost:{port}/mcp?agent_id={id}` remained the single transport for all agents -- user rejected in-process `McpSdkServerConfig` for parity with codex and gemini and to preserve the existing `AgentResolutionMiddleware` agent-id-via-URL convention. User directed a hard cutover with no opt-out flag; `koan/runners/claude.py`, `RunnerDiagnostic`, and `RunnerError` were deleted in one change. `interrupt()` was defined but no caller was wired in this work; `compact()` raises `NotImplementedError` everywhere because the SDK does not expose programmatic compaction. The decision superseded part of the 2026-04-02 rationale that had rejected an "API-based" Claude integration: the Agent abstraction is API-shaped via the SDK but preserves the persistent-process model, the `StreamEvent` contract, and the per-agent runner abstraction the earlier rejection was protecting.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
title: New docs/ files must avoid case-collision with the per-directory AGENTS.md
3+
conventions file
4+
type: procedure
5+
created: '2026-05-02T07:23:14Z'
6+
modified: '2026-05-02T07:23:14Z'
7+
---
8+
9+
The koan repository uses per-directory `AGENTS.md` conventions files at the project root, in `docs/`, in `frontend/`, and in `frontend/src/components/`. Each holds the conventions for agents working in that directory. On 2026-05-02, during intake re-planning of cancelled documentation work for the Claude Agent SDK migration, the agent discovered that the original `plan-milestone-3.md` (finalized 2026-04-30) prescribed writing the new Agent reference at `docs/agents.md`. On macOS's case-insensitive default filesystem (HFS+ and APFS in their default configuration), `docs/agents.md` and `docs/AGENTS.md` resolved to the same inode -- `diff` returned no output, confirming identical content. Writing `docs/agents.md` would have overwritten the conventions file. User directed the rename to `docs/agent-protocol.md` (selected from four alternatives offered). On the same date, user established the rule that any new file added to a directory containing an `AGENTS.md` conventions file must use a name that does not match `agents.md` in any case. The check is mechanical: a candidate filename whose lowercase form equals `agents.md` is unsafe and must be renamed. The rule applies to all directories carrying the per-directory conventions convention -- `docs/`, `frontend/`, `frontend/src/components/`, and the project root.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
title: Cancelled-initiative recovery -- fresh plan workflow with original run's artifacts
3+
as inputs
4+
type: procedure
5+
created: '2026-05-02T07:23:22Z'
6+
modified: '2026-05-02T07:23:22Z'
7+
related:
8+
- 0122-brief-contradictions-discovered-downstream-are-resolved-in-the-consumer-artifact-not-by-amending-the-frozen-brief.md
9+
---
10+
11+
The koan workflow engine (`koan/driver.py`, `koan/lib/workflows.py`) has no native primitive for resuming a cancelled `initiative` or `milestones` run mid-flight. Run-state -- the orchestrator's phase counter, `workflow_history`, in-memory `PhaseContext` -- is per-process and is not portable across orchestrator spawns. On 2026-05-02, user encountered a cancelled Claude Agent SDK migration at `~/.koan/runs/1777448300-422e9a02/` (the agent-abstraction and SDK-adapter milestones were complete; the documentation milestone's plan was finalized but never executed) and asked the agent to "plan to complete the work" in a new run. The agent surfaced two recovery paths in intake: re-plan from scratch in a fresh `plan` workflow with the cancelled run's `milestones.md` and `plan-milestone-N.md` as intake inputs, OR execute the already-finalized `plan-milestone-N.md` directly. User directed re-plan-fresh on 2026-05-02. Rationale captured in the new plan's brief: re-planning produced a focused single-purpose plan workflow rooted in the latest codebase state; direct execution risked shipping a plan whose assumptions had drifted since it was written (the SDK-adapter milestone's deletions had landed in the interim). On the same date, user adopted the recovery procedure -- read the cancelled run's `brief.md`, `milestones.md`, and the most-recent `plan-milestone-N.md` as intake inputs; produce a fresh `brief.md` scoping only the leftover work; produce a fresh `plan.md`; execute. The original run's artifacts are inputs, not outputs to amend -- the cancelled run's frozen `brief.md` is read-only, and any contradiction discovered downstream resolves in the consumer artifact (the new plan workflow's `brief.md`).
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
title: Negative-presence shell verification uses grep -q + echo to avoid silent shell-pipeline
3+
failures
4+
type: procedure
5+
created: '2026-05-02T07:23:27Z'
6+
modified: '2026-05-02T07:23:27Z'
7+
related:
8+
- 0114-safe-deletion-patterns-for-milestone-driven-removals-migrate-callers-before-delete-total-deletion-in-one-change-negative-presence-assertions-why-comments-at-deletion-sites-replace-not-repurpose.md
9+
---
10+
11+
The koan executor (`koan/subagent.py`, executor task instructions written to `~/.koan/runs/<run>/subagents/executor-*/task.json`) runs Bash verification steps that often include negative-presence assertions -- "grep should find no matches, because the symbol was deleted". On 2026-04-30, during the executor run that deleted `koan/runners/claude.py`, a verification step used raw `grep -nE "from .*claude.*import (RunnerDiagnostic|RunnerError)" path/to/file.py` to assert the symbol was no longer present. When the symbol was absent, `grep` found no match, exited 1, and the shell pipeline propagated the non-zero exit. The executor reported run failure despite all source changes having succeeded. On 2026-04-30, the agent revised the resumption plan to use the form `grep -q PATTERN path/to/file.py && echo "STILL IMPORTABLE: FAIL"`. The `grep -q` exits 0 when matches exist and 1 when absent; the `&& echo` prints only on the match-exists path; the overall pipeline exits 0 in both outcomes. The printed marker became the failure signal, not the shell exit code. On the same date, user established the rule that future plan-spec phases prescribing Bash verification of deleted-symbol absence must use this form. Companion lesson from the same run: `tests/test_runners.py` imported `ClaudeRunner`; the original plan deleted `koan/runners/claude.py` before `tests/test_runners.py` was updated to drop its `TestClaudeRunner` classes, causing pytest collection to fail with `ImportError`. The corrected ordering codified in the resumption guidance: test cleanup before symbol deletion before verification -- "callers" of a deleted symbol includes test modules in addition to source modules.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
title: Lazy-import discipline at the koan agents/runners boundary -- module-level
3+
imports of runner classes from koan/agents/ create a circular import
4+
type: procedure
5+
created: '2026-05-02T07:31:08Z'
6+
modified: '2026-05-02T07:31:08Z'
7+
related:
8+
- 0130-agent-abstraction-in-koanagents-replaces-runner.md
9+
---
10+
11+
The koan agent infrastructure (`koan/agents/`, `koan/runners/`) has a structural import cycle: `koan.runners/__init__.py` eagerly imports its submodules and codex/gemini import diagnostic types from `koan.agents.base`. On 2026-04-29, the first executor run for the agent-abstraction milestone of the Claude Agent SDK migration introduced module-level `__import__("koan.runners.codex", fromlist=["CodexRunner"]).CodexRunner` calls in `koan/agents/registry.py`. The transitive chain reached at agent-registry load time was `koan.state -> koan.projections -> koan.runners.base -> koan.runners/__init__.py -> koan.runners.codex -> koan.agents.base -> koan.agents/__init__.py -> koan.agents.registry`. When `koan.agents.registry` evaluated the `__import__` calls at module load, `koan.runners.codex` was mid-load and the partial module had no `CodexRunner` attribute yet, raising `AttributeError: partially initialized module ...` and aborting pytest collection. On 2026-04-29, the agent's exec-review revised the milestone plan to prescribe lazy imports inside method bodies as the canonical break; the resumption executor run on 2026-04-29 applied the fix and completed the milestone. On the same date, user accepted the rule that imports of runner classes inside `koan/agents/` must happen at method-body scope, not at module top-level -- specifically, `AgentRegistry.get_agent` performs `from koan.runners.codex import CodexRunner` inside its body, not at the top of `koan/agents/registry.py`. User extended the rule to the SDK import inside `ClaudeSDKAgent.run()`. Module-level imports of runner classes from inside `koan/agents/` re-create the cycle.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
title: Resumption-run pattern -- exec-review writes Resumption guidance header to
3+
plan-milestone-N.md, user yields to execute, next executor reads guidance first
4+
type: procedure
5+
created: '2026-05-02T07:31:15Z'
6+
modified: '2026-05-02T07:31:15Z'
7+
related:
8+
- 0102-milestonesmd-outcome-schema-integration-points.md
9+
- 0114-safe-deletion-patterns-for-milestone-driven-removals-migrate-callers-before-delete-total-deletion-in-one-change-negative-presence-assertions-why-comments-at-deletion-sites-replace-not-repurpose.md
10+
---
11+
12+
The koan workflow engine (`koan/phases/exec_review.py`, `koan/lib/workflows.py:MILESTONES_WORKFLOW`) supports recovery from partial executor runs through a header-mediated protocol. On 2026-04-29, during the agent-abstraction milestone of the Claude Agent SDK migration, the first executor run completed steps 1-15 of `plan-milestone-1.md` but stopped before completing steps 16-21 -- a circular-import error blocked test collection. The exec-review phase identified the gaps; the agent revised `plan-milestone-1.md` to insert a "Resumption guidance" header above the existing "Approach summary" section, describing what was done versus pending, the required plan amendment, and any ordering constraints discovered during the first run. User directed `koan_set_phase("execute")` with the instruction "Re-run the executor on the revised plan-milestone-N.md". The resumption executor read the "Resumption guidance" header first, applied the documented amendment, and completed the remaining steps. The same pattern applied on 2026-04-30 during the SDK-adapter milestone when a shell-pipeline trap blocked deletion verification. On 2026-04-30, user established the resumption protocol: exec-review writes a "Resumption guidance" header to `plan-milestone-N.md` describing (a) what is done vs pending, (b) any required plan amendments, (c) ordering constraints discovered during the first run; user yields back to execute with a re-run instruction; the next executor reads the resumption guidance before the original plan body. The header is preserved in `plan-milestone-N.md` as a record of the deviation; the "Deviations from plan" subsection of the milestone's eventual Outcome in `milestones.md` captures the same facts at the milestone level.

0 commit comments

Comments
 (0)