From bd357da4444affc09cdb95e2fb44e68d2d863492 Mon Sep 17 00:00:00 2001 From: Luke Mainwaring Date: Thu, 16 Jul 2026 18:08:48 -0400 Subject: [PATCH 1/2] docs: promote scattered rationale into docs/adr/ Four decisions that look wrong without context were only explained in comments next to the code that implements them. Promote each into a numbered ADR and cite it from the implementing docstring, per .claude/rules/conventions.md. - 0001 HNSW over IVFFlat for the track-embedding index - 0002 iTunes Search previews instead of Spotify preview_url - 0003 Two-layer error handling for agent tools - 0004 Binarize DEAP labels at each subject's own median The pgvector migration is touched in comments only; no migration code changes. Co-Authored-By: Claude Fable 5 --- backend/src/cortexdj/agents/hooks.py | 4 ++++ .../cortexdj/agents/tools/retrieval_tools.py | 4 ++++ ...add_pgvector_extension_and_track_audio_.py | 1 + backend/src/cortexdj/ml/dataset.py | 2 ++ .../cortexdj/models/track_audio_embedding.py | 7 +++++++ .../src/cortexdj/services/audio_catalog.py | 2 ++ docs/adr/0001-hnsw-over-ivfflat.md | 15 ++++++++++++++ ...tunes-previews-over-spotify-preview-url.md | 16 +++++++++++++++ .../0003-two-layer-tool-error-convention.md | 20 +++++++++++++++++++ .../0004-subject-median-label-binarization.md | 19 ++++++++++++++++++ 10 files changed, 90 insertions(+) create mode 100644 docs/adr/0001-hnsw-over-ivfflat.md create mode 100644 docs/adr/0002-itunes-previews-over-spotify-preview-url.md create mode 100644 docs/adr/0003-two-layer-tool-error-convention.md create mode 100644 docs/adr/0004-subject-median-label-binarization.md diff --git a/backend/src/cortexdj/agents/hooks.py b/backend/src/cortexdj/agents/hooks.py index 1fbebbc..7ca1290 100644 --- a/backend/src/cortexdj/agents/hooks.py +++ b/backend/src/cortexdj/agents/hooks.py @@ -7,6 +7,10 @@ stream mid-response. ``on_tool_execute_error`` intercepts those, logs the traceback, and returns a structured recovery payload so the agent can explain the failure to the user conversationally. + +This is layer 2 of the two-layer tool-error convention; see +docs/adr/0003-two-layer-tool-error-convention.md for why tools propagate +rather than wrap, and for the one sanctioned inline catch. """ import logging diff --git a/backend/src/cortexdj/agents/tools/retrieval_tools.py b/backend/src/cortexdj/agents/tools/retrieval_tools.py index f564e06..3a0cbeb 100644 --- a/backend/src/cortexdj/agents/tools/retrieval_tools.py +++ b/backend/src/cortexdj/agents/tools/retrieval_tools.py @@ -34,6 +34,10 @@ async def retrieve_tracks_from_brain_state( `note` field explains how to populate it. When the session's underlying DEAP data is missing on disk, the payload contains an `error` field that the agent should relay verbatim to the user. + + The `DeapFileMissingError` catch below is the one sanctioned deviation from + the propagate-to-hooks convention; see + docs/adr/0003-two-layer-tool-error-convention.md. Do not add a second one. """ try: hits = await retrieval_service.retrieve_similar_tracks(ctx.deps.db, session_id, k=k) diff --git a/backend/src/cortexdj/migrations/versions/77c744e4b096_add_pgvector_extension_and_track_audio_.py b/backend/src/cortexdj/migrations/versions/77c744e4b096_add_pgvector_extension_and_track_audio_.py index 62014a0..c68de0e 100644 --- a/backend/src/cortexdj/migrations/versions/77c744e4b096_add_pgvector_extension_and_track_audio_.py +++ b/backend/src/cortexdj/migrations/versions/77c744e4b096_add_pgvector_extension_and_track_audio_.py @@ -24,6 +24,7 @@ def upgrade() -> None: # pass; HNSW builds on empty and updates the graph on insert, with better # recall at our 2k–10k row scale. m=16, ef_construction=64 are pgvector # defaults — no tuning needed until the table grows past ~100k rows. + # See docs/adr/0001-hnsw-over-ivfflat.md. op.execute("CREATE EXTENSION IF NOT EXISTS vector") op.create_table( diff --git a/backend/src/cortexdj/ml/dataset.py b/backend/src/cortexdj/ml/dataset.py index 18ac008..5e9ce0a 100644 --- a/backend/src/cortexdj/ml/dataset.py +++ b/backend/src/cortexdj/ml/dataset.py @@ -42,6 +42,8 @@ CBRAMOD_SCALE_FACTOR = 0.01 # Label binarization strategy for DEAP's 1-9 Likert self-reports. +# See docs/adr/0004-subject-median-label-binarization.md for why the default +# deviates from the >= 5 threshold the DEAP literature uses. # # `median_per_subject` (default): splits each axis at that subject's own # median. Balanced within each subject, removes per-subject rating-scale bias. diff --git a/backend/src/cortexdj/models/track_audio_embedding.py b/backend/src/cortexdj/models/track_audio_embedding.py index 7d7349e..d1955b0 100644 --- a/backend/src/cortexdj/models/track_audio_embedding.py +++ b/backend/src/cortexdj/models/track_audio_embedding.py @@ -16,6 +16,13 @@ class TrackAudioEmbedding(Base): + """CLAP audio embeddings for tracks, searched by cosine distance. + + `get_top_k_similar`'s ordering is served by an HNSW index (built in + migration 77c744e4b096) — keep the order-by on `cosine_distance` so the + index applies; see docs/adr/0001-hnsw-over-ivfflat.md. + """ + __tablename__ = "track_audio_embeddings" id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) diff --git a/backend/src/cortexdj/services/audio_catalog.py b/backend/src/cortexdj/services/audio_catalog.py index 83d0c62..7632d38 100644 --- a/backend/src/cortexdj/services/audio_catalog.py +++ b/backend/src/cortexdj/services/audio_catalog.py @@ -4,6 +4,8 @@ 2024-11-27 — empirically verified 0/10 hits against this project's 2018 Spotify app. iTunes Search API is the audio-bytes source; Spotify stays as the source of truth for track identity. + +See docs/adr/0002-itunes-previews-over-spotify-preview-url.md. """ import asyncio diff --git a/docs/adr/0001-hnsw-over-ivfflat.md b/docs/adr/0001-hnsw-over-ivfflat.md new file mode 100644 index 0000000..e700bbb --- /dev/null +++ b/docs/adr/0001-hnsw-over-ivfflat.md @@ -0,0 +1,15 @@ +# HNSW over IVFFlat for the track-embedding index + +**Status**: Accepted — 2026-07-16 + +The `track_audio_embeddings` cosine-similarity index (pgvector) is created by +migration `77c744e4b096` on a table that is empty at migration time and filled +later by incremental seed passes. IVFFlat needs a training step over existing +rows to build its lists, so an IVFFlat index created here would be trained on +nothing and degrade to a sequential scan until a manual `REINDEX` after every +seed pass. We use HNSW instead: it builds on an empty table and updates its +graph on insert, and gives better recall at this project's ~2k–10k row scale. + +`m = 16, ef_construction = 64` are pgvector's defaults and are left untuned; +revisit only if the table grows past ~100k rows, where IVFFlat's smaller build +cost and memory footprint start to matter. diff --git a/docs/adr/0002-itunes-previews-over-spotify-preview-url.md b/docs/adr/0002-itunes-previews-over-spotify-preview-url.md new file mode 100644 index 0000000..65c79d8 --- /dev/null +++ b/docs/adr/0002-itunes-previews-over-spotify-preview-url.md @@ -0,0 +1,16 @@ +# iTunes Search previews instead of Spotify `preview_url` + +**Status**: Accepted — 2026-07-16 + +Spotify deprecated `preview_url` for standard-mode apps on 2024-11-27; against +this project's Spotify app it now returns 0/10 hits, so it cannot supply the 30s +audio the CLAP encoder needs. `services/audio_catalog.py` resolves previews from +the iTunes Search API instead, and Spotify remains the source of truth for track +identity — an iTunes hit is only accepted for a track Spotify already named. + +Because iTunes matches on a text query, a hit can be the wrong edit of the right +song. Spotify's `duration_ms` anchors the match: candidates outside a 3s +duration delta are rejected outright, and survivors are ranked by artist +similarity, then title similarity, then smallest delta. Do not drop the duration +filter in favor of text similarity alone — remasters and live versions score +identically on title. diff --git a/docs/adr/0003-two-layer-tool-error-convention.md b/docs/adr/0003-two-layer-tool-error-convention.md new file mode 100644 index 0000000..f8e3506 --- /dev/null +++ b/docs/adr/0003-two-layer-tool-error-convention.md @@ -0,0 +1,20 @@ +# Two-layer error handling for agent tools + +**Status**: Accepted — 2026-07-16 + +An exception escaping a Pydantic AI tool body would crash the Vercel AI SDK SSE +stream mid-response. Rather than wrap every tool in try/except — which spreads +error presentation across the tool layer and buries real bugs — tools let +unanticipated exceptions propagate to `agents/hooks.py`, whose +`tool_execute_error` hook logs the traceback and returns a structured recovery +payload naming the tool and exception class, so the agent apologizes +conversationally instead of dying. Anticipated failures (Spotify not configured, +token expired) still return `{"error": ...}` dicts from the tool body itself. + +The one sanctioned exception is `retrieval_tools.retrieve_tracks_from_brain_state`, +which catches `DeapFileMissingError` inline. The hook's payload deliberately +carries only the exception's class name, not its message — but this error's +message is the actionable part (which DEAP file is missing, and where to put +it). Catching it at the tool lets that text reach the user verbatim. Adding a +second inline catch is a signal to re-examine this ADR, not to follow the +precedent. diff --git a/docs/adr/0004-subject-median-label-binarization.md b/docs/adr/0004-subject-median-label-binarization.md new file mode 100644 index 0000000..db068d5 --- /dev/null +++ b/docs/adr/0004-subject-median-label-binarization.md @@ -0,0 +1,19 @@ +# Binarize DEAP labels at each subject's own median + +**Status**: Accepted — 2026-07-16 + +DEAP's valence/arousal labels are 1–9 Likert self-reports. The convention in the +DEAP literature is to threshold at `>= 5`, which here produces a ~25/75 class +skew and bakes in per-subject rating-scale bias — some participants never use +the low end of the scale, so the "low arousal" class is partly a fact about the +rater, not the trial. Under our leave-one-subject-out CV regime that bias lands +squarely in the held-out fold. + +`ml/dataset.py` therefore defaults to `median_per_subject`: each axis is split +at that subject's own median across their 40 trials, giving roughly balanced +labels per subject and removing the scale bias. `median_global` (pooled median) +and `fixed_5` remain opt-in via `--label-split`; `fixed_5` exists to reproduce +published numbers and should not be read as our baseline. + +**Consequence**: accuracy figures here are not directly comparable to papers +using the `>= 5` split — a `fixed_5` run is the apples-to-apples comparison. From 4a471addffde49fcddc2b9a1253c42915c7f40d8 Mon Sep 17 00:00:00 2001 From: Luke Mainwaring Date: Thu, 16 Jul 2026 18:08:58 -0400 Subject: [PATCH 2/2] docs: sync agent rules with adopted layouts The rules described the codebase as it was before the ADR directory and the frontend feature-slice registry landed, so they pointed agents at patterns that no longer exist. - conventions.md: docs/adr/ exists now; drop the "doesn't exist yet" note - vercel-ai-sdk.md: tool panels come from the registry, not a branch in the message renderer - frontend/code-conventions.md: document the feature-slice boundaries - AGENTS.md: point moved components at features/, note the registry in the data flow, and add pointers to docs/adr/ and the test tiers Both frontend rules files now match against frontend/features/**. Co-Authored-By: Claude Fable 5 --- .claude/rules/conventions.md | 11 +++++------ .claude/rules/frontend/code-conventions.md | 18 ++++++++++++++++++ .claude/rules/frontend/vercel-ai-sdk.md | 11 ++++++++--- AGENTS.md | 11 +++++++---- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/.claude/rules/conventions.md b/.claude/rules/conventions.md index 995031b..88148f3 100644 --- a/.claude/rules/conventions.md +++ b/.claude/rules/conventions.md @@ -64,9 +64,8 @@ keep it lean (ceiling ~200 lines). The *why* behind a non-obvious choice belongs in `docs/adr/` — and the docstring that implements it cites the ADR, so an agent meets the rationale before it "simplifies" the choice away. -`docs/adr/` doesn't exist yet; it lands via a `grill-with-docs` pass that will -promote today's scattered rationale (HNSW-over-IVFFlat in the pgvector -migration, iTunes-over-`preview_url` in `services/audio_catalog.py`, the -two-layer tool-error convention, label binarization at the subject median) into -numbered records. Until then, cite the existing home and keep new rationale out -of `AGENTS.md`. +`docs/adr/` now holds four numbered records — HNSW-over-IVFFlat (0001), +iTunes-over-`preview_url` (0002), the two-layer tool-error convention (0003), +and subject-median label binarization (0004). A new non-obvious choice gets a +new ADR (next number, format per `.agents/skills/domain-modeling/ADR-FORMAT.md`), +and the docstring implementing it cites the file by path. diff --git a/.claude/rules/frontend/code-conventions.md b/.claude/rules/frontend/code-conventions.md index 1272d01..56a900d 100644 --- a/.claude/rules/frontend/code-conventions.md +++ b/.claude/rules/frontend/code-conventions.md @@ -1,6 +1,7 @@ --- paths: - "frontend/components/**/*.{ts,tsx}" + - "frontend/features/**/*.{ts,tsx}" - "frontend/app/**/*.{ts,tsx}" - "frontend/hooks/**/*.{ts,tsx}" - "frontend/api/hooks/**/*.{ts,tsx}" @@ -11,6 +12,23 @@ paths: TypeScript/Next.js conventions for the cortexdj frontend. +## Feature slices + +Domain UI lives in `frontend/features//` (kebab-case files, same as +everywhere else) — `sessions/` and `retrieval/` today. What goes where: + +- **A slice never imports another slice.** `frontend/features/tool-panel-registry.ts` + is the composition root and the only file that reaches across slices; it + spreads each slice's `tool-panels.tsx` map into `TOOL_PANELS`. +- **Shared chat chrome stays in `components/`** (message renderer, input, + sidebar, `ui/`). A component used by two slices belongs there, not in either + slice. +- **Cross-cutting data hooks stay in `api/hooks/`**, shared parsing/format + helpers in `lib/`. A slice may own a hook only it uses. + +Intra-slice imports are relative (`./tool-panels`); anything crossing a +top-level directory uses `@/`. + ## Imports - Use the `@/` path alias for imports that cross top-level directories diff --git a/.claude/rules/frontend/vercel-ai-sdk.md b/.claude/rules/frontend/vercel-ai-sdk.md index dcbbf93..cf52001 100644 --- a/.claude/rules/frontend/vercel-ai-sdk.md +++ b/.claude/rules/frontend/vercel-ai-sdk.md @@ -1,6 +1,7 @@ --- paths: - "frontend/components/**/*.tsx" + - "frontend/features/**/*.{ts,tsx}" - "frontend/app/(chat)/**/*.{ts,tsx}" - "frontend/hooks/**/*.{ts,tsx}" - "frontend/api/hooks/**/*.{ts,tsx}" @@ -44,6 +45,10 @@ authoritative for the underlying contract that adapted UI must wire into. `ChatMessage` type in `frontend/lib/types.ts`. Carry that generic through every `UseChatHelpers` site — falling back to plain `UIMessage` loses the project's custom data/tool-part typing. -- **Tool-call panels switch on `part.type === "tool-"`** in the message - renderer. A new backend tool that needs a custom UI panel adds a branch - there. +- **Tool-call panels come from a registry, not a branch.** Each feature slice + exports a tool-name → panel map from `frontend/features//tool-panels.tsx`; + `frontend/features/tool-panel-registry.ts` spreads those maps into + `TOOL_PANELS`, and `message.tsx` looks up `part.type === "tool-"` against + it. A new backend tool that needs a custom UI panel = a slice component plus + one map entry — add `hideRawOutput: true` when the panel replaces ``'s + raw JSON output block. Don't reintroduce per-tool branching in the renderer. diff --git a/AGENTS.md b/AGENTS.md index af478ac..00ed024 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,6 +68,7 @@ FastAPI Python backend using async patterns throughout. - **Contrastive EEG↔CLAP**: `contrastive.py` (EegCLAPEncoder with CBraMod backbone + SimCLR projection head, symmetric soft-target InfoNCE, encode_session), `contrastive_dataset.py` (DeapClapPairDataset + host-portable audio cache + `trial_to_eeg_windows` shared slicer), `contrastive_train.py` (SequentialLR warmup+cosine, TensorBoard scalars + val embedding projector, grad accumulation). - **`src/cortexdj/core/config.py`**: Settings via pydantic-settings - **`src/cortexdj/migrations/`**: Alembic migrations for PostgreSQL +- **`tests/`**: Three tiers — `unit/` (default run), `integration/` (real Postgres, `-m integration`), `evals/` (real LLM, `-m eval`). See `DEVELOPMENT.md` and `.claude/rules/backend/code-conventions.md`. ### Frontend (`frontend/`) @@ -77,9 +78,10 @@ Next.js 16 with App Router. - **`app/(chat)/api/chat/route.ts`**: Proxy route to backend agent - **`components/chat.tsx`**: Chat orchestrator using `@ai-sdk/react` useChat hook - **`components/brain-context-badge.tsx`**: Displays active brain context (mood/arousal/valence) -- **`components/session-visualization.tsx`**: Tabbed session viewer — wraps `components/emotion-trajectory.tsx` (default, animated SVG trajectory through Russell's affect space) and a recharts arousal/valence timeline in Radix Tabs, with the band-power chart shared below. Auto-rendered by `components/message.tsx` when an `analyze_session` tool call is detected -- **`components/emotion-trajectory.tsx`**: Custom SVG + `motion/react` chart that plots each 4-second segment as a point in the valence/arousal plane, draws a smoothed rolling-mean path via an animated `motion.path` (`style={{ pathLength: progress }}`), and exposes a play/pause + scrubber driven by a `requestAnimationFrame` loop -- **`components/retrieved-tracks-panel.tsx`**: Ranked tracks rendered beneath `retrieve_tracks_from_brain_state` tool calls — similarity bars, inline 30s preview playback via a shared `