Dashboard sessions: switch `get_thoughts` to structured JSON for the renderer #69

New issue

Open

opened 2026-05-10 23:58:41 +02:00 by jesse · 0 comments

jesse commented

2026-05-10 23:58:41 +02:00

Owner

Background

The dashboard's session detail pane parses get_thoughts output by splitting a flat text blob on a regex anchored at Thought #\d+ (timestamp): headers. The formatter contract is ambiguous because a thought's own content can structurally match the header pattern, producing spurious splits with no schema-fallback safety net.

Source: code review of branch phase-8/dashboard-ui-pr3-sessions
File: crates/cognix-server/assets/sessions.js:22, crates/cognix-server/assets/sessions.js:180, crates/cognix-mcp/src/tools/core.rs (get_thoughts formatter)
Branch: phase-8/dashboard-ui-pr3-sessions

Failure mode

splitThoughtsText splits on /\n\n(?=Thought #\d+ \([^)]*\):\n)/.
A thought whose content contains a paragraph break followed by a line shaped like Thought #7 (foo): — e.g. a chat log, a screenshot transcript, or a thought quoting the formatter's own output — is split into multiple cards with bogus indices/timestamps.
parseThoughtCard matches each fragment's header regex on the bogus part, so the schema-fallback path is not reached; the user sees corrupted but plausible-looking cards.
The empty-state path uses a string-equality check on the literal No thoughts have been recorded yet. against the formatter's hard-coded sentinel, with no shared constant and no synchronization test. Either side changing wording silently regresses the empty state into the schema-fallback path.

Why deferred from `phase-8/dashboard-ui-pr3-sessions`

The fix crosses the MCP tool boundary — both the formatter and the dashboard renderer change together.
PR3 scope was the dashboard sessions sidebar, not the MCP get_thoughts contract.
Other tools in PR3 (list_sessions, get_session) already return JSON envelopes; aligning get_thoughts is the natural follow-up.

Proposed work

Extend get_thoughts to return a structured payload (e.g. { thoughts: [{ index, timestamp, content }, ...] }) at least for level: 'concise' | 'full' invocations
Add a schema validator in crates/cognix-server/assets/schema.js (e.g. assertThoughtsListShape) and switch sessions.js to consume the structured shape
Replace the magic-string No thoughts have been recorded yet. empty marker with an explicit empty-list signal in the payload
Delete splitThoughtsText and parseThoughtCard from sessions.js
Preserve a level: 'minimal' or stdio-friendly text format if existing non-dashboard consumers rely on it (audit cognix-cli and any agent integrations first)

Acceptance

A thought whose content contains the canary line Thought #99 (faux):\nspoof renders as a single card whose body includes the canary text verbatim
A session with zero thoughts renders the empty state without triggering schemaFallbackText
A snapshot test or string-equality test asserts the dashboard renderer and the MCP formatter agree on the empty-list signal
Existing e2e tests in e2e/dashboard/dashboard.spec.ts continue to pass
No behavior change for get_thoughts callers outside the dashboard (or, if the contract is changed, all callers updated in the same PR)

References

Branch: phase-8/dashboard-ui-pr3-sessions
Code review finding: MAJOR — sessions.js:22 regex content-driven split + sessions.js:180 empty-state magic-string coupling

## Background The dashboard's session detail pane parses `get_thoughts` output by splitting a flat text blob on a regex anchored at `Thought #\d+ (timestamp):` headers. The formatter contract is ambiguous because a thought's own content can structurally match the header pattern, producing spurious splits with no schema-fallback safety net. **Source:** code review of branch `phase-8/dashboard-ui-pr3-sessions` **File:** `crates/cognix-server/assets/sessions.js:22`, `crates/cognix-server/assets/sessions.js:180`, `crates/cognix-mcp/src/tools/core.rs` (`get_thoughts` formatter) **Branch:** `phase-8/dashboard-ui-pr3-sessions` ## Failure mode - `splitThoughtsText` splits on `/\n\n(?=Thought #\d+ \([^)]*\):\n)/`. - A thought whose content contains a paragraph break followed by a line shaped like `Thought #7 (foo):` — e.g. a chat log, a screenshot transcript, or a thought quoting the formatter's own output — is split into multiple cards with bogus indices/timestamps. - `parseThoughtCard` matches each fragment's header regex on the bogus part, so the schema-fallback path is not reached; the user sees corrupted but plausible-looking cards. - The empty-state path uses a string-equality check on the literal `No thoughts have been recorded yet.` against the formatter's hard-coded sentinel, with no shared constant and no synchronization test. Either side changing wording silently regresses the empty state into the schema-fallback path. ## Why deferred from `phase-8/dashboard-ui-pr3-sessions` - The fix crosses the MCP tool boundary — both the formatter and the dashboard renderer change together. - PR3 scope was the dashboard sessions sidebar, not the MCP `get_thoughts` contract. - Other tools in PR3 (`list_sessions`, `get_session`) already return JSON envelopes; aligning `get_thoughts` is the natural follow-up. ## Proposed work - [ ] Extend `get_thoughts` to return a structured payload (e.g. `{ thoughts: [{ index, timestamp, content }, ...] }`) at least for `level: 'concise' | 'full'` invocations - [ ] Add a schema validator in `crates/cognix-server/assets/schema.js` (e.g. `assertThoughtsListShape`) and switch `sessions.js` to consume the structured shape - [ ] Replace the magic-string `No thoughts have been recorded yet.` empty marker with an explicit empty-list signal in the payload - [ ] Delete `splitThoughtsText` and `parseThoughtCard` from `sessions.js` - [ ] Preserve a `level: 'minimal'` or stdio-friendly text format if existing non-dashboard consumers rely on it (audit `cognix-cli` and any agent integrations first) ## Acceptance - A thought whose content contains the canary line `Thought #99 (faux):\nspoof` renders as a single card whose body includes the canary text verbatim - A session with zero thoughts renders the empty state without triggering `schemaFallbackText` - A snapshot test or string-equality test asserts the dashboard renderer and the MCP formatter agree on the empty-list signal - Existing e2e tests in `e2e/dashboard/dashboard.spec.ts` continue to pass - No behavior change for `get_thoughts` callers outside the dashboard (or, if the contract is changed, all callers updated in the same PR) ## References - Branch: `phase-8/dashboard-ui-pr3-sessions` - Code review finding: MAJOR — `sessions.js:22` regex content-driven split + `sessions.js:180` empty-state magic-string coupling

jesse referenced this issue

2026-05-10 23:59:03 +02:00

MCP session tools: normalize session identifier key across list_sessions and get_session #70

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jesse/cognix#69

No description provided.

Rows
Columns

Dashboard sessions: switch get_thoughts to structured JSON for the renderer #69