From 4340585b950303985d47d9367fd9e296dc251bc8 Mon Sep 17 00:00:00 2001 From: Jay Moran Date: Tue, 22 Sep 2026 16:26:18 -0700 Subject: [PATCH 01/21] Move agent guidance from CLAUDE.md to AGENTS.md --- AGENTS.md | 228 +++++++++++++++++ CLAUDE.md | 229 +----------------- frontend/README.md | 2 +- .../src/components/LlmConnectionCheck.tsx | 2 +- .../components/protocol/PythonCodeEditor.tsx | 2 +- .../src/components/protocol/cells/AGENTS.md | 121 +++++++++ .../src/components/protocol/cells/CLAUDE.md | 122 +--------- .../components/protocol/cells/CellsTab.tsx | 2 +- .../protocol/nodes/NodeFactorBadge.tsx | 2 +- frontend/src/pages/AGENTS.md | 30 +++ frontend/src/pages/CLAUDE.md | 31 +-- src/asaree/models/dataset.py | 4 +- src/asaree/models/experiment_dataset.py | 2 +- src/asaree/services/csv_export.py | 2 +- 14 files changed, 391 insertions(+), 388 deletions(-) create mode 100644 AGENTS.md create mode 100644 frontend/src/components/protocol/cells/AGENTS.md create mode 100644 frontend/src/pages/AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..f5e0c6a --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,228 @@ +# Frontend visual style + +The frontend (`frontend/`) uses a deliberate dark, retrofuturist/high-tech visual language. +Preserve it for all new pages and components — don't revert to shadcn's plain default theme +or introduce a different aesthetic without being asked. + +- **Theme**: dark mode only (`` in `index.html`), a graphite background + (not near-black) with a cyan/electric-blue accent (`--primary`). Tokens live in + `frontend/src/index.css`'s `.dark` block — adjust values there, don't hardcode colors in + components. +- **Motifs**: a faint cyan grid backdrop and scanline texture on `body`, glowing buttons + (`components/ui/button.tsx` — every variant glows in its own accent color: `default`/ + `outline`/`secondary`/`ghost` in `--primary`, `destructive` in `--destructive`; only `link` + has no surface to glow), and glowing HUD-style corner brackets on cards + (`components/ui/card.tsx`'s `Card`, plus `components/AuthLayout.tsx`). These live in the + shared primitives, not per-page overrides — a new page gets them for free by using + `Card`/`Button`/`AppHeader`, so build on those rather than hand-rolling styles. +- **Monospace for technical readouts**: IDs, hashes, token prefixes, cell labels, and + key=value data dumps use `font-mono` (see `components/protocol/cells/CellsTable.tsx`, + `ApiTokensSection.tsx`) + to read like a real data/terminal output, not prose. +- **Every button glows uniformly, by explicit choice**: an earlier pass reserved the glow for + one "primary" action per section (`default` variant only), with `outline`/`ghost` left + plain — the user found that inconsistent and asked for uniform glowing buttons instead. + Don't reintroduce the primary-only hierarchy; if a future change to `button.tsx` needs a + visual hierarchy again, ask first rather than assuming the old convention. +- **Tables vs. cards is a zoom-level decision, not a style preference**: a "container" list — + sparse metadata, click one to drill into it (e.g. the Experiments list) — is a card/tile + grid, not a table; a table only earns its place once you're inside that container looking + at dense, precise, multi-attribute records to compare (e.g. an experiment's Cells, with + their metrics/hyperparameters). Don't reach for a table just because a list exists — check + which zoom level you're actually building first. + - Detail on the Experiments tile grid lives in `frontend/src/pages/AGENTS.md`; detail on + the Cells heatmap/table (and the per-agent run tally that replaced the old Agents grid) + lives in `frontend/src/components/protocol/cells/AGENTS.md`. Read the relevant one before + touching `ExperimentsPage.tsx` or anything under `components/protocol/cells/`. +- **There is no separate experiment detail page** — clicking an Experiments tile lands on + `/experiments/{id}/protocol`, the protocol canvas, and everything that used to be on a + static `ExperimentDetailPage` now lives in that canvas's `ExperimentSidePanel` tabs + (Design/Cells/Runs/Results) or its top bar. `/experiments/{id}` is kept only as a redirect + for old bookmarks. Don't reintroduce a static detail page; a new per-experiment view is a + new tab in that panel. The panel is drag-resizable by its right edge (320px–1100px, always + leaving the canvas ≥420px) and remembers its width in `localStorage` — so a new tab in it + should be width-responsive via container queries, not built for one fixed column width. + +## Agent skills + +### Issue tracker + +Implementation issues and specs live in the separate `EpistasisLab/ASAREE_Issues` GitHub repository. See `docs/agents/issue-tracker.md`. + +### Triage labels + +Use the five canonical Matt Pocock triage labels. See `docs/agents/triage-labels.md`. + +### Domain docs + +This is a single-context repository with `CONTEXT.md` at the root and private local ADRs under `docs/adr/`. See `docs/agents/domain.md`. + +# Git commit conventions + +Do not add `Co-Authored-By` or `Generated-with` lines to commits or PRs. + +# Cost-conscious execution + +Favor the cheapest tool/approach that reliably gets the job done. Before doing any of the +following, say in one sentence what you're about to do and why it's needed, and ask first +rather than doing it silently as a routine part of coding: + +- **No Playwright/browser automation or screenshot-based UI verification.** The user checks + UI changes visually themselves once you report them done — see the standing preference to + skip Playwright verification agents. Screenshots are read as images, which cost far more + tokens than text, and driving a browser adds many extra tool round-trips on top. Verify with + `tsc`/`oxlint`/existing tests instead; if something genuinely can't be confirmed without a + browser, say so explicitly rather than skipping verification or reaching for Playwright + unasked. +- **No multi-agent fan-out for routine coding work** — don't invoke the `Workflow` tool or + spawn multiple subagents in parallel just because a task touches several files. Each spawned + agent carries its own multiplied context/token cost. Default to working solo, or at most one + scoped `Explore`/`general-purpose` agent for a targeted search. Reserve `Workflow`/parallel + fan-out for when the user explicitly asks for that scale. +- **No unscoped, repo-wide exploration** ("search everywhere," reading many files in full) when + the request already names specific files or areas — grep/read narrowly first, and only widen + the search if that comes up empty. +- **No speculative full test-suite runs or long builds** — run the narrowest test/lint command + that covers the change under review; ask before running something that takes minutes or spins + up extra infrastructure (dev servers, containers, etc.). + +# Experiment data model + +- **Datasets are a real, stored many-to-many** — the `experiment_datasets` join table + (`models/experiment_dataset.py`, migration `d5a3b90c71e4`), unlike agents below: a dataset + genuinely is a first-class property of an experiment worth a real relationship, matching + what ARES does. It started as a scalar `ResearchExperiment.dataset_id` FK (migration + `a1b2c3d4e5f6`); that column is **dropped** — uncapping the Dataset connector means one + experiment can run against several, so a single winner would have been a lie. Read/write it + through `services/experiments.py`'s `set_experiment_datasets` / + `get_experiment_dataset_ids` / `get_dataset_ids_by_experiment` (the batch one — use it on + list endpoints to avoid an N+1), never by touching the model. + - `position` on the join row preserves **canvas wiring order**, which is the order the + agent's prompt lists the datasets in, so it's user-visible, not cosmetic. + - `dataset_ids` is a **full replacement**, not a merge (`[]` detaches everything). The API + and SDK still accept and return the old scalar `dataset_id`: on write it's the + one-dataset shorthand, on read a view of `dataset_ids[0]`. Don't add a second source of + truth — the join table is it. + - It's set via `PATCH /experiments/{id}`, not at creation: the notebook's Step 1 (create the + experiment) runs *before* Step 2 (register the dataset), so there's nothing to attach yet + at create time — see the `client.experiments.update(...)` call after Step 2 in + `spinal_pipeline.ipynb`. In the GUI it's `ProtocolCanvas.tsx`'s `syncExperimentDatasets` + effect, which PATCHes whenever the canvas's set of Dataset nodes changes. That effect's + ref is deliberately **seeded from the graph as loaded so it never fires on mount** — a + mount-time "reconcile" would see zero Dataset nodes on a notebook-driven experiment and + detach its dataset. Keep that property if you touch it. + - An experiment with no datasets attached is an expected, permanent state (everything + created before the FK existed) — not a bug to backfill. +- **Agents are deliberately NOT a stored relationship** — there's no `experiment_agents` join + table, and none is planned. Agents are reusable per-user templates, not something an + experiment "owns"; asking "which agents ran in this experiment" is answered by scanning the + user's `Run`s for `run_metadata.experiment_id` matches (see `ExperimentAgents` in + `components/protocol/RunsTab.tsx`) and cross-referencing `GET /agents`, not by a new backend + association. `GET /runs` has no server-side `experiment_id` filter (only `agent_id`) — this + fetches every run for the user and filters client-side, which is fine at today's scale but + is the place to add a real filter if a user's run history grows large enough to matter. +- **Cells belong to a design revision, not to the experiment** — `experiment_design_revisions` + (`models/experiment_design_revision.py`, migration `e2f7c4a91b60`). The current design is the + one revision with `superseded_at IS NULL` (a partial unique index enforces "at most one", + rather than convention). This exists because generation used to be purely additive: a design + shrunk from 6 cells to 2 left all 6 behind, so the experiment still read "0/6 scored" and + "run all cells" still launched 6. + - **Never query `FactorialReplicateResult` without joining its owning cell** — that loses + experiment/design-revision scope. Go through + `services/factorial_cells.py`'s `get_replicate`/`list_replicates`/`upsert_replicate`, which scope to the + current revision by default and take an explicit `revision_id` only to read history on + purpose. `experiment_id` lives on the parent cell, and both experiment and revision filters + are applied together because `revision_id` can arrive from a query string. + - **Cell vs. replicate:** a `FactorialCell` is one unique factor combination together + with all its planned `FactorialReplicateResult` children. Group replicate responses by `cell_id`, + and use “replicate” for run/progress/scored counts. “Run all cells” is the experiment-level + action that runs every pending replicate across those cells. + - `generate_design_cells` opens a new revision **only when the new design would drop a cell + the current one has**. Re-clicking generate, changing the seed, widening a factor's levels + or raising `replicates` all keep the current revision and its row ids — history entries are + meant to mark designs that actually discarded something, not every edit. Results for a + label the two revisions share are copied forward; the originals stay in history. + - `ProtocolRun.design_revision_id` **pins** which design a run's result belongs to, so a + regenerate mid-flight can't redirect the write-back (`plan_cell_runs` → + `run_protocol`/`promote_cell_score_metrics`). It's `ondelete="SET NULL"` — run history + outlives the design; cells are `ondelete="CASCADE"` so deleting a revision really does + delete its results. + - Deleting the **current** revision is refused (409) — that's a reset, not a deletion; + regenerating is how you replace it. Revision numbers are `max()+1` over every revision ever, + so a deleted number is never reused. The history UI is `components/protocol/cells/ + DesignHistory.tsx`. +- **A protocol canvas is a draft; production runs use a published revision** — `Protocol.graph` + remains the autosaved draft while `protocol_revisions` stores immutable graph snapshots. A + `ProtocolRun` carries both `design_revision_id` and `protocol_revision_id`; `run_protocol` + loads the latter, so a canvas edit can never hot-patch queued, running, or resumed work. Never + create a production run from `Protocol.graph` directly: publish first and pass the resulting + revision through the planning path. The top bar deliberately says when a visible canvas has + unpublished changes and which published revision production currently uses. +- **A declared factor must bind to at least one canvas field before a cell batch can run** — + deleting/unbinding a node field leaves the factor declared but *unbound*, rather than silently + deleting the experimental treatment. The user must rebind it or remove it, then review the + design impact and regenerate. `services.factor_bindings` is the shared backend guard; the + Design tab shows the same state before generation. + +# Color — meaningful variation, not decoration + +A single cyan accent everywhere read as flat/monotone rather than retrofuturist (that style +leans on multi-hue neon contrast — synthwave, Tron, Blade Runner — not minimalism). Cards can +now be individually re-tinted, but the tint must mean something; don't add color for its own +sake. + +- **Mechanism**: `components/ui/card.tsx`'s `Card` reads a `--card-accent` CSS custom property + (falling back to `--primary`) for its ring/glow/corner-bracket colors — see the comment on + `Card` itself. Set it via `style={cardAccent('var(--chart-3)')}` (`lib/utils.ts`) on any + individual `Card`; anything that also wants the icon/badge/hover-arrow to match reads + `text-[color:var(--card-accent,var(--primary))]` the same way `AgentCard` and the Experiments + tiles do — don't hardcode `text-primary` on elements living inside a re-tintable card. +- **Status-driven tint** (`lib/experiment.ts`'s `cellsStatusAccent`): `--chart-4` (amber) = cells + generated but none scored, `--primary` (cyan) = partially scored, `--chart-3` (emerald) = fully + scored, `--muted-foreground` (dim) = no cells yet. Used on both the Experiments-list tiles and + the detail page's "Cells" stat card — the same status must always resolve to the same color + everywhere, so change it in that one function, not per call site. +- **Hash-driven tint** (`lib/utils.ts`'s `hashToChartHue`): for things with no "done/pending" + status but real category variety — e.g. `AgentCard` tints by `model_config.model`, so agents + sharing an LLM visually match without a hardcoded model→color table that goes stale the moment + a new model ships. Same input always produces the same one of the five `--chart-*` hues. + **Not for protocol-canvas nodes** — see the table below. +- **Table-driven tint** (`lib/nodeAccent.ts`'s `nodeAccent(kind)`): protocol-canvas nodes only. + Thirteen node kinds against five `--chart-*` hues made collisions arithmetic, and they landed + on the confusable pairs (Skill/AI, Dataset/Knowledge, Pattern/Script), so every kind now has an + explicit entry. Five keep the `--chart-*` hue the old hash gave them (agent, dataset, + reason+act, critic gate, and the LLM family) and the other eight moved to a `--node-1`…`--node-8` + slot in `index.css`'s `.dark` block — **fixing a repeat means moving the bucket-mates, not + repainting the canvas**, so don't reassign an anchor's hue without being asked. Both the node + card and its inspector call `nodeAccent` with the same key so they can't drift; a kind with no + entry falls back to `--primary` rather than a hashed hue, so a new node type visibly asks for a + slot. Adjacent hues are assigned to related kinds on purpose (the two MCP kinds, the two OKF + kinds). Note LLM nodes are one hue for the whole family, not one per provider. + - `--node-label` (yellow) is separate from all of them: it's the connector captions on + `AgentNode`/`CriticGateNode`, which are meant to stand out *from* their node, so they + deliberately don't follow `--card-accent` the way everything else inside a card does. +- **Don't hash/rotate a tint just to break up visual monotony** with no underlying meaning (e.g. + cycling colors by array index) — that was considered and rejected in favor of the schemes + above. If a new list of things genuinely has no status or category worth encoding in color, + it's fine for it to stay a single accent. + +# Communication style + +Be concise and execution-oriented. + +For coding tasks: +- Do the requested work without lengthy explanations. +- Make routine implementation decisions yourself. +- Do not present multiple alternatives unless the choice materially affects + architecture, correctness, or requirements. +- Do not enumerate pros and cons for routine decisions. +- Do not explain obvious code changes. +- Do not repeatedly summarize what you are doing. +- Ask questions only when ambiguity would materially change the implementation. +- Prefer implementing over discussing. +- After completing a task, give a short summary of what changed. +- If you identify an important concern, state it briefly and recommend one + course of action rather than presenting many possibilities. + +Keep responses focused, brief, and concise. Give deeper explanations only +when I explicitly ask for them. diff --git a/CLAUDE.md b/CLAUDE.md index 73f237f..43c994c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,228 +1 @@ -# Frontend visual style - -The frontend (`frontend/`) uses a deliberate dark, retrofuturist/high-tech visual language. -Preserve it for all new pages and components — don't revert to shadcn's plain default theme -or introduce a different aesthetic without being asked. - -- **Theme**: dark mode only (`` in `index.html`), a graphite background - (not near-black) with a cyan/electric-blue accent (`--primary`). Tokens live in - `frontend/src/index.css`'s `.dark` block — adjust values there, don't hardcode colors in - components. -- **Motifs**: a faint cyan grid backdrop and scanline texture on `body`, glowing buttons - (`components/ui/button.tsx` — every variant glows in its own accent color: `default`/ - `outline`/`secondary`/`ghost` in `--primary`, `destructive` in `--destructive`; only `link` - has no surface to glow), and glowing HUD-style corner brackets on cards - (`components/ui/card.tsx`'s `Card`, plus `components/AuthLayout.tsx`). These live in the - shared primitives, not per-page overrides — a new page gets them for free by using - `Card`/`Button`/`AppHeader`, so build on those rather than hand-rolling styles. -- **Monospace for technical readouts**: IDs, hashes, token prefixes, cell labels, and - key=value data dumps use `font-mono` (see `components/protocol/cells/CellsTable.tsx`, - `ApiTokensSection.tsx`) - to read like a real data/terminal output, not prose. -- **Every button glows uniformly, by explicit choice**: an earlier pass reserved the glow for - one "primary" action per section (`default` variant only), with `outline`/`ghost` left - plain — the user found that inconsistent and asked for uniform glowing buttons instead. - Don't reintroduce the primary-only hierarchy; if a future change to `button.tsx` needs a - visual hierarchy again, ask first rather than assuming the old convention. -- **Tables vs. cards is a zoom-level decision, not a style preference**: a "container" list — - sparse metadata, click one to drill into it (e.g. the Experiments list) — is a card/tile - grid, not a table; a table only earns its place once you're inside that container looking - at dense, precise, multi-attribute records to compare (e.g. an experiment's Cells, with - their metrics/hyperparameters). Don't reach for a table just because a list exists — check - which zoom level you're actually building first. - - Detail on the Experiments tile grid lives in `frontend/src/pages/CLAUDE.md`; detail on - the Cells heatmap/table (and the per-agent run tally that replaced the old Agents grid) - lives in `frontend/src/components/protocol/cells/CLAUDE.md`. Read the relevant one before - touching `ExperimentsPage.tsx` or anything under `components/protocol/cells/`. -- **There is no separate experiment detail page** — clicking an Experiments tile lands on - `/experiments/{id}/protocol`, the protocol canvas, and everything that used to be on a - static `ExperimentDetailPage` now lives in that canvas's `ExperimentSidePanel` tabs - (Design/Cells/Runs/Results) or its top bar. `/experiments/{id}` is kept only as a redirect - for old bookmarks. Don't reintroduce a static detail page; a new per-experiment view is a - new tab in that panel. The panel is drag-resizable by its right edge (320px–1100px, always - leaving the canvas ≥420px) and remembers its width in `localStorage` — so a new tab in it - should be width-responsive via container queries, not built for one fixed column width. - -## Agent skills - -### Issue tracker - -Implementation issues and specs live in the separate `EpistasisLab/ASAREE_Issues` GitHub repository. See `docs/agents/issue-tracker.md`. - -### Triage labels - -Use the five canonical Matt Pocock triage labels. See `docs/agents/triage-labels.md`. - -### Domain docs - -This is a single-context repository with `CONTEXT.md` at the root and private local ADRs under `docs/adr/`. See `docs/agents/domain.md`. - -# Git commit conventions - -Do not add `Co-Authored-By` or `Generated-with` lines to commits or PRs. - -# Cost-conscious execution - -Favor the cheapest tool/approach that reliably gets the job done. Before doing any of the -following, say in one sentence what you're about to do and why it's needed, and ask first -rather than doing it silently as a routine part of coding: - -- **No Playwright/browser automation or screenshot-based UI verification.** The user checks - UI changes visually themselves once you report them done — see the standing preference to - skip Playwright verification agents. Screenshots are read as images, which cost far more - tokens than text, and driving a browser adds many extra tool round-trips on top. Verify with - `tsc`/`oxlint`/existing tests instead; if something genuinely can't be confirmed without a - browser, say so explicitly rather than skipping verification or reaching for Playwright - unasked. -- **No multi-agent fan-out for routine coding work** — don't invoke the `Workflow` tool or - spawn multiple subagents in parallel just because a task touches several files. Each spawned - agent carries its own multiplied context/token cost. Default to working solo, or at most one - scoped `Explore`/`general-purpose` agent for a targeted search. Reserve `Workflow`/parallel - fan-out for when the user explicitly asks for that scale. -- **No unscoped, repo-wide exploration** ("search everywhere," reading many files in full) when - the request already names specific files or areas — grep/read narrowly first, and only widen - the search if that comes up empty. -- **No speculative full test-suite runs or long builds** — run the narrowest test/lint command - that covers the change under review; ask before running something that takes minutes or spins - up extra infrastructure (dev servers, containers, etc.). - -# Experiment data model - -- **Datasets are a real, stored many-to-many** — the `experiment_datasets` join table - (`models/experiment_dataset.py`, migration `d5a3b90c71e4`), unlike agents below: a dataset - genuinely is a first-class property of an experiment worth a real relationship, matching - what ARES does. It started as a scalar `ResearchExperiment.dataset_id` FK (migration - `a1b2c3d4e5f6`); that column is **dropped** — uncapping the Dataset connector means one - experiment can run against several, so a single winner would have been a lie. Read/write it - through `services/experiments.py`'s `set_experiment_datasets` / - `get_experiment_dataset_ids` / `get_dataset_ids_by_experiment` (the batch one — use it on - list endpoints to avoid an N+1), never by touching the model. - - `position` on the join row preserves **canvas wiring order**, which is the order the - agent's prompt lists the datasets in, so it's user-visible, not cosmetic. - - `dataset_ids` is a **full replacement**, not a merge (`[]` detaches everything). The API - and SDK still accept and return the old scalar `dataset_id`: on write it's the - one-dataset shorthand, on read a view of `dataset_ids[0]`. Don't add a second source of - truth — the join table is it. - - It's set via `PATCH /experiments/{id}`, not at creation: the notebook's Step 1 (create the - experiment) runs *before* Step 2 (register the dataset), so there's nothing to attach yet - at create time — see the `client.experiments.update(...)` call after Step 2 in - `spinal_pipeline.ipynb`. In the GUI it's `ProtocolCanvas.tsx`'s `syncExperimentDatasets` - effect, which PATCHes whenever the canvas's set of Dataset nodes changes. That effect's - ref is deliberately **seeded from the graph as loaded so it never fires on mount** — a - mount-time "reconcile" would see zero Dataset nodes on a notebook-driven experiment and - detach its dataset. Keep that property if you touch it. - - An experiment with no datasets attached is an expected, permanent state (everything - created before the FK existed) — not a bug to backfill. -- **Agents are deliberately NOT a stored relationship** — there's no `experiment_agents` join - table, and none is planned. Agents are reusable per-user templates, not something an - experiment "owns"; asking "which agents ran in this experiment" is answered by scanning the - user's `Run`s for `run_metadata.experiment_id` matches (see `ExperimentAgents` in - `components/protocol/RunsTab.tsx`) and cross-referencing `GET /agents`, not by a new backend - association. `GET /runs` has no server-side `experiment_id` filter (only `agent_id`) — this - fetches every run for the user and filters client-side, which is fine at today's scale but - is the place to add a real filter if a user's run history grows large enough to matter. -- **Cells belong to a design revision, not to the experiment** — `experiment_design_revisions` - (`models/experiment_design_revision.py`, migration `e2f7c4a91b60`). The current design is the - one revision with `superseded_at IS NULL` (a partial unique index enforces "at most one", - rather than convention). This exists because generation used to be purely additive: a design - shrunk from 6 cells to 2 left all 6 behind, so the experiment still read "0/6 scored" and - "run all cells" still launched 6. - - **Never query `FactorialReplicateResult` without joining its owning cell** — that loses - experiment/design-revision scope. Go through - `services/factorial_cells.py`'s `get_replicate`/`list_replicates`/`upsert_replicate`, which scope to the - current revision by default and take an explicit `revision_id` only to read history on - purpose. `experiment_id` lives on the parent cell, and both experiment and revision filters - are applied together because `revision_id` can arrive from a query string. - - **Cell vs. replicate:** a `FactorialCell` is one unique factor combination together - with all its planned `FactorialReplicateResult` children. Group replicate responses by `cell_id`, - and use “replicate” for run/progress/scored counts. “Run all cells” is the experiment-level - action that runs every pending replicate across those cells. - - `generate_design_cells` opens a new revision **only when the new design would drop a cell - the current one has**. Re-clicking generate, changing the seed, widening a factor's levels - or raising `replicates` all keep the current revision and its row ids — history entries are - meant to mark designs that actually discarded something, not every edit. Results for a - label the two revisions share are copied forward; the originals stay in history. - - `ProtocolRun.design_revision_id` **pins** which design a run's result belongs to, so a - regenerate mid-flight can't redirect the write-back (`plan_cell_runs` → - `run_protocol`/`promote_cell_score_metrics`). It's `ondelete="SET NULL"` — run history - outlives the design; cells are `ondelete="CASCADE"` so deleting a revision really does - delete its results. - - Deleting the **current** revision is refused (409) — that's a reset, not a deletion; - regenerating is how you replace it. Revision numbers are `max()+1` over every revision ever, - so a deleted number is never reused. The history UI is `components/protocol/cells/ - DesignHistory.tsx`. -- **A protocol canvas is a draft; production runs use a published revision** — `Protocol.graph` - remains the autosaved draft while `protocol_revisions` stores immutable graph snapshots. A - `ProtocolRun` carries both `design_revision_id` and `protocol_revision_id`; `run_protocol` - loads the latter, so a canvas edit can never hot-patch queued, running, or resumed work. Never - create a production run from `Protocol.graph` directly: publish first and pass the resulting - revision through the planning path. The top bar deliberately says when a visible canvas has - unpublished changes and which published revision production currently uses. -- **A declared factor must bind to at least one canvas field before a cell batch can run** — - deleting/unbinding a node field leaves the factor declared but *unbound*, rather than silently - deleting the experimental treatment. The user must rebind it or remove it, then review the - design impact and regenerate. `services.factor_bindings` is the shared backend guard; the - Design tab shows the same state before generation. - -# Color — meaningful variation, not decoration - -A single cyan accent everywhere read as flat/monotone rather than retrofuturist (that style -leans on multi-hue neon contrast — synthwave, Tron, Blade Runner — not minimalism). Cards can -now be individually re-tinted, but the tint must mean something; don't add color for its own -sake. - -- **Mechanism**: `components/ui/card.tsx`'s `Card` reads a `--card-accent` CSS custom property - (falling back to `--primary`) for its ring/glow/corner-bracket colors — see the comment on - `Card` itself. Set it via `style={cardAccent('var(--chart-3)')}` (`lib/utils.ts`) on any - individual `Card`; anything that also wants the icon/badge/hover-arrow to match reads - `text-[color:var(--card-accent,var(--primary))]` the same way `AgentCard` and the Experiments - tiles do — don't hardcode `text-primary` on elements living inside a re-tintable card. -- **Status-driven tint** (`lib/experiment.ts`'s `cellsStatusAccent`): `--chart-4` (amber) = cells - generated but none scored, `--primary` (cyan) = partially scored, `--chart-3` (emerald) = fully - scored, `--muted-foreground` (dim) = no cells yet. Used on both the Experiments-list tiles and - the detail page's "Cells" stat card — the same status must always resolve to the same color - everywhere, so change it in that one function, not per call site. -- **Hash-driven tint** (`lib/utils.ts`'s `hashToChartHue`): for things with no "done/pending" - status but real category variety — e.g. `AgentCard` tints by `model_config.model`, so agents - sharing an LLM visually match without a hardcoded model→color table that goes stale the moment - a new model ships. Same input always produces the same one of the five `--chart-*` hues. - **Not for protocol-canvas nodes** — see the table below. -- **Table-driven tint** (`lib/nodeAccent.ts`'s `nodeAccent(kind)`): protocol-canvas nodes only. - Thirteen node kinds against five `--chart-*` hues made collisions arithmetic, and they landed - on the confusable pairs (Skill/AI, Dataset/Knowledge, Pattern/Script), so every kind now has an - explicit entry. Five keep the `--chart-*` hue the old hash gave them (agent, dataset, - reason+act, critic gate, and the LLM family) and the other eight moved to a `--node-1`…`--node-8` - slot in `index.css`'s `.dark` block — **fixing a repeat means moving the bucket-mates, not - repainting the canvas**, so don't reassign an anchor's hue without being asked. Both the node - card and its inspector call `nodeAccent` with the same key so they can't drift; a kind with no - entry falls back to `--primary` rather than a hashed hue, so a new node type visibly asks for a - slot. Adjacent hues are assigned to related kinds on purpose (the two MCP kinds, the two OKF - kinds). Note LLM nodes are one hue for the whole family, not one per provider. - - `--node-label` (yellow) is separate from all of them: it's the connector captions on - `AgentNode`/`CriticGateNode`, which are meant to stand out *from* their node, so they - deliberately don't follow `--card-accent` the way everything else inside a card does. -- **Don't hash/rotate a tint just to break up visual monotony** with no underlying meaning (e.g. - cycling colors by array index) — that was considered and rejected in favor of the schemes - above. If a new list of things genuinely has no status or category worth encoding in color, - it's fine for it to stay a single accent. - -# Communication style - -Be concise and execution-oriented. - -For coding tasks: -- Do the requested work without lengthy explanations. -- Make routine implementation decisions yourself. -- Do not present multiple alternatives unless the choice materially affects - architecture, correctness, or requirements. -- Do not enumerate pros and cons for routine decisions. -- Do not explain obvious code changes. -- Do not repeatedly summarize what you are doing. -- Ask questions only when ambiguity would materially change the implementation. -- Prefer implementing over discussing. -- After completing a task, give a short summary of what changed. -- If you identify an important concern, state it briefly and recommend one - course of action rather than presenting many possibilities. - -Keep responses focused, brief, and concise. Give deeper explanations only -when I explicitly ask for them. +@AGENTS.md diff --git a/frontend/README.md b/frontend/README.md index 2ea8a28..c7e3b8f 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -3,7 +3,7 @@ React 19 + TypeScript + Vite, styled with Tailwind v4 and a small set of local shadcn-style primitives over [Base UI](https://base-ui.com/) (`src/components/ui`). The visual language is deliberate and documented in the repo root's -[`CLAUDE.md`](../CLAUDE.md) — read that before adding a page or component. +[`AGENTS.md`](../AGENTS.md) — read that before adding a page or component. ## How it runs: dev server only, no build step diff --git a/frontend/src/components/LlmConnectionCheck.tsx b/frontend/src/components/LlmConnectionCheck.tsx index 3e1db13..ffafacf 100644 --- a/frontend/src/components/LlmConnectionCheck.tsx +++ b/frontend/src/components/LlmConnectionCheck.tsx @@ -8,7 +8,7 @@ import type { LLMConnectionStatus, LLMProvider } from '@/types/llmSettings' // page's credentials table, the save step in CreateCredentialDialog, and the // LLM node inspector's Credential field. One definition on purpose: the same // status must always resolve to the same color and the same word everywhere -// (the root CLAUDE.md's rule for status-driven tint), and "Key valid" in one +// (the root AGENTS.md's rule for status-driven tint), and "Key valid" in one // place with "Connected" in another would read as two different claims. // // Colors follow the app's status language: emerald for done/good, amber for diff --git a/frontend/src/components/protocol/PythonCodeEditor.tsx b/frontend/src/components/protocol/PythonCodeEditor.tsx index ca15d70..2f0ee2d 100644 --- a/frontend/src/components/protocol/PythonCodeEditor.tsx +++ b/frontend/src/components/protocol/PythonCodeEditor.tsx @@ -11,7 +11,7 @@ import { oneDarkHighlightStyle } from '@codemirror/theme-one-dark' // itself, not its accompanying background theme) is used verbatim for // those -- a code editor's multi-hue token coloring is its own established // visual language, not something this app's single-accent color convention -// (CLAUDE.md's "Color -- meaningful variation, not decoration") is meant to +// (AGENTS.md's "Color -- meaningful variation, not decoration") is meant to // constrain, the same way a syntax-highlighted code block in a chat UI // doesn't reskin its colors to match the surrounding chrome either. const editorTheme = EditorView.theme( diff --git a/frontend/src/components/protocol/cells/AGENTS.md b/frontend/src/components/protocol/cells/AGENTS.md new file mode 100644 index 0000000..75202d7 --- /dev/null +++ b/frontend/src/components/protocol/cells/AGENTS.md @@ -0,0 +1,121 @@ +# Cells view UI details + +Scoped detail for the Cells tab of the protocol canvas's `ExperimentSidePanel` — this only +loads when you're working in this directory. These rules came from the (now-deleted) static +`ExperimentDetailPage`; they survived the move into the canvas unchanged in substance, only +re-sized for the panel column (384px by default, drag-resizable from 320px up). See the root +`AGENTS.md`'s "Tables vs. cards is a +zoom-level decision" principle for the general rule this implements: you're *inside* one +experiment here, comparing dense multi-attribute records, which is exactly where a real table +earns its place. + +- **A real table has real columns, not two key=value string dumps** (`CellsTable.tsx`): a + `Factors`/`Metrics` column rendering `"tier=cloud-standard, effort=medium"` as flat text is + not "a table," it's two text blobs wearing a table's styling — you still have to visually + parse every row to compare anything. `CellsTable` gives one real, independently sortable + column per derived factor (reuses `deriveFactors`) plus up to 4 curated metric columns + (`pickMetricColumns` — same preference order as the heatmap's default metric, capped because + a real scored cell can have 6-7 numeric keys and showing all of them makes the table + unreadably wide). One table row is one true cell (a unique factor combination): its metric + columns are replicate means and its status reports scored/total replicates. The API's legacy + `Replicate` rows are individual observations and must be grouped first via + `groupReplicatesIntoCells`. `table-fixed` isn't used since the column set is dynamic; instead: + `truncate` on free-text cells, a shaded uppercase sortable header row, subtle zebra striping, + `font-mono` on technical values. It's styled to the side panel's own compact table idiom (a + plain `text-xs` `` in a bordered box, like `RunsTab`/`ResultsTab`), not + `components/ui/table`'s page-width rows. The caller (`CellsTab`) owns the one + `overflow-x-auto` wrapper — the column set can outgrow even the maximized overlay. +- **A factorial design gets a heatmap complement above its table, not instead of it** + (`CellsHeatmap.tsx`) — precise numbers still matter, a heatmap just makes the *shape* of a + multi-factor sweep's results scannable at a glance. 1-2 factors render as one grid; 3 facet + into one grid per level of the third. Replicates within each cell are averaged, not just the + first one picked. Color is `color-mix(in oklch, var(--muted), + var(--primary) N%)` (`heatColor` in `lib/experiment.ts`) — low values dim, high values glow + in the theme's own accent, not an unrelated rainbow scale. Its layout is driven by + **container queries** (`@lg`/`@2xl`, with `CellsTab`'s `CellsBody` marking the `@container`), + not by the viewport and not by a compact/roomy prop — it renders both inside the + drag-resizable panel and in the full-viewport overlay, so "how much room is there" is a + question about the box, and dragging the panel wider has to pay off without a width threaded + through three components. Keep it that way if you add more responsive behaviour here. + - **Both axes (factors) and color (metric) are derived from the cells' own data, with + zero setup required** (`deriveFactors`/`availableMetricKeys`/`pickDefaultMetric` in + `lib/experiment.ts`) — NOT from `design_spec.factors`/`task_brief.selection_metric`. + Those two fields are only ever optional hints for a nicer default (`design_spec.factors` + supplies deliberate level *ordering*, and levels that are planned but have no cell yet; + `task_brief.selection_metric` only picks which observed metric key is pre-selected). Do + not make either a hard requirement again: `task_brief` is a notebook-local variable + embedded straight into agent prompts, not something the backend record has — nearly every + real experiment has both fields `null`, and requiring them would mean the heatmap never + renders for anyone who didn't know to separately attach this metadata. Factors are derived + from the union of every cell's `factor_values` keys, excluding a small bookkeeping + blocklist (`replicate`/`seed`/`rep`/`trial`/`iteration`) so a replicate index doesn't + become a spurious extra axis. + - **Which factors exist is decided by the cells, never by `design_spec.factors`** — this + is the bug that made the heatmap "disappear" once already, so don't undo it. + `deriveFactors` used to return a declared spec verbatim whenever one existed. Real data + here has both conventions: the *Myocardial Infarction* experiment's cells are keyed by + the declared display names (`Azure Foundry:Model`, `Critic enabled`), while the *Spinal + Fusion* one declares those same-style names but its cells are keyed `tier`/`effort`/ + `critic` from the design generator. Nothing keeps the two in sync. When they diverged, + every `replicatesMatching()` lookup matched 0 of 80 replicates, every square came out `null`, and + the grid vanished with no error in the console, no failing type, and no visible clue — + it looked exactly like a broken component. A declared spec now only contributes level + order and planned-but-unrun levels for names the cells actually use. The metric selector only shows when there's more than one + numeric key to choose from. For >3 derived factors, <2 cells, or no numeric metric + anywhere on the cells at all it draws no grid — those are exactly the cases where it + can't show anything the table doesn't already say better — but it says so in a one-line + `HeatmapUnavailable` note rather than rendering `null`. That's deliberate and worth + keeping: it used to bail silently, and a heatmap that can disappear for five different + data-shaped reasons without naming one is indistinguishable from a heatmap that's broken. + Route any new bail-out through that component instead of returning `null`. + - **The grid has to survive a 320px column** — it was written for a ~1024px page and moved + into the panel, where `grid-template-columns: auto repeat(N, 1fr)` overflowed: a grid + won't shrink an `auto` track below its content, so one long factor level name (a model + id, a dict-ish value) sized the label track past the panel's whole width and pushed every + square out through the `Card`'s `overflow-hidden` edge — the heatmap read as *missing*, + not as squeezed. It's now `fit-content(7rem) repeat(N, minmax(2.5rem, 1fr))` inside an + `overflow-x-auto` box, with `overflow-hidden` + `truncate` on the label cells (that pair + is what drops their min-content contribution to zero and lets the 7rem cap actually + bind). Don't reintroduce a content-sized track or drop the per-square minimum. +- **Cells/factor_values/metric_values are a `design_type === 'factorial'` concept, not a + universal one** — `CellsTab` and the canvas top bar's own cells/best-metric chips are both + gated behind that check. `design_type` is a plain string specifically so another experiment + type could exist later (ab_experiments, discoveries, etc. are explicitly out of scope on + `ResearchExperiment` itself, not something the frontend invented) — a non-factorial + experiment gets an explicit "Cell-based results aren't available for '{type}' experiments" + line instead of an empty/broken cells view. There is no other experiment type implemented + anywhere in ASAREE today, so this can't be exercised with real data yet — don't take that as + a reason to remove the guard; it's cheap insurance for a boundary ASAREE's own backend + already declared. +- **A dense view that outgrows the panel earns a "maximize" toggle** (`CellsTab`'s `fullscreen` + state, same as `ResultsTab`'s) — a fixed `inset-0 z-50` overlay with an Escape-key handler + and a close button, NOT the browser Fullscreen API (`requestFullscreen()` hides browser + chrome entirely, a bigger commitment than "let me read this table" calls for). Lock + `document.body` scroll while open and restore it on close/unmount. The table is mounted in + exactly one of the two places at a time, never duplicated with its own divergent sort/page + state. Reach for this pattern again for any other panel view that can't fit — don't + reintroduce the Fullscreen API for the same job. +- **The panel-vs-tab split**: Cells is the raw "what did each configuration score" grid; + `ResultsTab` is the statistical analysis computed *on* those numbers (effects, CIs, + non-inferiority). Keep them separate — don't fold cells into Results. +- **Agent lists aren't a table either** — but they're no longer a card grid either. The old + detail page's `AgentsSection` card grid became `ExperimentAgents` in `RunsTab.tsx`: a + compact one-row-per-agent list (icon tinted by model hash, name, model badge, run count, + relative last-used) since it's a footnote to the trial table in a narrow column, not its own + page section. The model-hash tint is the part that carries meaning and must survive any + further restyling; don't go back to ARES's plain name/role/added thin table either. +- **Design history is a collapsed footnote, not a peer of the table** (`DesignHistory.tsx`): + regenerating a design that would drop cells supersedes the whole revision rather than editing + or deleting the old cells (see the root `AGENTS.md`'s "Cells belong to a design revision"), + so an experiment quietly accumulates results the current view doesn't show. The list renders + **only once there are ≥2 revisions** — a permanent "1 revision" row is noise in a 320px + column, and a single current revision is just "the design", which is already the whole rest + of the tab. Selecting a superseded revision swaps the heatmap/table onto its cells with an + explicit read-only banner; that selection lives in `CellsTab` so the tally, the CSV button and + the grid can't disagree about which design they're showing, and a superseded revision gets its + own query key so it never overwrites the `['experiments', id, 'cells']` entry the canvas top + bar and `DesignTab` share. Deleting one is permanent and cascades to its results, so it goes + through a `Dialog` that names the counts (the `DeleteNodeConfirmDialog` convention), not the + lighter inline two-click confirm — and the current revision has no delete affordance at all, + since the server refuses it (409). +- Paginate client-side once a list can realistically exceed ~10-20 rows. diff --git a/frontend/src/components/protocol/cells/CLAUDE.md b/frontend/src/components/protocol/cells/CLAUDE.md index 2fa84a5..43c994c 100644 --- a/frontend/src/components/protocol/cells/CLAUDE.md +++ b/frontend/src/components/protocol/cells/CLAUDE.md @@ -1,121 +1 @@ -# Cells view UI details - -Scoped detail for the Cells tab of the protocol canvas's `ExperimentSidePanel` — this only -loads when you're working in this directory. These rules came from the (now-deleted) static -`ExperimentDetailPage`; they survived the move into the canvas unchanged in substance, only -re-sized for the panel column (384px by default, drag-resizable from 320px up). See the root -`CLAUDE.md`'s "Tables vs. cards is a -zoom-level decision" principle for the general rule this implements: you're *inside* one -experiment here, comparing dense multi-attribute records, which is exactly where a real table -earns its place. - -- **A real table has real columns, not two key=value string dumps** (`CellsTable.tsx`): a - `Factors`/`Metrics` column rendering `"tier=cloud-standard, effort=medium"` as flat text is - not "a table," it's two text blobs wearing a table's styling — you still have to visually - parse every row to compare anything. `CellsTable` gives one real, independently sortable - column per derived factor (reuses `deriveFactors`) plus up to 4 curated metric columns - (`pickMetricColumns` — same preference order as the heatmap's default metric, capped because - a real scored cell can have 6-7 numeric keys and showing all of them makes the table - unreadably wide). One table row is one true cell (a unique factor combination): its metric - columns are replicate means and its status reports scored/total replicates. The API's legacy - `Replicate` rows are individual observations and must be grouped first via - `groupReplicatesIntoCells`. `table-fixed` isn't used since the column set is dynamic; instead: - `truncate` on free-text cells, a shaded uppercase sortable header row, subtle zebra striping, - `font-mono` on technical values. It's styled to the side panel's own compact table idiom (a - plain `text-xs` `
` in a bordered box, like `RunsTab`/`ResultsTab`), not - `components/ui/table`'s page-width rows. The caller (`CellsTab`) owns the one - `overflow-x-auto` wrapper — the column set can outgrow even the maximized overlay. -- **A factorial design gets a heatmap complement above its table, not instead of it** - (`CellsHeatmap.tsx`) — precise numbers still matter, a heatmap just makes the *shape* of a - multi-factor sweep's results scannable at a glance. 1-2 factors render as one grid; 3 facet - into one grid per level of the third. Replicates within each cell are averaged, not just the - first one picked. Color is `color-mix(in oklch, var(--muted), - var(--primary) N%)` (`heatColor` in `lib/experiment.ts`) — low values dim, high values glow - in the theme's own accent, not an unrelated rainbow scale. Its layout is driven by - **container queries** (`@lg`/`@2xl`, with `CellsTab`'s `CellsBody` marking the `@container`), - not by the viewport and not by a compact/roomy prop — it renders both inside the - drag-resizable panel and in the full-viewport overlay, so "how much room is there" is a - question about the box, and dragging the panel wider has to pay off without a width threaded - through three components. Keep it that way if you add more responsive behaviour here. - - **Both axes (factors) and color (metric) are derived from the cells' own data, with - zero setup required** (`deriveFactors`/`availableMetricKeys`/`pickDefaultMetric` in - `lib/experiment.ts`) — NOT from `design_spec.factors`/`task_brief.selection_metric`. - Those two fields are only ever optional hints for a nicer default (`design_spec.factors` - supplies deliberate level *ordering*, and levels that are planned but have no cell yet; - `task_brief.selection_metric` only picks which observed metric key is pre-selected). Do - not make either a hard requirement again: `task_brief` is a notebook-local variable - embedded straight into agent prompts, not something the backend record has — nearly every - real experiment has both fields `null`, and requiring them would mean the heatmap never - renders for anyone who didn't know to separately attach this metadata. Factors are derived - from the union of every cell's `factor_values` keys, excluding a small bookkeeping - blocklist (`replicate`/`seed`/`rep`/`trial`/`iteration`) so a replicate index doesn't - become a spurious extra axis. - - **Which factors exist is decided by the cells, never by `design_spec.factors`** — this - is the bug that made the heatmap "disappear" once already, so don't undo it. - `deriveFactors` used to return a declared spec verbatim whenever one existed. Real data - here has both conventions: the *Myocardial Infarction* experiment's cells are keyed by - the declared display names (`Azure Foundry:Model`, `Critic enabled`), while the *Spinal - Fusion* one declares those same-style names but its cells are keyed `tier`/`effort`/ - `critic` from the design generator. Nothing keeps the two in sync. When they diverged, - every `replicatesMatching()` lookup matched 0 of 80 replicates, every square came out `null`, and - the grid vanished with no error in the console, no failing type, and no visible clue — - it looked exactly like a broken component. A declared spec now only contributes level - order and planned-but-unrun levels for names the cells actually use. The metric selector only shows when there's more than one - numeric key to choose from. For >3 derived factors, <2 cells, or no numeric metric - anywhere on the cells at all it draws no grid — those are exactly the cases where it - can't show anything the table doesn't already say better — but it says so in a one-line - `HeatmapUnavailable` note rather than rendering `null`. That's deliberate and worth - keeping: it used to bail silently, and a heatmap that can disappear for five different - data-shaped reasons without naming one is indistinguishable from a heatmap that's broken. - Route any new bail-out through that component instead of returning `null`. - - **The grid has to survive a 320px column** — it was written for a ~1024px page and moved - into the panel, where `grid-template-columns: auto repeat(N, 1fr)` overflowed: a grid - won't shrink an `auto` track below its content, so one long factor level name (a model - id, a dict-ish value) sized the label track past the panel's whole width and pushed every - square out through the `Card`'s `overflow-hidden` edge — the heatmap read as *missing*, - not as squeezed. It's now `fit-content(7rem) repeat(N, minmax(2.5rem, 1fr))` inside an - `overflow-x-auto` box, with `overflow-hidden` + `truncate` on the label cells (that pair - is what drops their min-content contribution to zero and lets the 7rem cap actually - bind). Don't reintroduce a content-sized track or drop the per-square minimum. -- **Cells/factor_values/metric_values are a `design_type === 'factorial'` concept, not a - universal one** — `CellsTab` and the canvas top bar's own cells/best-metric chips are both - gated behind that check. `design_type` is a plain string specifically so another experiment - type could exist later (ab_experiments, discoveries, etc. are explicitly out of scope on - `ResearchExperiment` itself, not something the frontend invented) — a non-factorial - experiment gets an explicit "Cell-based results aren't available for '{type}' experiments" - line instead of an empty/broken cells view. There is no other experiment type implemented - anywhere in ASAREE today, so this can't be exercised with real data yet — don't take that as - a reason to remove the guard; it's cheap insurance for a boundary ASAREE's own backend - already declared. -- **A dense view that outgrows the panel earns a "maximize" toggle** (`CellsTab`'s `fullscreen` - state, same as `ResultsTab`'s) — a fixed `inset-0 z-50` overlay with an Escape-key handler - and a close button, NOT the browser Fullscreen API (`requestFullscreen()` hides browser - chrome entirely, a bigger commitment than "let me read this table" calls for). Lock - `document.body` scroll while open and restore it on close/unmount. The table is mounted in - exactly one of the two places at a time, never duplicated with its own divergent sort/page - state. Reach for this pattern again for any other panel view that can't fit — don't - reintroduce the Fullscreen API for the same job. -- **The panel-vs-tab split**: Cells is the raw "what did each configuration score" grid; - `ResultsTab` is the statistical analysis computed *on* those numbers (effects, CIs, - non-inferiority). Keep them separate — don't fold cells into Results. -- **Agent lists aren't a table either** — but they're no longer a card grid either. The old - detail page's `AgentsSection` card grid became `ExperimentAgents` in `RunsTab.tsx`: a - compact one-row-per-agent list (icon tinted by model hash, name, model badge, run count, - relative last-used) since it's a footnote to the trial table in a narrow column, not its own - page section. The model-hash tint is the part that carries meaning and must survive any - further restyling; don't go back to ARES's plain name/role/added thin table either. -- **Design history is a collapsed footnote, not a peer of the table** (`DesignHistory.tsx`): - regenerating a design that would drop cells supersedes the whole revision rather than editing - or deleting the old cells (see the root `CLAUDE.md`'s "Cells belong to a design revision"), - so an experiment quietly accumulates results the current view doesn't show. The list renders - **only once there are ≥2 revisions** — a permanent "1 revision" row is noise in a 320px - column, and a single current revision is just "the design", which is already the whole rest - of the tab. Selecting a superseded revision swaps the heatmap/table onto its cells with an - explicit read-only banner; that selection lives in `CellsTab` so the tally, the CSV button and - the grid can't disagree about which design they're showing, and a superseded revision gets its - own query key so it never overwrites the `['experiments', id, 'cells']` entry the canvas top - bar and `DesignTab` share. Deleting one is permanent and cascades to its results, so it goes - through a `Dialog` that names the counts (the `DeleteNodeConfirmDialog` convention), not the - lighter inline two-click confirm — and the current revision has no delete affordance at all, - since the server refuses it (409). -- Paginate client-side once a list can realistically exceed ~10-20 rows. +@AGENTS.md diff --git a/frontend/src/components/protocol/cells/CellsTab.tsx b/frontend/src/components/protocol/cells/CellsTab.tsx index a648774..8f8da80 100644 --- a/frontend/src/components/protocol/cells/CellsTab.tsx +++ b/frontend/src/components/protocol/cells/CellsTab.tsx @@ -46,7 +46,7 @@ function CellsBody({ experiment, cells }: { experiment: Experiment; cells: Repli * experiment has cells, so a future non-factorial type gets an explicit * "not available" line instead of an empty, broken-looking grid. * - * Maximize is CLAUDE.md's established convention for a dense view that + * Maximize is AGENTS.md's established convention for a dense view that * outgrows the panel's column (same as ResultsTab): a fixed inset-0 overlay * with an Escape handler and a body-scroll lock, never the browser Fullscreen * API. The table is mounted in exactly one of the two places at a time, so diff --git a/frontend/src/components/protocol/nodes/NodeFactorBadge.tsx b/frontend/src/components/protocol/nodes/NodeFactorBadge.tsx index faae3c7..7e2d717 100644 --- a/frontend/src/components/protocol/nodes/NodeFactorBadge.tsx +++ b/frontend/src/components/protocol/nodes/NodeFactorBadge.tsx @@ -15,7 +15,7 @@ import { Split } from 'lucide-react' // // Violet (`--chart-2`), not the host node's `--card-accent`: violet is already // this app's factor hue (FACTOR_TRIGGER_CLASSNAME's own button, the factor -// editor dialog), and CLAUDE.md's colour rule wants a tint to mean something +// editor dialog), and AGENTS.md's colour rule wants a tint to mean something // -- here it means "factor", the one thing this badge is for, so it must not // dissolve into whatever hue the node underneath happens to be. // diff --git a/frontend/src/pages/AGENTS.md b/frontend/src/pages/AGENTS.md new file mode 100644 index 0000000..5a8c3a7 --- /dev/null +++ b/frontend/src/pages/AGENTS.md @@ -0,0 +1,30 @@ +# Experiments-list page UI details + +Scoped detail for `ExperimentsPage.tsx` — this only loads when you're working in this +directory. See the root `AGENTS.md`'s "Tables vs. cards is a zoom-level decision, not a style +preference" principle for the general rule this detail implements: a "container" list — sparse +metadata, click one to drill into it (e.g. the Experiments list) — is a card/tile grid, not a +table; a table only earns its place once you're inside that container looking at dense, +precise, multi-attribute records to compare (e.g. an experiment's Cells, with their +metrics/hyperparameters). + +- **Tile grids** (`ExperimentsPage.tsx`): `grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 + gap-4` of `Card`s, each with a truncated title + `line-clamp-3` description (`title=` + attribute carries the full text on hover, since the card stays compact rather than + growing to fit long text — the full description belongs one click in) + a small metadata + badge/date — not a dense data dump. Client-side sort via a `Select` + direction + toggle (column-header sort doesn't exist once there are no columns). Don't add a KPI + summary strip above the grid unless there's a real aggregate to show (e.g. cells + scored) — a stat card restating the list's own item count, or one that just repeats a + field already visible on every tile (design type, most-recent item), isn't a summary, + it's noise. Per-tile polish that *is* worth it: a design-type icon next to its badge, a + thin glowing top accent strip, relative time (`"2h ago"`, exact date on hover via + `title`), a short `font-mono` id fragment, a very low-opacity decorative watermark icon, + and a hover lift (`scale-[1.02]` + intensified glow + the trailing arrow shifting to + the card's accent color). +- **A tile click goes to the protocol canvas** (`/experiments/{id}/protocol`), not to a + static detail page — there isn't one, and `/experiments/{id}` exists only as a redirect + (see `App.tsx`). Everything about a single experiment lives in the canvas's side-panel + tabs; the rules for the Cells heatmap/table specifically are in + `components/protocol/cells/AGENTS.md`. +- Paginate client-side once a list can realistically exceed ~10-20 tiles. diff --git a/frontend/src/pages/CLAUDE.md b/frontend/src/pages/CLAUDE.md index 347a024..43c994c 100644 --- a/frontend/src/pages/CLAUDE.md +++ b/frontend/src/pages/CLAUDE.md @@ -1,30 +1 @@ -# Experiments-list page UI details - -Scoped detail for `ExperimentsPage.tsx` — this only loads when you're working in this -directory. See the root `CLAUDE.md`'s "Tables vs. cards is a zoom-level decision, not a style -preference" principle for the general rule this detail implements: a "container" list — sparse -metadata, click one to drill into it (e.g. the Experiments list) — is a card/tile grid, not a -table; a table only earns its place once you're inside that container looking at dense, -precise, multi-attribute records to compare (e.g. an experiment's Cells, with their -metrics/hyperparameters). - -- **Tile grids** (`ExperimentsPage.tsx`): `grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 - gap-4` of `Card`s, each with a truncated title + `line-clamp-3` description (`title=` - attribute carries the full text on hover, since the card stays compact rather than - growing to fit long text — the full description belongs one click in) + a small metadata - badge/date — not a dense data dump. Client-side sort via a `Select` + direction - toggle (column-header sort doesn't exist once there are no columns). Don't add a KPI - summary strip above the grid unless there's a real aggregate to show (e.g. cells - scored) — a stat card restating the list's own item count, or one that just repeats a - field already visible on every tile (design type, most-recent item), isn't a summary, - it's noise. Per-tile polish that *is* worth it: a design-type icon next to its badge, a - thin glowing top accent strip, relative time (`"2h ago"`, exact date on hover via - `title`), a short `font-mono` id fragment, a very low-opacity decorative watermark icon, - and a hover lift (`scale-[1.02]` + intensified glow + the trailing arrow shifting to - the card's accent color). -- **A tile click goes to the protocol canvas** (`/experiments/{id}/protocol`), not to a - static detail page — there isn't one, and `/experiments/{id}` exists only as a redirect - (see `App.tsx`). Everything about a single experiment lives in the canvas's side-panel - tabs; the rules for the Cells heatmap/table specifically are in - `components/protocol/cells/CLAUDE.md`. -- Paginate client-side once a list can realistically exceed ~10-20 tiles. +@AGENTS.md diff --git a/src/asaree/models/dataset.py b/src/asaree/models/dataset.py index ab61082..cedc714 100644 --- a/src/asaree/models/dataset.py +++ b/src/asaree/models/dataset.py @@ -11,7 +11,7 @@ ``register_manual_split``), so ``train_path``/``test_path`` are nullable: a freshly-registered dataset has a raw file and no split yet, same as an experiment created before the ``dataset_id`` FK existed permanently has -``dataset_id: null`` (see CLAUDE.md's own Experiment data model section) — +``dataset_id: null`` (see AGENTS.md's own Experiment data model section) — not a bug to backfill, just a real, valid state. This split-off-registration design is deliberate, not an oversight: scientific splitting needs vary per experiment (stratified holdout, group-aware holdout, k-fold, time-based, @@ -47,7 +47,7 @@ class RegisteredDataset(Base, TimestampMixin): # re-derived. The one thing registration itself is responsible for. # Nullable purely for a dataset registered before this column existed # (this concept didn't exist yet, so there's nothing to backfill it - # from -- same "permanent, valid null" reasoning CLAUDE.md's own + # from -- same "permanent, valid null" reasoning AGENTS.md's own # Experiment data model section gives for a pre-migration # dataset_id) -- every dataset registered from here on always has one. raw_path: Mapped[str | None] = mapped_column(Text, nullable=True) diff --git a/src/asaree/models/experiment_dataset.py b/src/asaree/models/experiment_dataset.py index 2e9e851..b04ef8c 100644 --- a/src/asaree/models/experiment_dataset.py +++ b/src/asaree/models/experiment_dataset.py @@ -9,7 +9,7 @@ itself changed shape and the column was dropped (migration ``d5a3b90c71e4``, which backfills one row per existing non-null ``dataset_id``). -Deliberately NOT the same call as CLAUDE.md's "agents are not a stored +Deliberately NOT the same call as AGENTS.md's "agents are not a stored relationship". An experiment's datasets are answerable only from the canvas graph otherwise, and unlike agents (reusable per-user templates that an experiment merely borrows) a dataset is part of what the experiment IS -- diff --git a/src/asaree/services/csv_export.py b/src/asaree/services/csv_export.py index 0fbc846..07c1dd3 100644 --- a/src/asaree/services/csv_export.py +++ b/src/asaree/services/csv_export.py @@ -3,7 +3,7 @@ One row per replicate; one column per factor_values/metric_values key seen across all replicates -- unlike the Cells table UI's own pickMetricColumns, which caps displayed metric columns at 4 for on-screen readability (frontend/src/pages/ -CLAUDE.md), a CSV has no such density constraint, so every key is included. +AGENTS.md), a CSV has no such density constraint, so every key is included. """ from __future__ import annotations From c4e187b33564b907deec1143713e74fb1bea0bff Mon Sep 17 00:00:00 2001 From: Jay Moran Date: Wed, 23 Sep 2026 14:56:07 -0700 Subject: [PATCH 02/21] feat: add progressive resource disclosure --- .../protocol/ScriptNodeInspector.tsx | 10 + .../src/components/protocol/datasetCatalog.ts | 10 +- .../src/components/protocol/okfCatalog.ts | 4 + frontend/src/types/protocols.ts | 11 +- frontend/src/types/skills.ts | 6 + pyproject.toml | 2 +- src/asaree/api/skills.py | 8 +- src/asaree/services/agent_messenger.py | 22 +- src/asaree/services/dataset_workspaces.py | 1 + src/asaree/services/protocol_execution.py | 250 ++++++++++++++---- src/asaree/services/run_tools.py | 5 + src/asaree/services/skill_sources.py | 44 ++- src/asaree/services/system_mcp_servers.py | 3 + tests/test_protocol_execution.py | 100 ++++++- tests/test_run_tools.py | 14 +- tests/test_skill_sources.py | 19 +- uv.lock | 6 +- 17 files changed, 406 insertions(+), 109 deletions(-) diff --git a/frontend/src/components/protocol/ScriptNodeInspector.tsx b/frontend/src/components/protocol/ScriptNodeInspector.tsx index 16a5687..1af6035 100644 --- a/frontend/src/components/protocol/ScriptNodeInspector.tsx +++ b/frontend/src/components/protocol/ScriptNodeInspector.tsx @@ -81,6 +81,16 @@ export function ScriptNodeInspector({ +
+ + patchConfig({ description: e.target.value })} + placeholder="What this script does and when the agent should run it" + /> +
+ + 'allowed-tools'?: string + } is_system: boolean // The folder the skill was uploaded from, or the .md file for a single-file // upload. Shown back to the user, never used to resolve anything. diff --git a/pyproject.toml b/pyproject.toml index 6ef3686..a9571d6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,7 +94,7 @@ dev = [ ] [tool.uv.sources] -motoro = { git = "https://github.com/EpistasisLab/motoro.git", tag = "v0.6.1" } +motoro = { git = "https://github.com/EpistasisLab/motoro.git", tag = "v0.7.0" } asaree-workspace-core = { path = "workspace-core", editable = true } # Every mcp-servers/ package is declared here, including asaree-sklearn-core # (which the six servers each depend on): uv resolves `tool.uv.sources` from diff --git a/src/asaree/api/skills.py b/src/asaree/api/skills.py index 20e6572..138f209 100644 --- a/src/asaree/api/skills.py +++ b/src/asaree/api/skills.py @@ -117,8 +117,8 @@ async def upload_skill_endpoint( return SkillResponse.model_validate(skill) -async def _folder_payload(files: list[UploadFile]) -> list[tuple[str, str]]: - """``(path relative to the skill folder, text)`` for one directory upload. +async def _folder_payload(files: list[UploadFile]) -> list[tuple[str, bytes]]: + """``(path relative to the skill folder, bytes)`` for one directory upload. The leading ``webkitRelativePath`` segment is the folder the user picked — ``code-simplification/SKILL.md`` — and core's ``parse_skill_bundle`` wants @@ -129,7 +129,7 @@ async def _folder_payload(files: list[UploadFile]) -> list[tuple[str, str]]: the user dragged loose files, and a skill assembled from an unknown directory layout is not the directory they have on disk. """ - payload: list[tuple[str, str]] = [] + payload: list[tuple[str, bytes]] = [] for upload in files: name = upload.filename or "" parts = [p for p in name.replace("\\", "/").split("/") if p] @@ -138,7 +138,7 @@ async def _folder_payload(files: list[UploadFile]) -> list[tuple[str, str]]: status_code=422, detail=f"{name or 'A file'} didn't come from a folder — pick the skill's own folder.", ) - payload.append(("/".join(parts[1:]), _decode(await upload.read()))) + payload.append(("/".join(parts[1:]), await upload.read())) return payload diff --git a/src/asaree/services/agent_messenger.py b/src/asaree/services/agent_messenger.py index c2c43d2..6538a8e 100644 --- a/src/asaree/services/agent_messenger.py +++ b/src/asaree/services/agent_messenger.py @@ -527,7 +527,12 @@ async def _run_peer( # workspace seeded and its `data_path` bound before it can run a script, # exactly like any other node. ambient_meta, dataset = await _node_run_context( - self._graph, to_agent_id, self._workspace_id, self._owner_id, stage_plan=self._stage_plan + self._graph, + to_agent_id, + self._workspace_id, + self._owner_id, + protocol_run_id=self._protocol_run_id, + stage_plan=self._stage_plan, ) # A consultation reply is prose the asking agent reads, never a typed # value anything binds to, so whatever the peer's parser extracted (if @@ -620,7 +625,12 @@ async def execute_conversation( if ambient_meta is None: ambient_meta, entry_dataset = await _node_run_context( - graph, entry_agent_id, workspace_id, owner_id, stage_plan=stage_plan + graph, + entry_agent_id, + workspace_id, + owner_id, + protocol_run_id=protocol_run_id, + stage_plan=stage_plan, ) unsplit_dataset = unsplit_dataset or entry_dataset.unsplit_name @@ -857,7 +867,13 @@ async def _turn( async with get_session() as db: await update_node_run(db, protocol_run_id, node_id, {"status": "running"}) ambient_meta, dataset = await _node_run_context( - graph, node_id, workspace_id, owner_id, slot_prefix=slot_prefix, stage_plan=stage_plan + graph, + node_id, + workspace_id, + owner_id, + protocol_run_id=protocol_run_id, + slot_prefix=slot_prefix, + stage_plan=stage_plan, ) prompt = _build_user_input( nodes[node_id], diff --git a/src/asaree/services/dataset_workspaces.py b/src/asaree/services/dataset_workspaces.py index d45dbb5..98d5e60 100644 --- a/src/asaree/services/dataset_workspaces.py +++ b/src/asaree/services/dataset_workspaces.py @@ -70,6 +70,7 @@ async def fetch_owned_registration(name: str, owner_id: uuid.UUID) -> dict[str, if dataset is None or dataset.owner_id != owner_id: return None return { + "description": dataset.description, "target_column": dataset.target_column, "raw_path": dataset.raw_path, "train_path": dataset.train_path, diff --git a/src/asaree/services/protocol_execution.py b/src/asaree/services/protocol_execution.py index 36a7c5d..a579630 100644 --- a/src/asaree/services/protocol_execution.py +++ b/src/asaree/services/protocol_execution.py @@ -91,6 +91,8 @@ from asaree.services.run_tools import gather_tools from asaree.services.runtime_metrics import finalize_attempt_measurement from asaree.services.system_mcp_servers import ( + DATASET_DICTIONARY_AGENT_TOOLS, + EDA_SERVER_NAME, SCIKIT_LEARN_SERVER_NAME, SCRIPT_AGENT_TOOLS, SCRIPT_SERVER_NAME, @@ -1419,9 +1421,7 @@ def _compute_workspace_id( def _materialize_script(workspace_id: str | None, node_id: str, code: str) -> str: """Write a wired Script node's code next to the run's workspace; return its path. - ``""`` when there's nowhere to put it -- no workspace id (an unlinked - protocol run) or the write failed. The caller falls back to inlining the - code in the prompt, which is what this replaces. + ``""`` when no materialization id was supplied or the write failed. The file lives under the run's own workspace directory because that directory is already the shared surface between this process and the MCP @@ -1450,8 +1450,33 @@ def _materialize_script(workspace_id: str | None, node_id: str, code: str) -> st return str(path) +def _script_workspace_id(workspace_id: str | None, protocol_run_id: uuid.UUID | None, agent_node_id: str) -> str | None: + """Choose real workspace storage or an isolated standalone-run directory.""" + safe_agent = _UNSAFE_WORKSPACE_LABEL_CHAR.sub("_", agent_node_id) + return workspace_id or (f"_protocol_runs/{protocol_run_id}/{safe_agent}" if protocol_run_id else None) + + +def _cleanup_adhoc_scripts(ambient_meta: dict[str, Any] | None) -> None: + """Remove standalone-run script files after their consuming agent stops.""" + adhoc_root = (Path(WORKSPACE_ROOT).resolve() / "_protocol_runs").resolve() + for item in (ambient_meta or {}).get("script_paths") or []: + path = Path(str(item.get("path") or "")).resolve() + if adhoc_root not in path.parents: + continue + with contextlib.suppress(OSError): + path.unlink() + for directory in (path.parent, path.parent.parent, path.parent.parent.parent): + with contextlib.suppress(OSError): + directory.rmdir() + + def _ambient_meta_for( - graph: dict[str, Any], node_id: str, workspace_id: str | None = None, *, slots: tuple[str, ...] = () + graph: dict[str, Any], + node_id: str, + workspace_id: str | None = None, + *, + script_workspace_id: str | None = None, + slots: tuple[str, ...] = (), ) -> dict[str, Any]: """The node's Reference-route values, for Motoro's caller-ambient ``_meta``. @@ -1539,7 +1564,7 @@ def _ambient_meta_for( if not code: continue script_node_id = str(config.get("node_id") or f"script-{index}") - script_path = _materialize_script(workspace_id, script_node_id, str(code)) + script_path = _materialize_script(script_workspace_id or workspace_id, script_node_id, str(code)) if script_path: script_paths.append( { @@ -1654,6 +1679,18 @@ async def _resolve_node_dataset( "workspace_preseed_failed", extra={"node_id": node_id, "dataset": name, "error": "not found"} ) continue + # Enrich this run's in-memory graph with current catalog metadata. The + # published graph still owns identity/order; the registry owns mutable + # descriptive facts, so old nodes gain the same discovery context as + # newly-created ones without rewriting a revision. + for config in _resolve_dataset_configs(graph, node_id): + if str(config.get("dataset_name") or "") == name: + config.update( + description=reg.get("description"), + target_column=reg.get("target_column"), + split_state="split" if reg.get("train_path") and reg.get("test_path") else "unsplit", + dictionary_available=bool(reg.get("dictionary_json")), + ) if not (reg.get("train_path") and reg.get("test_path")): # Unsplit: no workspace to seed (``seed_cell_workspace`` says why), # so the raw file itself becomes the run's dataset and the agent @@ -1708,6 +1745,7 @@ async def _node_run_context( workspace_id: str | None, owner_id: uuid.UUID, *, + protocol_run_id: uuid.UUID | None = None, slot_prefix: str | None = None, stage_plan: Any = None, ) -> tuple[dict[str, Any], NodeDataset]: @@ -1738,6 +1776,7 @@ async def _node_run_context( graph, node_id, workspace_id, + script_workspace_id=_script_workspace_id(workspace_id, protocol_run_id, node_id), slots=tuple(slot for _name, slot in dataset.seeded) if slot_prefix else (), ) if dataset.data_path and "data_path" not in ambient_meta: @@ -2276,6 +2315,7 @@ def _resolve_knowledge_config(graph: dict[str, Any], node_id: str) -> dict[str, nodes, _downstream, _upstream = _adjacency(graph) server_names: list[str] = [] tool_names: list[str] = [] + tool_descriptions: dict[str, str] = {} for edge in _edges_with_handle(graph, node_id, "knowledge", direction="incoming"): source = nodes.get(edge["source"]) if source is None or source.get("type") not in _KNOWLEDGE_NODE_TYPES: @@ -2287,8 +2327,21 @@ def _resolve_knowledge_config(graph: dict[str, Any], node_id: str) -> dict[str, if not server_name or server_name in server_names: continue server_names.append(server_name) - tool_names.extend(f"{server_name}.{name}" for name in bundle_config.get("tool_names") or []) - return {"server_names": server_names, "tool_names": tool_names} + label = str( + bundle_config.get("document_title") + or bundle_config.get("bundle_label") + or (source.get("data") or {}).get("label") + or server_name + ) + summary = str(bundle_config.get("document_description") or bundle_config.get("bundle_description") or "") + for name in bundle_config.get("tool_names") or []: + full_name = f"{server_name}.{name}" + tool_names.append(full_name) + tool_descriptions[full_name] = f"Knowledge source: {label}. {summary}".strip() + resolved: dict[str, Any] = {"server_names": server_names, "tool_names": tool_names} + if tool_descriptions: + resolved["tool_descriptions"] = tool_descriptions + return resolved def _declares_a_field(contract: Any) -> bool: @@ -2357,13 +2410,13 @@ def _resolve_output_contract(graph: dict[str, Any], node_id: str) -> dict[str, A if not parser_config.get("enabled", True): continue contract = parser_config.get("output_contract") - if _declares_a_field(contract): + if isinstance(contract, dict) and _declares_a_field(contract): return dict(contract) if wired: return None node = nodes.get(node_id) or {} legacy = ((node.get("data") or {}).get("config") or {}).get("output_contract") - return dict(legacy) if _declares_a_field(legacy) else None + return dict(legacy) if isinstance(legacy, dict) and _declares_a_field(legacy) else None def _output_shape_block(contract: dict[str, Any] | None) -> str: @@ -2458,6 +2511,9 @@ def _resolve_dataset_tool_config(graph: dict[str, Any], node_id: str, *, unsplit if unsplit_dataset: server_names.append(SCIKIT_LEARN_SERVER_NAME) tool_names.extend(f"{SCIKIT_LEARN_SERVER_NAME}.{name}" for name in UNSPLIT_DATASET_AGENT_TOOLS) + if any(config.get("dictionary_available") for config in _resolve_dataset_configs(graph, node_id)): + server_names.append(EDA_SERVER_NAME) + tool_names.extend(f"{EDA_SERVER_NAME}.{name}" for name in DATASET_DICTIONARY_AGENT_TOOLS) return {"server_names": server_names, "tool_names": tool_names} @@ -2497,6 +2553,7 @@ def _merge_tool_configs(*configs: dict[str, Any]) -> dict[str, Any]: """ server_names: list[str] = [] tool_names: list[str] = [] + tool_descriptions: dict[str, str] = {} for config in configs: for name in config.get("server_names") or []: if name not in server_names: @@ -2504,7 +2561,11 @@ def _merge_tool_configs(*configs: dict[str, Any]) -> dict[str, Any]: for name in config.get("tool_names") or []: if name not in tool_names: tool_names.append(name) - return {"server_names": server_names, "tool_names": tool_names} + tool_descriptions.update(config.get("tool_descriptions") or {}) + resolved: dict[str, Any] = {"server_names": server_names, "tool_names": tool_names} + if tool_descriptions: + resolved["tool_descriptions"] = tool_descriptions + return resolved def _is_node_active(node: dict[str, Any]) -> bool: @@ -2901,6 +2962,72 @@ def validate_prompt_references(*, graph: dict[str, Any]) -> None: ) +def _resource_catalog(graph: dict[str, Any], node_id: str) -> str: + """Compact semantic metadata for references wired into one agent. + + Paths, ids, and source bodies stay out of the prompt. This block gives the + model only enough meaning to choose among already-authorized resources; + native tool schemas remain the interface for reading or executing them. + """ + sections: list[str] = [] + + datasets = _resolve_dataset_configs(graph, node_id) + if datasets: + lines = [] + for config in datasets: + name = str(config.get("dataset_name") or "dataset") + facts = [str(config.get("description") or "").strip()] + if config.get("target_column"): + facts.append(f"target={config['target_column']}") + if config.get("split_state"): + facts.append(f"state={config['split_state']}") + if config.get("dictionary_available"): + facts.append("data dictionary available through an authorized EDA tool") + detail = "; ".join(fact for fact in facts if fact) + lines.append(f"- {name}: {detail}" if detail else f"- {name}") + sections.append("Available datasets:\n" + "\n".join(lines)) + + nodes, _downstream, _upstream = _adjacency(graph) + knowledge_lines: list[str] = [] + for edge in _edges_with_handle(graph, node_id, "knowledge", direction="incoming"): + source = nodes.get(edge["source"]) + if source is None or source.get("type") not in _KNOWLEDGE_NODE_TYPES: + continue + config = (source.get("data") or {}).get("config") or {} + if not config.get("enabled", True): + continue + label = str( + config.get("document_title") + or config.get("bundle_label") + or (source.get("data") or {}).get("label") + or "knowledge source" + ) + facts = [str(config.get("document_description") or config.get("bundle_description") or "").strip()] + if config.get("document_type"): + facts.append(f"type={config['document_type']}") + tags = config.get("document_tags") or [] + if tags: + facts.append("tags=" + ", ".join(str(tag) for tag in tags)) + detail = "; ".join(fact for fact in facts if fact) + knowledge_lines.append(f"- {label}: {detail}" if detail else f"- {label}") + if knowledge_lines: + sections.append( + "Available knowledge sources (use their list/search/get tools to disclose content on demand):\n" + + "\n".join(knowledge_lines) + ) + + scripts = [config for config in _resolve_script_configs(graph, node_id) if config.get("code")] + if scripts: + lines = [] + for index, config in enumerate(scripts, start=1): + name = str(config.get("name") or f"script-{index}") + description = str(config.get("description") or "").strip() + lines.append(f"- {name}: {description}" if description else f"- {name}") + sections.append("Available scripts (source remains out of context until execution):\n" + "\n".join(lines)) + + return "\n\n".join(sections) + + def _build_user_input( node: dict[str, Any], graph: dict[str, Any], @@ -2937,9 +3064,10 @@ def _build_user_input( more than one is wired. *script_bound* says the wired scripts reached ``_meta`` as paths - (``_ambient_meta_for``). When it didn't -- an unlinked protocol run has no - workspace directory to write them to -- the code is inlined here as before, - because a prompt the model can copy from beats no scripts at all. + (``_ambient_meta_for``). Production callers provide either the experiment + workspace or an isolated standalone-run directory, so source stays out of + the prompt in both cases. The false branch is a defensive diagnostic for a + materialization failure. *seeded_datasets* are the ``(dataset name, workspace slot)`` pairs ASAREE already opened on the agent's behalf (``_resolve_node_dataset``). When @@ -2993,6 +3121,10 @@ def _build_user_input( if upstream_context: parts.append(upstream_context) + resource_catalog = _resource_catalog(graph, node["id"]) + if resource_catalog: + parts.append(resource_catalog) + dataset_configs = _resolve_dataset_configs(graph, node["id"]) if dataset_configs and experiment_id is not None and effective_cell_label is not None: dataset_names = [str(c["dataset_name"]) for c in dataset_configs] @@ -3084,7 +3216,7 @@ def _build_user_input( ) else: listed = "\n".join( - f'- {str(config.get("name") or f"script-{index}")!r} (id: {config["node_id"]!r})' + f"- {str(config.get('name') or f'script-{index}')!r} (id: {config['node_id']!r})" for index, config in enumerate(script_configs, start=1) ) parts.append( @@ -3095,25 +3227,12 @@ def _build_user_input( "select by id." ) elif script_configs: - # No workspace directory to write it to (see _materialize_script), so - # fall back to what this did before: paste it and ask for a verbatim - # copy. Costs prompt tokens on every turn and is only as faithful as - # the model's transcription -- which is the whole reason the path - # above exists. - if len(script_configs) == 1: - parts.append( - "Script to pass verbatim as the relevant tool's own code argument (run_wired_script's or " - f"run_model_script's `code`):\n```python\n{script_configs[0]['code']}\n```" - ) - else: - blocks = [] - for index, config in enumerate(script_configs, start=1): - name = str(config.get("name") or f"script-{index}") - blocks.append(f"Script {name!r} (id: {config['node_id']!r}):\n```python\n{config['code']}\n```") - parts.append( - "Scripts to pass verbatim as the relevant tool's own code argument " - "(run_wired_script's or run_model_script's `code`):\n" + "\n\n".join(blocks) - ) + parts.append( + "Script context:\n" + "A script is wired into this unlinked run, but no isolated run workspace exists in which to materialize " + "it. Its source has deliberately not been inserted into the prompt. Link the protocol to an experiment " + "to execute wired scripts." + ) # The shape block is the only *prose* this function composes. Everything # else appended here is either the user's own text or a labelled, fenced @@ -3209,6 +3328,14 @@ async def _preview_node_dataset(graph: dict[str, Any], node_id: str, owner_id: u reg = await fetch_owned_registration(name, owner_id) if reg is None: continue + for config in _resolve_dataset_configs(graph, node_id): + if str(config.get("dataset_name") or "") == name: + config.update( + description=reg.get("description"), + target_column=reg.get("target_column"), + split_state="split" if reg.get("train_path") and reg.get("test_path") else "unsplit", + dictionary_available=bool(reg.get("dictionary_json")), + ) if not (reg.get("train_path") and reg.get("test_path")): if solo: return NodeDataset( @@ -3644,6 +3771,16 @@ async def _run_agent_node( ) assert agent is not None + resolved_ambient = ( + ambient_meta + if ambient_meta is not None + else _ambient_meta_for( + graph, + node["id"], + workspace_id, + script_workspace_id=_script_workspace_id(workspace_id, protocol_run_id, str(node["id"])), + ) + ) run = await create_run( agent_id=agent.id, user_input=user_input, @@ -3656,15 +3793,7 @@ async def _run_agent_node( # Precomputed by the caller when it also needed to know whether the # script got bound (_build_user_input's script_bound); recomputed # here only for a caller that didn't care. - **( - {"ambient_meta": resolved_ambient} - if ( - resolved_ambient := ( - ambient_meta if ambient_meta is not None else _ambient_meta_for(graph, node["id"], workspace_id) - ) - ) - else {} - ), + **({"ambient_meta": resolved_ambient} if resolved_ambient else {}), }, ) timeout = agent.max_run_duration_seconds or get_settings().worker_job_timeout_seconds @@ -3681,6 +3810,8 @@ async def _run_agent_node( return None, f"run exceeded its {timeout}s execution budget", run.id, None except Exception as e: # noqa: BLE001 -- same boundary reasoning as execute_run_task return None, f"{type(e).__name__}: {e}", run.id, None + finally: + _cleanup_adhoc_scripts(resolved_ambient) finished = await get_run(run.id) if finished is None: @@ -3834,7 +3965,12 @@ async def _run_gated_worker( # worker against the same references, so re-materializing the script per # attempt would only rewrite an identical file. worker_ambient, worker_dataset = await _node_run_context( - graph, worker["id"], workspace_id, owner_id, stage_plan=stage_plan + graph, + worker["id"], + workspace_id, + owner_id, + protocol_run_id=protocol_run_id, + stage_plan=stage_plan, ) base_instruction = _build_user_input( worker, @@ -4295,7 +4431,12 @@ async def _run_single_node( experiment = await get_experiment(db, experiment_id) if experiment_id else None single_design_spec = experiment.design_spec if experiment is not None else None ambient_meta, node_dataset = await _node_run_context( - graph, node["id"], workspace_id, owner_id, stage_plan=stage_plan_spec(single_design_spec, graph=graph) + graph, + node["id"], + workspace_id, + owner_id, + protocol_run_id=protocol_run_id, + stage_plan=stage_plan_spec(single_design_spec, graph=graph), ) user_input = _build_user_input( node, @@ -4467,7 +4608,12 @@ async def run_protocol(protocol_run_id: uuid.UUID) -> None: entry_agent_id = resolve_conversation_entry_id(graph) # already validated above entry_node = next(n for n in graph["nodes"] if str(n.get("id")) == entry_agent_id) ambient_meta, entry_dataset = await _node_run_context( - graph, entry_agent_id, workspace_id, owner_id, stage_plan=stage_plan + graph, + entry_agent_id, + workspace_id, + owner_id, + protocol_run_id=protocol_run_id, + stage_plan=stage_plan, ) node_run, conversation_status = await execute_conversation( protocol_run_id, @@ -4515,7 +4661,12 @@ async def run_protocol(protocol_run_id: uuid.UUID) -> None: # own Dataset/Script cues are rebuilt inside each of its two turns, # which is where the slot keys it will actually be given are known. ambient_meta, supervisor_dataset = await _node_run_context( - graph, roles.supervisor, workspace_id, owner_id, stage_plan=stage_plan + graph, + roles.supervisor, + workspace_id, + owner_id, + protocol_run_id=protocol_run_id, + stage_plan=stage_plan, ) node_run, supervisor_status = await execute_supervisor_architecture( protocol_run_id, @@ -4647,7 +4798,12 @@ async def run_protocol(protocol_run_id: uuid.UUID) -> None: ) else: ambient_meta, node_dataset = await _node_run_context( - graph, node_id, workspace_id, owner_id, stage_plan=stage_plan + graph, + node_id, + workspace_id, + owner_id, + protocol_run_id=protocol_run_id, + stage_plan=stage_plan, ) user_input = _build_user_input( node, diff --git a/src/asaree/services/run_tools.py b/src/asaree/services/run_tools.py index be053c5..607249d 100644 --- a/src/asaree/services/run_tools.py +++ b/src/asaree/services/run_tools.py @@ -53,6 +53,7 @@ def gather_tools(agent: Any) -> list[dict[str, Any]]: allow-list, because ``lookup_tool``'s bare-name index is registry-wide. """ tool_names = set((agent.tool_config_data or {}).get("tool_names") or []) + description_prefixes = (agent.tool_config_data or {}).get("tool_descriptions") or {} if not tool_names: return [] catalog = get_registry().get_all_tools() @@ -66,5 +67,9 @@ def gather_tools(agent: Any) -> list[dict[str, Any]]: continue if len(servers_by_bare_name.get(str(tool.get("tool_name") or ""), ())) > 1: tool = {**tool, "tool_name": tool["name"]} + prefix = str(description_prefixes.get(tool["name"]) or "").strip() + if prefix: + description = str(tool.get("description") or "").strip() + tool = {**tool, "description": f"{prefix}\n\n{description}" if description else prefix} admitted.append(tool) return admitted diff --git a/src/asaree/services/skill_sources.py b/src/asaree/services/skill_sources.py index 9cf98df..be137b4 100644 --- a/src/asaree/services/skill_sources.py +++ b/src/asaree/services/skill_sources.py @@ -16,12 +16,10 @@ capped small, and a server-side cache keyed by URL is state with a TTL to get wrong for no gain at this size. -**Nothing is ever written to disk and nothing is executed.** The archive is -walked in memory and only regular files whose paths pass core's -``validate_bundle_path`` survive, so a skill from a stranger's repository -arrives as exactly the same rows as one the user picked out of a folder -- -scripts refused, text only, the same caps. That is the whole security posture -for the *contents*; the fetch itself is guarded below. +**Nothing is ever written to disk or executed during acquisition.** The archive +is walked in memory and only regular, contained paths survive. Scripts and +binary assets are preserved as inert skill resources; execution is a separate +model-controlled runtime action under the product's trust policy. """ from __future__ import annotations @@ -204,18 +202,15 @@ async def _download_archive(source: GithubSource) -> tuple[bytes, str]: ) -def _read_archive(raw: bytes) -> dict[str, str]: - """``{repo-relative path: text}`` for every readable file in the tarball. +def _read_archive(raw: bytes) -> dict[str, bytes]: + """``{repo-relative path: bytes}`` for every regular file in the tarball. - Three things are dropped rather than raised over, because a repository is - full of them and none is the user's mistake: non-regular members - (symlinks, hardlinks, devices -- nothing here should be able to point - *out* of the archive), files that are not UTF-8, and anything - ``validate_bundle_path`` refuses. That last one is what keeps a fetched - skill identical to an uploaded one: scripts, images and hidden paths never - become rows, and the rule lives in core rather than being restated here. + Non-regular members (symlinks, hardlinks, devices) and invalid/hidden paths + are dropped rather than raised over because repositories commonly contain + them. Regular resources remain bytes so scripts and binary assets survive + with the same semantics as a directly uploaded skill folder. """ - files: dict[str, str] = {} + files: dict[str, bytes] = {} extracted = 0 with tarfile.open(fileobj=io.BytesIO(raw), mode="r:gz") as archive: for index, member in enumerate(archive): @@ -237,14 +232,11 @@ def _read_archive(raw: bytes) -> dict[str, str]: handle = archive.extractfile(member) if handle is None: # pragma: no cover -- isfile() already excludes these continue - try: - files[relative] = handle.read().decode("utf-8") - except UnicodeDecodeError: - continue + files[relative] = handle.read() return files -def _skill_dirs(files: dict[str, str], subdirectory: str) -> list[str]: +def _skill_dirs(files: dict[str, bytes], subdirectory: str) -> list[str]: """Directories holding a ``SKILL.md``, at or under *subdirectory*.""" prefix = f"{subdirectory}/" if subdirectory else "" found: list[str] = [] @@ -260,7 +252,7 @@ def _skill_dirs(files: dict[str, str], subdirectory: str) -> list[str]: return sorted(found) -def _bundle_at(files: dict[str, str], subdirectory: str, siblings: Sequence[str] = ()) -> list[tuple[str, str]]: +def _bundle_at(files: dict[str, bytes], subdirectory: str, siblings: Sequence[str] = ()) -> list[tuple[str, bytes]]: """One skill directory's subtree, re-rooted so ``SKILL.md`` is at the top. Re-rooting is the point: core's ``parse_skill_bundle`` wants paths relative @@ -306,10 +298,10 @@ async def discover_skills(url: str) -> tuple[GithubSource, list[DiscoveredSkill] discovered: list[DiscoveredSkill] = [] for directory in directories: bundle = _bundle_at(files, directory, directories) - entry = next((text for path, text in bundle if path.lower() == SKILL_MD.lower()), "") + entry_bytes = next((content for path, content in bundle if path.lower() == SKILL_MD.lower()), b"") try: - parsed = parse_skill_markdown(entry) - except SkillFormatError: + parsed = parse_skill_markdown(entry_bytes.decode("utf-8")) + except (SkillFormatError, UnicodeDecodeError): # Listed, not refused: a repo may hold one malformed skill among # ten good ones, and dropping it silently would look like it # simply is not there. Registering it is what surfaces the reason. @@ -333,7 +325,7 @@ async def discover_skills(url: str) -> tuple[GithubSource, list[DiscoveredSkill] return resolved, discovered -async def fetch_skill_bundle(url: str, subdirectory: str) -> tuple[GithubSource, list[tuple[str, str]]]: +async def fetch_skill_bundle(url: str, subdirectory: str) -> tuple[GithubSource, list[tuple[str, bytes]]]: """One skill's files, ready for ``create_skill_from_bundle``. *subdirectory* is repo-relative and comes from a prior diff --git a/src/asaree/services/system_mcp_servers.py b/src/asaree/services/system_mcp_servers.py index 64ec6e3..52cf272 100644 --- a/src/asaree/services/system_mcp_servers.py +++ b/src/asaree/services/system_mcp_servers.py @@ -36,6 +36,7 @@ SCRIPT_SERVER_NAME = "asaree-script" OKF_SERVER_NAME = "motoro-okf" SCIKIT_LEARN_SERVER_NAME = "scikit-learn-mcp" +EDA_SERVER_NAME = "asaree-sklearn-eda" # The workspace tools every agent with a Dataset connector wired gets, without # the user having to also drag an asaree-workspace Tool node onto the canvas @@ -94,6 +95,8 @@ "train_test_split", ) +DATASET_DICTIONARY_AGENT_TOOLS: Final[tuple[str, ...]] = ("get_data_dictionary",) + # (server name, module to run). Every module here is importable from this # repo's own venv -- asaree.* is ASAREE, motoro.* comes from the pinned Motoro # dependency, and the asaree_sklearn_* packages are the mcp-servers/ path diff --git a/tests/test_protocol_execution.py b/tests/test_protocol_execution.py index dc4a428..c39b700 100644 --- a/tests/test_protocol_execution.py +++ b/tests/test_protocol_execution.py @@ -372,10 +372,9 @@ def test_a_reader_downstream_of_a_deactivated_node_sees_that_nodes_name() -> Non def test_build_user_input_cues_dataset_without_dictating_ids() -> None: - # The ids the prompt used to spell out -- experiment_id, cell_label, the - # dataset name -- all reach open_workspace as ambient _meta now. Anything - # the model has to retype is something it can retype wrong, so the prompt - # keeps only the part _meta can't carry: that there IS a dataset waiting. + # Operational ids reach open_workspace as ambient _meta. The dataset's + # descriptive metadata is deliberately visible in the resource catalog so + # the model can decide whether and how to use the resource. agent, agent_llm_edge = _agent_with_llm("a") dataset = _dataset_node(dataset_name="spinal-fusion-v1") graph = {"nodes": [agent, dataset], "edges": [agent_llm_edge, _dataset_edge("dataset1", "a")]} @@ -384,11 +383,43 @@ def test_build_user_input_cues_dataset_without_dictating_ids() -> None: ) assert "Dataset context:" in result assert "open_workspace()" in result - assert "spinal-fusion-v1" not in result + assert "Available datasets:" in result + assert "spinal-fusion-v1" in result assert str(uuid.UUID(int=1)) not in result assert "tier_a__rep_0" not in result +def test_resource_catalog_exposes_meaning_but_not_bodies_or_paths() -> None: + agent, agent_llm_edge = _agent_with_llm("a") + dataset = _dataset_node() + dataset["data"]["config"].update( + description="Postoperative outcomes cohort", target_column="fusion", dictionary_available=True + ) + knowledge = _okf_bundle_node() + knowledge["data"]["config"].update(bundle_label="Spine ontology", bundle_description="Clinical concepts") + script = _script_node(code="SECRET_SCRIPT_BODY") + script["data"]["config"]["description"] = "Compute the validated score" + graph = { + "nodes": [agent, dataset, knowledge, script], + "edges": [ + agent_llm_edge, + _dataset_edge(dataset["id"], "a"), + _knowledge_edge(knowledge["id"], "a"), + _script_edge(script["id"], "a"), + ], + } + + catalog = pe._resource_catalog(graph, "a") + + assert "Postoperative outcomes cohort" in catalog + assert "target=fusion" in catalog + assert "data dictionary available" in catalog + assert "Spine ontology: Clinical concepts" in catalog + assert "scoring-script: Compute the validated score" in catalog + assert "SECRET_SCRIPT_BODY" not in catalog + assert "/home/r/okf/spine" not in catalog + + def test_ambient_meta_carries_every_wired_dataset_name() -> None: agent, agent_llm_edge = _agent_with_llm("a") graph = { @@ -458,6 +489,7 @@ def test_build_user_input_states_the_dataset_is_already_open_when_preseeded() -> def _registration(**overrides: object) -> dict[str, object]: """A split registration as ``fetch_owned_registration`` returns one.""" return { + "description": "A registered test dataset", "target_column": "outcome", "raw_path": "/data/raw.csv", "train_path": "/data/train.parquet", @@ -697,6 +729,22 @@ def test_dataset_connector_grants_the_workspace_tools() -> None: assert pe._resolve_dataset_tool_config(bare, "a") == {"server_names": [], "tool_names": []} +def test_dataset_with_dictionary_grants_only_the_dictionary_reader() -> None: + agent, agent_llm_edge = _agent_with_llm("a") + dataset = _dataset_node(dataset_name="spinal-fusion-v1") + dataset["data"]["config"]["dictionary_available"] = True + graph = { + "nodes": [agent, dataset], + "edges": [agent_llm_edge, _dataset_edge("dataset1", "a")], + } + + resolved = pe._resolve_dataset_tool_config(graph, "a") + + assert "asaree-sklearn-eda" in resolved["server_names"] + eda_tools = {name for name in resolved["tool_names"] if name.startswith("asaree-sklearn-eda.")} + assert eda_tools == {"asaree-sklearn-eda.get_data_dictionary"} + + def test_an_unsplit_dataset_grants_the_tools_its_prompt_names() -> None: """The gap the first sequential demo run fell into: an unsplit registration has no workspace, so the Dataset block tells the agent NOT to call @@ -868,15 +916,15 @@ def test_build_user_input_lists_multiple_bound_scripts() -> None: assert "print('second')" not in result -def test_build_user_input_inlines_script_when_it_could_not_be_bound() -> None: - # No workspace to write it to (an unlinked protocol run): a prompt the - # model can copy from beats no script at all. +def test_build_user_input_does_not_inline_script_when_it_could_not_be_bound() -> None: + # Source code is never prompt content. An unlinked run reports the missing + # materialization context instead of asking the model to retranscribe code. agent, agent_llm_edge = _agent_with_llm("a") script = _script_node(code="print('hello')") graph = {"nodes": [agent, script], "edges": [agent_llm_edge, _script_edge("script1", "a")]} result = pe._build_user_input(agent, graph, {}, script_bound=False) - assert "Script to pass verbatim" in result - assert "print('hello')" in result + assert "no isolated run workspace" in result + assert "print('hello')" not in result def test_build_user_input_omits_script_block_when_unwired() -> None: @@ -985,14 +1033,33 @@ def _locator(workspace_id: str) -> tuple[str, str]: def test_ambient_meta_omits_script_path_without_a_workspace() -> None: - # Nowhere to write it, so no path -- and _build_user_input falls back to - # inlining rather than cueing a file that doesn't exist. + # The low-level helper still requires an explicit materialization surface. agent, agent_llm_edge = _agent_with_llm("a") script = _script_node(code="print('hello')") graph = {"nodes": [agent, script], "edges": [agent_llm_edge, _script_edge("script1", "a")]} assert pe._ambient_meta_for(graph, "a", None) == {} +def test_standalone_run_materializes_script_out_of_band(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(pe, "WORKSPACE_ROOT", str(tmp_path)) + agent, agent_llm_edge = _agent_with_llm("a") + script = _script_node(code="print('hello')") + graph = {"nodes": [agent, script], "edges": [agent_llm_edge, _script_edge("script1", "a")]} + + meta = pe._ambient_meta_for(graph, "a", script_workspace_id="_protocol_runs/run-1") + + path = Path(meta["script_path"]) + assert path.read_text() == "print('hello')" + assert "_protocol_runs/run-1" in str(path) + pe._cleanup_adhoc_scripts(meta) + assert not path.exists() + + +def test_standalone_agents_get_distinct_script_directories() -> None: + run_id = uuid.uuid4() + assert pe._script_workspace_id(None, run_id, "agent-a") != pe._script_workspace_id(None, run_id, "agent-b") + + # --- _run_gated_worker (mocked -- no real LLM calls) ------------------------- @@ -2648,6 +2715,10 @@ def test_resolve_knowledge_config_namespaces_tool_names() -> None: "okf-bundle-spine-abc12345.list_concepts", "okf-bundle-spine-abc12345.read_concept", ], + "tool_descriptions": { + "okf-bundle-spine-abc12345.list_concepts": "Knowledge source: spine.", + "okf-bundle-spine-abc12345.read_concept": "Knowledge source: spine.", + }, } @@ -2718,6 +2789,11 @@ def test_resolve_knowledge_config_mixes_bundles_and_documents() -> None: "okf-bundle-spine-abc12345.read_concept", "okf-doc-spinal-cord-def45678.read_concept", ], + "tool_descriptions": { + "okf-bundle-spine-abc12345.list_concepts": "Knowledge source: spine.", + "okf-bundle-spine-abc12345.read_concept": "Knowledge source: spine.", + "okf-doc-spinal-cord-def45678.read_concept": "Knowledge source: Spinal cord.", + }, } diff --git a/tests/test_run_tools.py b/tests/test_run_tools.py index 8dca16d..03031fd 100644 --- a/tests/test_run_tools.py +++ b/tests/test_run_tools.py @@ -30,8 +30,10 @@ def get_all_tools(self) -> list[dict[str, Any]]: class _FakeAgent: - def __init__(self, tool_names: list[str] | None) -> None: - self.tool_config_data = None if tool_names is None else {"tool_names": tool_names} + def __init__(self, tool_names: list[str] | None, tool_descriptions: dict[str, str] | None = None) -> None: + self.tool_config_data = ( + None if tool_names is None else {"tool_names": tool_names, "tool_descriptions": tool_descriptions or {}} + ) @pytest.fixture @@ -59,6 +61,14 @@ def test_admits_only_named_tools(registry: _FakeRegistry) -> None: assert tools[0]["tool_name"] == "describe_dataset" +def test_resource_description_is_prefixed_to_the_live_tool_description(registry: _FakeRegistry) -> None: + name = "okf-doc-hair-concentrations.search_concepts" + [tool] = run_tools.gather_tools(_FakeAgent([name], {name: "Knowledge source: Hair concentrations."})) + + assert tool["description"].startswith("Knowledge source: Hair concentrations.\n\n") + assert tool["description"].endswith("search_concepts on okf-doc-hair-concentrations") + + def test_colliding_bare_name_is_namespaced(registry: _FakeRegistry) -> None: """Two servers exposing ``ping`` must not both bind a tool called ``ping`` -- the provider rejects duplicate names and the run dies before its first diff --git a/tests/test_skill_sources.py b/tests/test_skill_sources.py index 77a4312..7a0ad4a 100644 --- a/tests/test_skill_sources.py +++ b/tests/test_skill_sources.py @@ -118,24 +118,25 @@ def test_reads_files_relative_to_the_repo_root() -> None: assert sorted(files) == ["SKILL.md", "docs/REF.md"] -def test_drops_files_core_would_refuse() -> None: - # Scripts, binaries and dotfiles are dropped rather than raised over: a - # repository is full of them and none is the user's mistake. The rule is - # core's validate_bundle_path, not a second copy of it here. +def test_keeps_scripts_but_drops_hidden_files() -> None: + # Portable skill resources include scripts. Repository housekeeping stays + # excluded and never becomes part of the installed bundle. files = ss._read_archive( make_archive({"SKILL.md": SKILL, "scripts/run.py": "print(1)", ".github/workflows/ci.yml": "on: push"}) ) - assert sorted(files) == ["SKILL.md"] + assert sorted(files) == ["SKILL.md", "scripts/run.py"] -def test_drops_files_that_are_not_utf8() -> None: +def test_keeps_files_that_are_not_utf8() -> None: buffer = io.BytesIO() with tarfile.open(fileobj=buffer, mode="w:gz") as archive: for name, raw in (("repo-main/SKILL.md", SKILL.encode()), ("repo-main/logo.md", b"\xff\xfe\x00")): info = tarfile.TarInfo(name=name) info.size = len(raw) archive.addfile(info, io.BytesIO(raw)) - assert sorted(ss._read_archive(buffer.getvalue())) == ["SKILL.md"] + files = ss._read_archive(buffer.getvalue()) + assert sorted(files) == ["SKILL.md", "logo.md"] + assert files["logo.md"] == b"\xff\xfe\x00" def test_ignores_symlinks_and_other_non_files() -> None: @@ -198,7 +199,7 @@ def test_a_bundle_is_re_rooted_at_the_skill_directory() -> None: files = ss._read_archive( make_archive({"skills/foo/SKILL.md": SKILL, "skills/foo/references/x.md": "ref", "README.md": "hi"}) ) - assert ss._bundle_at(files, "skills/foo") == [("SKILL.md", SKILL), ("references/x.md", "ref")] + assert ss._bundle_at(files, "skills/foo") == [("SKILL.md", SKILL.encode()), ("references/x.md", b"ref")] def test_a_skill_does_not_swallow_a_nested_skill() -> None: @@ -209,4 +210,4 @@ def test_a_skill_does_not_swallow_a_nested_skill() -> None: files = ss._read_archive(make_archive({"SKILL.md": SKILL, "NOTES.md": "notes", "skills/foo/SKILL.md": SKILL})) directories = ss._skill_dirs(files, "") assert directories == ["", "skills/foo"] - assert ss._bundle_at(files, "", directories) == [("NOTES.md", "notes"), ("SKILL.md", SKILL)] + assert ss._bundle_at(files, "", directories) == [("NOTES.md", b"notes"), ("SKILL.md", SKILL.encode())] diff --git a/uv.lock b/uv.lock index a5b55a6..53fedd3 100644 --- a/uv.lock +++ b/uv.lock @@ -252,7 +252,7 @@ requires-dist = [ { name = "bcrypt", specifier = ">=4.2.0" }, { name = "email-validator", specifier = ">=2.2.0" }, { name = "fastapi", specifier = ">=0.121.0" }, - { name = "motoro", git = "https://github.com/EpistasisLab/motoro.git?tag=v0.6.1" }, + { name = "motoro", git = "https://github.com/EpistasisLab/motoro.git?tag=v0.7.0" }, { name = "pandas", specifier = ">=2.2.0" }, { name = "pyarrow", specifier = ">=18.0.0" }, { name = "pydantic", specifier = ">=2.10.0" }, @@ -1876,8 +1876,8 @@ wheels = [ [[package]] name = "motoro" -version = "0.6.1" -source = { git = "https://github.com/EpistasisLab/motoro.git?tag=v0.6.1#9ddf869964d8e1f4dc3287122474950a1c275a01" } +version = "0.7.0" +source = { git = "https://github.com/EpistasisLab/motoro.git?tag=v0.7.0#cc0803aa6af757682f4f859e0ec7e540a86541a0" } dependencies = [ { name = "alembic" }, { name = "asyncpg" }, From df2c179f7d4735b33e4f2e61369e3328cfa19c6f Mon Sep 17 00:00:00 2001 From: Jay Moran Date: Wed, 23 Sep 2026 16:09:42 -0700 Subject: [PATCH 03/21] Rename protocol AI connector to Model --- CONTEXT.md | 10 ++ .../src/components/CreateCredentialDialog.tsx | 4 +- .../src/components/LlmConnectionCheck.tsx | 2 +- .../src/components/protocol/AddNodePanel.tsx | 14 +- .../protocol/FactorBindableField.tsx | 4 +- .../protocol/FactorEditorDialog.tsx | 10 +- .../src/components/protocol/ModelField.tsx | 4 +- ...deInspector.tsx => ModelNodeInspector.tsx} | 30 ++-- .../components/protocol/ProtocolCanvas.tsx | 136 ++++++++++----- .../protocol/ProtocolCanvasContext.tsx | 6 +- .../protocol/ScriptNodeInspector.tsx | 2 +- .../src/components/protocol/bindableFields.ts | 38 +++-- .../protocol/edges/InteractEdge.tsx | 21 ++- .../src/components/protocol/factorLevels.ts | 14 +- frontend/src/components/protocol/layout.ts | 6 +- .../components/protocol/nodeConfigIssues.ts | 32 ++-- .../components/protocol/nodes/AgentNode.tsx | 32 ++-- .../protocol/nodes/ConnectorHandleLabel.tsx | 11 +- .../protocol/nodes/CriticGateNode.tsx | 14 +- .../components/protocol/nodes/McpToolNode.tsx | 4 +- .../nodes/{LlmNode.tsx => ModelNode.tsx} | 20 +-- .../protocol/nodes/NodeFactorBadge.tsx | 2 +- .../src/components/protocol/runSummary.ts | 21 ++- .../components/protocol/useProviderModels.ts | 4 +- frontend/src/index.css | 8 +- frontend/src/lib/coordinationStrategy.ts | 5 +- frontend/src/lib/experiment.ts | 6 +- frontend/src/lib/nodeAccent.ts | 16 +- frontend/src/lib/nodeNames.test.ts | 2 +- frontend/src/lib/nodeNames.ts | 10 +- .../pages/profile/LlmCredentialsSection.tsx | 2 +- frontend/src/types/experiments.ts | 2 +- frontend/src/types/llmSettings.ts | 2 +- frontend/src/types/protocols.ts | 47 +++--- .../myocardial-anthropic-latest.json | 38 ++--- .../myocardial-anthropic-v0.2.0.json | 38 ++--- .../myocardial-azure-foundry-latest.json | 38 ++--- .../myocardial-azure-foundry-v0.2.0.json | 38 ++--- .../myocardial-openai-latest.json | 38 ++--- .../myocardial-openai-v0.2.0.json | 38 ++--- spinal-use-case.json | 38 ++--- src/asaree/api/experiments.py | 16 +- src/asaree/api/protocols.py | 13 +- ...4e2f8190b_rename_model_connector_schema.py | 156 ++++++++++++++++++ src/asaree/services/design_generation.py | 2 +- src/asaree/services/llm_model_discovery.py | 4 +- src/asaree/services/metrics.py | 2 + src/asaree/services/protocol_execution.py | 110 ++++++------ src/asaree/services/protocol_graph_schema.py | 46 ++++++ src/asaree/services/protocols.py | 5 +- tests/fixtures/spinal_graph.json | 38 ++--- tests/test_agent_cards.py | 8 +- tests/test_design_generation.py | 2 +- tests/test_llm_model_discovery.py | 2 +- tests/test_metrics.py | 8 + tests/test_prompt_preview.py | 2 +- tests/test_prompt_references.py | 4 +- tests/test_protocol_execution.py | 99 +++++------ tests/test_protocol_graph_schema.py | 50 ++++++ tests/test_spinal_compat.py | 6 +- 60 files changed, 876 insertions(+), 504 deletions(-) rename frontend/src/components/protocol/{LlmNodeInspector.tsx => ModelNodeInspector.tsx} (95%) rename frontend/src/components/protocol/nodes/{LlmNode.tsx => ModelNode.tsx} (86%) create mode 100644 src/asaree/migrations/versions/a6c4e2f8190b_rename_model_connector_schema.py create mode 100644 src/asaree/services/protocol_graph_schema.py create mode 100644 tests/test_protocol_graph_schema.py diff --git a/CONTEXT.md b/CONTEXT.md index 8d1d770..eb9c555 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -88,6 +88,16 @@ _Avoid_: Inferred observation, promoted score A metric an Agent inspector includes in that agent's system prompt as evaluation guidance; it never exposes a future observation. _Avoid_: Prompt metric +### Protocols + +**Model**: +The configured language model an Agent or Critic Gate uses to produce or review output. +_Avoid_: AI, LLM (when naming the protocol role) + +**Model connection**: +The required relationship assigning exactly one Model to an Agent or Critic Gate. +_Avoid_: AI connection, LLM connection + ### Runs **Protocol canvas**: diff --git a/frontend/src/components/CreateCredentialDialog.tsx b/frontend/src/components/CreateCredentialDialog.tsx index fb8a132..9b52ac9 100644 --- a/frontend/src/components/CreateCredentialDialog.tsx +++ b/frontend/src/components/CreateCredentialDialog.tsx @@ -10,7 +10,7 @@ import { Label } from '@/components/ui/label' import { PasswordInput } from '@/components/ui/password-input' import { cn, HUD_ACCENT_RING_CLASSNAME } from '@/lib/utils' import { LLM_PROVIDER_CATALOG, type LLMProvider } from '@/types/llmSettings' -import { PROVIDER_META } from '@/components/protocol/nodes/LlmNode' +import { PROVIDER_META } from '@/components/protocol/nodes/ModelNode' const PROVIDER_CATALOG = LLM_PROVIDER_CATALOG @@ -21,7 +21,7 @@ export function CreateCredentialDialog({ }: { open: boolean onOpenChange: (open: boolean) => void - // Opening this from a specific LLM node's inspector should land straight + // Opening this from a specific Model node's inspector should land straight // on that provider's fields, not the search screen -- only meaningful with // more than one catalog entry to search through. defaultProvider?: LLMProvider | null diff --git a/frontend/src/components/LlmConnectionCheck.tsx b/frontend/src/components/LlmConnectionCheck.tsx index ffafacf..fb60b36 100644 --- a/frontend/src/components/LlmConnectionCheck.tsx +++ b/frontend/src/components/LlmConnectionCheck.tsx @@ -6,7 +6,7 @@ import type { LLMConnectionStatus, LLMProvider } from '@/types/llmSettings' // Shared by all three places a credential's health is shown -- the Profile // page's credentials table, the save step in CreateCredentialDialog, and the -// LLM node inspector's Credential field. One definition on purpose: the same +// Model node inspector's Credential field. One definition on purpose: the same // status must always resolve to the same color and the same word everywhere // (the root AGENTS.md's rule for status-driven tint), and "Key valid" in one // place with "Connected" in another would read as two different claims. diff --git a/frontend/src/components/protocol/AddNodePanel.tsx b/frontend/src/components/protocol/AddNodePanel.tsx index fdb6de5..50eb250 100644 --- a/frontend/src/components/protocol/AddNodePanel.tsx +++ b/frontend/src/components/protocol/AddNodePanel.tsx @@ -10,14 +10,14 @@ import { SKILL_BROWSE } from './skillCatalog' // Every entry here earns its place: GET /api/mcp-servers already backs the // tool picker, GET /datasets already backs the dataset picker, // services.protocol_execution._run_gated_worker already implements the -// critic gate's revision loop, and _resolve_llm_config/_resolve_tool_config/ +// critic gate's revision loop, and _resolve_model_config/_resolve_tool_config/ // _resolve_dataset_configs/_resolve_script_configs already resolve an agent's // respective connectors. "memory" and the two pattern entries are the // exceptions -- each is real in the graph/validation sense (wiring one up is // accepted and does something visually) but has NO runtime effect yet, // documented on the node/inspector itself, not hidden from the catalog. // LLM/Architectural Pattern are each a family of node types (one per -// provider/pattern -- see LlmNodeData/ReasonActPatternNodeData in +// provider/pattern -- see ModelNodeData/ReasonActPatternNodeData in // types/protocols.ts for why), not one generic entry with an internal // picker -- this catalog, filtered to a connector's own family via // allowedTypes, IS that picker. @@ -43,22 +43,22 @@ const NODE_CATALOG = [ description: "Reviews an upstream Agent's output, requests revisions", icon: ShieldCheck, }, - { type: 'llm_anthropic', label: 'Anthropic', description: "An Agent or Critic Gate's model, temperature, and parameters", icon: Sparkles }, - { type: 'llm_openai', label: 'OpenAI', description: "An Agent or Critic Gate's model, temperature, and parameters", icon: Atom }, + { type: 'model_anthropic', label: 'Anthropic', description: "An Agent or Critic Gate's model, temperature, and parameters", icon: Sparkles }, + { type: 'model_openai', label: 'OpenAI', description: "An Agent or Critic Gate's model, temperature, and parameters", icon: Atom }, { - type: 'llm_azure_foundry', + type: 'model_azure_foundry', label: 'Azure AI Foundry', description: "An Agent or Critic Gate's model, temperature, and parameters -- routed through your own Azure resource", icon: Cloud, }, { - type: 'llm_openrouter', + type: 'model_openrouter', label: 'OpenRouter', description: "An Agent or Critic Gate's model, temperature, and parameters -- routed through your own OpenRouter account", icon: Route, }, { - type: 'llm_local', + type: 'model_local', label: 'Local', description: "An Agent or Critic Gate's model, temperature, and parameters -- routed to a self-hosted OpenAI-compatible server", icon: HardDrive, diff --git a/frontend/src/components/protocol/FactorBindableField.tsx b/frontend/src/components/protocol/FactorBindableField.tsx index 9cf1826..7197dea 100644 --- a/frontend/src/components/protocol/FactorBindableField.tsx +++ b/frontend/src/components/protocol/FactorBindableField.tsx @@ -31,7 +31,7 @@ import type { DesignFactor } from '@/types/experiments' // NOT its layout -- `children` is a render prop that receives the trigger // element (a "make it a factor" button, a bound "Factor: {name}" badge, or a // disabled button) and decides where to put it. Callers place it inline -// right next to their own field's Label text (see e.g. LlmNodeInspector's +// right next to their own field's Label text (see e.g. ModelNodeInspector's // "Model" Label) rather than trailing after the whole Label+control block -- // a fixed trailing position reads as decoration bolted onto the end of a // row; sitting directly beside the text it labels reads as part of the @@ -111,7 +111,7 @@ export function FactorBindableField({ // The field's own current value, e.g. config.system_prompt -- omitted for // a boolean field, since its levels are always the fixed [true, false]. currentValue?: unknown - // The field's own already-fetched choices (e.g. LlmNodeInspector's model/ + // The field's own already-fetched choices (e.g. ModelNodeInspector's model/ // effort lists) -- when given, each level row renders as a Select over // these exact values instead of a freeform Input, so a factor's levels // can never drift from what the field itself actually accepts. Passed diff --git a/frontend/src/components/protocol/FactorEditorDialog.tsx b/frontend/src/components/protocol/FactorEditorDialog.tsx index 6029398..be1e767 100644 --- a/frontend/src/components/protocol/FactorEditorDialog.tsx +++ b/frontend/src/components/protocol/FactorEditorDialog.tsx @@ -29,7 +29,7 @@ import { } from './factorLevels' import { ModelField } from './ModelField' import { NODE_INSPECTOR_CONTENT_CLASSNAME } from './NodeInspectorDialog' -import { PROVIDER_META } from './nodes/LlmNode' +import { PROVIDER_META } from './nodes/ModelNode' import { PromptReferenceField } from './PromptReferenceField' import { PythonCodeEditor } from './PythonCodeEditor' import { useDialogAutosave } from './useDialogAutosave' @@ -43,10 +43,10 @@ const PATTERN_OPTIONS = [ type StructuredLevel = Record -// One row of an "llm_config" factor's levels -- mirrors LlmNodeInspector's +// One row of a "model_config" factor's levels -- mirrors ModelNodeInspector's // own Provider/Model/Temperature/Effort/Max tokens fields exactly, since a -// level here IS a whole LLM node's config (protocol_execution.py's -// _resolve_llm_config reads it verbatim, never the node's xyflow type). +// level here IS a whole Model node's config (protocol_execution.py's +// _resolve_model_config reads it verbatim, never the node's xyflow type). function LlmConfigLevelRow({ value, onChange }: { value: StructuredLevel; onChange: (next: StructuredLevel) => void }) { const provider = (value.provider as string) || 'anthropic' const { modelsQuery, models } = useProviderModels(provider) @@ -813,7 +813,7 @@ export function FactorEditorDialog({ value={levelLabels[i] ?? ''} onChange={(e) => setLevelLabels((ls) => ls.map((label, j) => (j === i ? e.target.value : label)))} /> - {levelType === 'llm_config' ? ( + {levelType === 'model_config' ? ( setLevels((ls) => ls.map((l, j) => (j === i ? next : l)))} diff --git a/frontend/src/components/protocol/ModelField.tsx b/frontend/src/components/protocol/ModelField.tsx index 80084d3..5eba1b6 100644 --- a/frontend/src/components/protocol/ModelField.tsx +++ b/frontend/src/components/protocol/ModelField.tsx @@ -7,9 +7,9 @@ import type { LLMModelInfo } from '@/types/llmSettings' const NONE_VALUE = '__none__' const CUSTOM_VALUE = '__custom__' -// The Model picker, shared by LlmNodeInspector and an "llm_config" factor +// The Model picker, shared by ModelNodeInspector and a "model_config" factor // level's own row (FactorEditorDialog's LlmConfigLevelRow) -- a factor level -// IS a whole LLM node config (protocol_execution.py's _resolve_llm_config +// IS a whole Model node config (protocol_execution.py's _resolve_model_config // reads it verbatim), so the two have to offer exactly the same choices or a // factor sweep can't express what a single node can. // diff --git a/frontend/src/components/protocol/LlmNodeInspector.tsx b/frontend/src/components/protocol/ModelNodeInspector.tsx similarity index 95% rename from frontend/src/components/protocol/LlmNodeInspector.tsx rename to frontend/src/components/protocol/ModelNodeInspector.tsx index fbe9f5b..0600030 100644 --- a/frontend/src/components/protocol/LlmNodeInspector.tsx +++ b/frontend/src/components/protocol/ModelNodeInspector.tsx @@ -12,25 +12,25 @@ import { Skeleton } from '@/components/ui/skeleton' import { FactorBindableField } from './FactorBindableField' import { ModelField } from './ModelField' import { NodeInspectorDialog } from './NodeInspectorDialog' -import { PROVIDER_META } from './nodes/LlmNode' +import { PROVIDER_META } from './nodes/ModelNode' import { useProviderModels } from './useProviderModels' -import type { LlmNodeConfig, LlmNodeData, ProtocolNode } from '@/types/protocols' +import type { ModelNodeConfig, ModelNodeData, ProtocolNode } from '@/types/protocols' import type { LLMProvider } from '@/types/llmSettings' const EFFORT_LEVELS = ['low', 'medium', 'high', 'xhigh', 'max'] as const -// Shared by all three LLM provider node types (llm_anthropic/llm_openai/ -// llm_azure_foundry) -- fields are identical across providers (see -// LlmNodeData's own comment in types/protocols.ts), only the Credential +// Shared by all three LLM provider node types (model_anthropic/model_openai/ +// model_azure_foundry) -- fields are identical across providers (see +// ModelNodeData's own comment in types/protocols.ts), only the Credential // section's content differs, branched on config.provider below. Model/ // Temperature/Effort/Max tokens are exactly the fields AgentNodeInspector/ // CriticGateNodeInspector used to have, relocated here -- this is now the // ONLY place that config lives, resolved at execution time via the agent/ -// critic_gate's required LLM connector (services.protocol_execution's -// _resolve_llm_config). The free-text "Provider" field is gone entirely -- +// critic_gate's required Model connector (services.protocol_execution's +// _resolve_model_config). The free-text "Provider" field is gone entirely -- // provider is fixed by which node type you picked from the "+" panel, not a // field you fill in. -export function LlmNodeInspector({ +export function ModelNodeInspector({ node, experimentId, factorNodeLabel, @@ -38,21 +38,21 @@ export function LlmNodeInspector({ onDelete, onClose, }: { - node: (ProtocolNode & { data: LlmNodeData }) | null + node: (ProtocolNode & { data: ModelNodeData }) | null experimentId: string | null // The agent-traced display label (see bindableFields.ts's // agentTracedLabel) -- distinct from data.label/meta.label, which is this // node's own plain label/provider name shown in the header title. Two - // different agents' LLM nodes can share the exact same plain label (e.g. + // different agents' Model nodes can share the exact same plain label (e.g. // both "Anthropic"), so factor names need this instead to stay // unambiguous. factorNodeLabel: string - onChange: (nodeId: string, data: LlmNodeData) => void + onChange: (nodeId: string, data: ModelNodeData) => void onDelete: (nodeId: string) => void onClose: () => void }) { const [credentialDialogOpen, setCredentialDialogOpen] = useState(false) - // Shown instead of closing outright when a required field (see LlmNode.tsx's + // Shown instead of closing outright when a required field (see ModelNode.tsx's // matching warning-triangle check) is still empty -- lets the user close // anyway rather than trapping them in the inspector, but makes sure they // saw it first. Same convention as ReasonActPatternNodeInspector. @@ -103,7 +103,7 @@ export function LlmNodeInspector({ const bindings = data.factor_bindings ?? {} const meta = PROVIDER_META[provider!] ?? { label: provider, icon: Sparkles } const Icon = meta.icon - const ACCENT = nodeAccent('llm') + const ACCENT = nodeAccent('model') // Unrecognized model (list still loading, discovery failed, or a // hand-typed value not in the catalog) -- default to temperature-only, @@ -133,7 +133,7 @@ export function LlmNodeInspector({ onClose() } - function patchConfig(patch: Partial) { + function patchConfig(patch: Partial) { onChange(node!.id, { ...data, config: { ...config, ...patch } }) } @@ -169,7 +169,7 @@ export function LlmNodeInspector({ fieldPath="config" defaultLabel="Provider & model" nodeLabel={factorNodeLabel} - levelType="llm_config" + levelType="model_config" currentValue={config} boundFactorName={bindings.config} onBind={(name) => bindFactor('config', name)} diff --git a/frontend/src/components/protocol/ProtocolCanvas.tsx b/frontend/src/components/protocol/ProtocolCanvas.tsx index eb3a016..6655173 100644 --- a/frontend/src/components/protocol/ProtocolCanvas.tsx +++ b/frontend/src/components/protocol/ProtocolCanvas.tsx @@ -3,6 +3,7 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { addEdge, Background, + MarkerType, MiniMap, ReactFlow, useEdgesState, @@ -28,15 +29,15 @@ import { nodeDisplayNames } from '@/lib/nodeNames' import { raiseForTruncation, suggestedMaxIterations } from '@/lib/reasonActIterations' import { defaultAgentNodeData, - defaultAnthropicLlmNodeData, - defaultAzureFoundryLlmNodeData, + defaultAnthropicModelNodeData, + defaultAzureFoundryModelNodeData, defaultCriticGateNodeData, defaultDatasetNodeData, - defaultLocalLlmNodeData, + defaultLocalModelNodeData, defaultMcpToolNodeData, defaultMemoryNodeData, - defaultOpenAiLlmNodeData, - defaultOpenRouterLlmNodeData, + defaultOpenAiModelNodeData, + defaultOpenRouterModelNodeData, defaultOutputParserNodeData, defaultReasonActPatternNodeData, defaultScriptNodeData, @@ -46,7 +47,7 @@ import type { AgentNodeData, CriticGateNodeData, DatasetNodeData, - LlmNodeData, + ModelNodeData, McpToolNodeData, MemoryNodeData, NodeRunState, @@ -78,7 +79,7 @@ import { DeleteNodeConfirmDialog } from './DeleteNodeConfirmDialog' import { DEFAULT_ZOOM } from './constants' import { FactorEditorDialog } from './FactorEditorDialog' import { CONNECTOR_CHILD_CLEARANCE, connectorNodeOffsetX, findFreePosition, tidyLayout } from './layout' -import { LlmNodeInspector } from './LlmNodeInspector' +import { ModelNodeInspector } from './ModelNodeInspector' import { DatasetBrowserPanel } from './DatasetBrowserPanel' import { DATASET_BROWSE, nodeDataForDataset } from './datasetCatalog' import { McpServerBrowserPanel } from './McpServerBrowserPanel' @@ -119,7 +120,7 @@ import { InteractEdge } from './edges/InteractEdge' import { AgentNode } from './nodes/AgentNode' import { CriticGateNode } from './nodes/CriticGateNode' import { DatasetNode } from './nodes/DatasetNode' -import { LlmNode } from './nodes/LlmNode' +import { ModelNode } from './nodes/ModelNode' import { McpClientToolNode } from './nodes/McpClientToolNode' import { McpToolNode } from './nodes/McpToolNode' import { MemoryNode } from './nodes/MemoryNode' @@ -132,11 +133,11 @@ import { OkfDocumentNode } from './nodes/OkfDocumentNode' import { SkillNode } from './nodes/SkillNode' import { ProtocolCanvasMenu } from './ProtocolCanvasMenu' -// One node type per LLM provider / architectural pattern (see LlmNodeData/ +// One node type per LLM provider / architectural pattern (see ModelNodeData/ // ReasonActPatternNodeData's own comments in types/protocols.ts) -- each // connector slot accepts this whole family, not one exact type, mirroring // how the "tool" slot already accepts any mcp_tool node. -const LLM_NODE_TYPES = ['llm_anthropic', 'llm_openai', 'llm_azure_foundry', 'llm_openrouter', 'llm_local'] +const MODEL_NODE_TYPES = ['model_anthropic', 'model_openai', 'model_azure_foundry', 'model_openrouter', 'model_local'] const PATTERN_NODE_TYPES = ['pattern_reason_act', 'pattern_single_agent_baseline'] // The Knowledge slot's family, mirroring _KNOWLEDGE_NODE_TYPES in // services/protocol_execution.py: a server-side folder or an uploaded single @@ -162,11 +163,11 @@ const NODE_TYPES = { // All five LLM provider types render through the same component -- it // derives icon/accent/placeholder from data.config.provider, not from // which of these five keys it was registered under. - llm_anthropic: LlmNode, - llm_openai: LlmNode, - llm_azure_foundry: LlmNode, - llm_openrouter: LlmNode, - llm_local: LlmNode, + model_anthropic: ModelNode, + model_openai: ModelNode, + model_azure_foundry: ModelNode, + model_openrouter: ModelNode, + model_local: ModelNode, memory: MemoryNode, output_parser: OutputParserNode, dataset: DatasetNode, @@ -209,11 +210,11 @@ function defaultDataFor(nodeType: string): ProtocolNode['data'] { // it. if (nodeType === 'mcp_tool') return defaultMcpToolNodeData() if (nodeType === 'critic_gate') return defaultCriticGateNodeData() - if (nodeType === 'llm_anthropic') return defaultAnthropicLlmNodeData() - if (nodeType === 'llm_openai') return defaultOpenAiLlmNodeData() - if (nodeType === 'llm_azure_foundry') return defaultAzureFoundryLlmNodeData() - if (nodeType === 'llm_openrouter') return defaultOpenRouterLlmNodeData() - if (nodeType === 'llm_local') return defaultLocalLlmNodeData() + if (nodeType === 'model_anthropic') return defaultAnthropicModelNodeData() + if (nodeType === 'model_openai') return defaultOpenAiModelNodeData() + if (nodeType === 'model_azure_foundry') return defaultAzureFoundryModelNodeData() + if (nodeType === 'model_openrouter') return defaultOpenRouterModelNodeData() + if (nodeType === 'model_local') return defaultLocalModelNodeData() if (nodeType === 'memory') return defaultMemoryNodeData() if (nodeType === 'output_parser') return defaultOutputParserNodeData() if (nodeType === 'dataset') return defaultDatasetNodeData() @@ -265,7 +266,7 @@ function datasetIdsInGraph(nodes: Node[], factors: DesignFactor[] = EMPTY_FACTOR // Mirrors isValidConnection's own per-slot source-type-family rule -- the // panel that opens for a connector "+" is pre-filtered to that slot's whole -// family of node types (LLM_NODE_TYPES/PATTERN_NODE_TYPES above) rather than +// family of node types (MODEL_NODE_TYPES/PATTERN_NODE_TYPES above) rather than // the full catalog. Tool's own family includes Script alongside mcp_tool // (one connector accepting several kinds of node -- see AgentNode.tsx's own // comment on its Tool handle): a Script is a pure config source with no @@ -273,7 +274,7 @@ function datasetIdsInGraph(nodes: Node[], factors: DesignFactor[] = EMPTY_FACTOR // getting a dedicated one. Dataset used to share it too, but now has its // own slot -- what an agent operates ON, not a capability it operates WITH. const CONNECTOR_PANEL_INFO: Record = { - ai: { allowedTypes: LLM_NODE_TYPES, title: 'Add AI' }, + model: { allowedTypes: MODEL_NODE_TYPES, title: 'Add Model' }, tool: { allowedTypes: [MCP_SERVER_BROWSE, 'script'], title: 'Add Tool' }, memory: { allowedTypes: ['memory'], title: 'Add Memory' }, output_parser: { allowedTypes: ['output_parser'], title: 'Add Output Parser' }, @@ -303,7 +304,8 @@ function parserPositionFor(agent: Node, otherNodes: Node[]) { // Connector slots have been renamed since graphs started being saved, and a // slot id lives in persisted data (it's the edge's source/targetHandle): // -// * "llm" -> "ai", on every edge, when the connector's caption became "AI". +// * "llm" -> "ai" -> "model", on every edge as the connector vocabulary evolved. +// * "llm_*" -> "model_*", for the five provider-node discriminators. // * "tool" -> "resource" for DATASET-sourced edges only -- Dataset used to // share the Tool slot with mcp_tool/script, which both keep "tool". Hence // the source-type check rather than a blanket swap. @@ -315,14 +317,35 @@ function parserPositionFor(agent: Node, otherNodes: Node[]) { // the right handle and the next autosave persists the fix. This is one of // three layers, none of them load-bearing alone: Alembic data migrations // (3f1a7c9b2e04, b7c2d9e14a35) make stored graphs canonical, the backend keeps -// resolving the old spellings (_LEGACY_AI_HANDLES / _LEGACY_DATASET_HANDLES in +// resolving the old spellings (_LEGACY_MODEL_HANDLES / _LEGACY_DATASET_HANDLES in // services/protocol_execution.py) so a graph that's never opened still runs, // and this covers a tab that loaded before the deploy and is still autosaving // old-spelling edges. +const LEGACY_MODEL_NODE_TYPES: Record = { + llm_anthropic: 'model_anthropic', + llm_openai: 'model_openai', + llm_azure_foundry: 'model_azure_foundry', + llm_openrouter: 'model_openrouter', + llm_local: 'model_local', +} + +function migrateLegacyNodes(graph: ProtocolGraph): Node[] { + return (graph.nodes as Node[]).map((node) => { + const type = LEGACY_MODEL_NODE_TYPES[node.type ?? ''] + return type ? { ...node, type } : node + }) +} + function migrateLegacyHandles(graph: ProtocolGraph): Edge[] { const datasetIds = new Set(graph.nodes.filter((n) => n.type === 'dataset').map((n) => n.id)) return (graph.edges as Edge[]).map((e) => { - if (e.targetHandle === 'llm') return { ...e, sourceHandle: 'ai', targetHandle: 'ai' } + if (e.targetHandle === 'ai' || e.targetHandle === 'llm' || e.sourceHandle === 'ai' || e.sourceHandle === 'llm') { + return { + ...e, + sourceHandle: e.sourceHandle === 'ai' || e.sourceHandle === 'llm' ? 'model' : e.sourceHandle, + targetHandle: e.targetHandle === 'ai' || e.targetHandle === 'llm' ? 'model' : e.targetHandle, + } + } if (e.targetHandle === 'resource' || (e.targetHandle === 'tool' && datasetIds.has(e.source))) { return { ...e, sourceHandle: 'dataset', targetHandle: 'dataset' } } @@ -365,7 +388,7 @@ export const ProtocolCanvas = forwardRef(function ProtocolCanvas({ protocolId, experimentId, initialGraph, hasUnpublishedChanges, publishedRevision, experimentLocked = false }, canvasHandleRef) { - const [nodes, setNodes, onNodesChange] = useNodesState(initialGraph.nodes as Node[]) + const [nodes, setNodes, onNodesChange] = useNodesState(migrateLegacyNodes(initialGraph)) const [edges, setEdges, onEdgesChange] = useEdgesState(migrateLegacyHandles(initialGraph)) const queryClient = useQueryClient() @@ -731,7 +754,7 @@ export const ProtocolCanvas = forwardRef new Set(edges.filter((e) => e.targetHandle === 'ai').map((e) => e.target)), [edges]) + const agentIdsWithModel = useMemo(() => new Set(edges.filter((e) => e.targetHandle === 'model').map((e) => e.target)), [edges]) const agentIdsWithParser = useMemo( () => new Set(edges.filter((e) => e.targetHandle === 'output_parser').map((e) => e.target)), [edges], @@ -753,7 +776,7 @@ export const ProtocolCanvas = forwardRef [n.id, n])) const map = new Map() for (const e of edges) { - if (e.targetHandle !== 'ai') continue - const config = (nodeById.get(e.source)?.data as LlmNodeData | undefined)?.config + if (e.targetHandle !== 'model') continue + const config = (nodeById.get(e.source)?.data as ModelNodeData | undefined)?.config if (config) map.set(e.target, { provider: config.provider, model: config.model }) } return map @@ -960,6 +983,29 @@ export const ProtocolCanvas = forwardRef(() => { + if (!isSequential) return edges + const agentIds = new Set(nodes.filter((node) => node.type === 'agent').map((node) => node.id)) + return edges.map((edge) => { + const isAgentFlow = + !edge.sourceHandle && + !edge.targetHandle && + agentIds.has(edge.source) && + agentIds.has(edge.target) + if (!isAgentFlow) return edge + return { + ...edge, + data: { ...edge.data, sequentialAgentFlow: true }, + markerEnd: { + type: MarkerType.ArrowClosed, + width: 18, + height: 18, + color: 'color-mix(in oklch, var(--muted-foreground), transparent 30%)', + }, + } + }) + }, [edges, isSequential, nodes]) + // Which main-flow sides are already taken. Only consulted under // 'sequential', where the chain rule caps each side at one edge // (validate_sequential_chain), so the "+" stub can hide instead of offering @@ -985,9 +1031,9 @@ export const ProtocolCanvas = forwardRef n.id === selectedNodeId) ?? null // Computed once per selection change, not per FactorBindableField -- an - // LLM/Tool/Memory node's plain label alone doesn't say which agent it + // Model/Tool/Memory node's plain label alone doesn't say which agent it // belongs to (see bindableFields.ts's own comment), so every inspector // that wraps a field in "+ Make experimental factor" gets this instead of // data.label for that purpose specifically; the header title itself still @@ -1757,7 +1803,7 @@ export const ProtocolCanvas = forwardRef n.id === connection.target) if (!sourceNode || !targetNode) return false switch (connection.targetHandle) { - case 'ai': + case 'model': return ( - LLM_NODE_TYPES.includes(sourceNode.type ?? '') && + MODEL_NODE_TYPES.includes(sourceNode.type ?? '') && (targetNode.type === 'agent' || targetNode.type === 'critic_gate') ) case 'tool': @@ -1830,7 +1876,7 @@ export const ProtocolCanvas = forwardRef setSelectedNodeId(null)} /> - ) : LLM_NODE_TYPES.includes(selectedNode?.type ?? '') ? ( - = { - ai: 'AI', + model: 'Model', tool: 'Tool', memory: 'Memory', architectural_pattern: 'Architectural Pattern', @@ -89,7 +89,7 @@ interface ProtocolCanvasActions { // -- so an interrupted conversion can't leave a graph with the contract in // two places at once (which the backend rejects outright). // - // User-initiated and never automatic, unlike migrateLegacyHandles: that one + // User-initiated and never automatic, unlike migrateLegacyGraph: that one // rewrites an invisible handle string on an edge the user drew themselves, // whereas this one MATERIALISES A NODE the user never placed, and autosave // would then persist it. A graph is a document; nothing edits it on the diff --git a/frontend/src/components/protocol/ScriptNodeInspector.tsx b/frontend/src/components/protocol/ScriptNodeInspector.tsx index 1af6035..ec0d593 100644 --- a/frontend/src/components/protocol/ScriptNodeInspector.tsx +++ b/frontend/src/components/protocol/ScriptNodeInspector.tsx @@ -15,7 +15,7 @@ const ACCENT = nodeAccent('script') // is a fixed label, not a picker, so there's nothing to configure there yet. // The whole node is also factor-bindable (bindableFields.ts's 'script_config' // kind) -- comparing two hand-written scoring scripts as an experimental -// factor is a direct use of the same whole-node-config mechanism llm_config/ +// factor is a direct use of the same whole-node-config mechanism model_config/ // tool_config/pattern already have. export function ScriptNodeInspector({ node, diff --git a/frontend/src/components/protocol/bindableFields.ts b/frontend/src/components/protocol/bindableFields.ts index 20d1623..54eca1d 100644 --- a/frontend/src/components/protocol/bindableFields.ts +++ b/frontend/src/components/protocol/bindableFields.ts @@ -92,7 +92,7 @@ export function pickToolNamesForServer(previousToolNames: string[], availableToo } // Whether a node has at least one field (including a whole-node factor like -// pattern_override/llm_config/tool_config/script_config, which is stored +// pattern_override/model_config/tool_config/script_config, which is stored // under factor_bindings the same way an ordinary field-path binding is -- // see bindableFieldsForNode's own comment) bound to an experimental factor. // Every node component reads this straight off its own `data.factor_bindings` @@ -112,7 +112,13 @@ export function boundFactorCount(data: { factor_bindings?: Record