feat(example): agent-driven grounded reporting in web_ui_demo — Compose cited reports over MCP - #361
Merged
Conversation
Read-only tool-calling loop for agent-driven grounded reporting (#353): derives citations deterministically from each MCP tool's specr/anchors _meta, filters to readOnlyHint tools, and bounds rounds/tool-calls/tokens. I/O is injected so the loop is unit-testable without a live API or key. Refs #353 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…porting Adds POST /report — an agent-driven grounded-reporting endpoint that streams its tool-calling steps live (step/usage/done/error NDJSON). Reuses the MCP + OpenAI plumbing but hands the model only readOnlyHint tools, so the composer cannot mutate state; the internal __readOnly flag is stripped before the OpenAI wire. Bounded rounds/tool-calls/token budget guard the corpus case. Black-box integration test drives the full stream against mock OpenAI + MCP. Refs #353 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-through citations Adds the demo's flagship 'agent driving grounded report composition' surface: a Compose tab whose request box drives the read-only /report loop, streams each grounded tool call live (running → done/error), renders the composed narrative, and lists every source as a click-through chip that opens the cited paragraph in the Report/audit view. Surfaces the cost/scope meter (rounds · calls · tokens), a read-only badge, and a disabled Download-PDF affordance deferred to #352. Verified end-to-end in a browser against a mock OpenAI+MCP: streamed steps, cited narrative, and citation→Report navigation all work. Refs #353 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Refs #353 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds agent-driven grounded reporting to ChangesAgent-driven grounded reporting feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant compose.js
participant server.mjs
participant report-bridge.mjs
participant MCP tools
Browser->>compose.js: submit report request
compose.js->>server.mjs: POST /report (request, scope)
server.mjs->>report-bridge.mjs: runReport(...)
loop bounded rounds
report-bridge.mjs->>MCP tools: read-only tool call
MCP tools-->>report-bridge.mjs: result + anchors
report-bridge.mjs-->>server.mjs: step / usage / done
server.mjs-->>compose.js: NDJSON event stream
compose.js-->>Browser: render steps and citations
end
report-bridge.mjs-->>server.mjs: reply + citations
server.mjs-->>compose.js: final done event
compose.js-->>Browser: render report output
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…get (report bridge) Codex review of #361 found the read-only guarantee was enforced only by filtering the ADVERTISED tool list — a hallucinated or injection-induced tool call for a name never advertised was still forwarded to MCP by name, so write tools could execute (the demo's MCP server exposes read+write by default). Fixes: - Deny-by-default at the execution boundary: build an allow-list Set of the read-only tool names and reject any call outside it before it reaches MCP, answering the tool_call_id with an error so the compose turn stays valid. - Per-call budget check: a single assistant message with many tool calls could blow past maxToolCalls because the cap was only checked after the whole batch. Now each call checks the budget first; over-budget calls are skipped (no MCP) and still answered. Pinned with regression tests (unit: never reaches MCP / stops mid-batch; and an end-to-end server test asserting a model-emitted write tool never hits MCP). Refs #353 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thewrz
marked this pull request as ready for review
July 4, 2026 13:42
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
examples/web_ui_demo/js/compose.js (1)
65-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent null-safety for
regenBtn.
regenBtn?.addEventListenertreats the element as optional, butrun()/renderDone()dereferenceregenBtn.hiddendirectly without a guard. Either includeregenBtnin the line 65 required-elements check or use optional chaining consistently at the two write sites.Also applies to: 127-127, 152-152, 171-171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/web_ui_demo/js/compose.js` at line 65, The null-safety around regenBtn is inconsistent in compose.js: it is treated as optional for addEventListener but later accessed directly in run() and renderDone() via regenBtn.hidden. Fix this by making regenBtn part of the required-elements guard alongside input, runBtn, stepsEl, and outputEl, or by switching the hidden assignments in run() and renderDone() to optional chaining so all reads/writes match the same lifecycle.examples/web_ui_demo/server.report.test.mjs (1)
111-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid deterministic test ports.
The PID-based port formula can collide with a local service or parallel test run, and
waitForPort()would accept any responder. Allocate a free port before spawning the child.Suggested fix
- const demoPort = 3000 + (process.pid % 500) + 7; + const demoPort = await getFreePort(); @@ - const demoPort = 3000 + (process.pid % 500) + 8; + const demoPort = await getFreePort(); @@ async function waitForPort(port) { @@ } + +async function getFreePort() { + const server = createServer(); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + const { port } = server.address(); + await new Promise((resolve, reject) => { + server.close((err) => (err ? reject(err) : resolve())); + }); + return port; +}Also applies to: 172-172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/web_ui_demo/server.report.test.mjs` around lines 111 - 112, The server.report.test.mjs port setup is deterministic and can collide with local services or parallel runs, so update the test to allocate an unused port before spawning the child instead of using the PID-based formula. Use the existing test flow around the demo server launch and waitForPort() to first reserve a free port, pass that port into the child process, and keep the port selection logic centralized where demoPort is currently derived.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-03-agent-driven-grounded-reporting.md`:
- Around line 180-181: Remove the unconditional final deps.callModel(messages,
[]) after the loop in runReport, since it can تجاوز the tokenBudget cap and
violate the bounded-cost behavior. Instead, have runReport return the best
available reply from the last assistant message or a truncated/partial closing
state when limits.maxToolCalls or estimateTokens(messages) exceeds the budget,
and keep the existing toolCalls, citations, and usage reporting consistent with
that early stop.
In `@examples/web_ui_demo/js/compose.js`:
- Around line 25-42: The readNdjson helper drops any buffered bytes left in the
TextDecoder when the stream ends, so trailing multi-byte UTF-8 characters can be
lost. Update readNdjson to flush the decoder after the read loop completes by
calling decoder.decode() once more and appending any returned text to buf before
processing the final tail; keep the rest of the NDJSON parsing logic in place so
safeParse still handles complete lines in readNdjson.
In `@examples/web_ui_demo/README.md`:
- Around line 147-150: The README paragraph in the PDF note is rendering
incorrectly because the raw `#352` at the start of a line is being parsed as a
Markdown heading. Update the text in the affected description so the reference
is written as issue `#352` or otherwise escapes the hash, keeping the surrounding
wording in the same PDF/download explanation.
In `@examples/web_ui_demo/report-bridge.mjs`:
- Around line 111-124: The tool-result handling in executeToolCall currently
appends the full tool response to ctx.messages before any token-budget guard is
applied, so oversized MCP output can still reach the final model turn. Add a
size check or truncation/fail-closed path before pushing the tool content into
messages, and apply the same safeguard anywhere tool results are collected
before the next call, including the other executeToolCall/overBudget flow sites
referenced in the report-bridge flow. Use the existing ctx.deps.execTool,
ctx.messages, and overBudget logic to keep the message payload within
tokenBudget before the forced final model step.
- Around line 222-226: The early-exit path in the report loop is passing the max
allowed rounds into finish instead of the number actually consumed, which skews
the meter. Update the finish call in report-bridge.mjs to use the current round
count/actual rounds used from the loop state rather than limits.maxRounds, and
keep the existing fallback message behavior intact.
In `@examples/web_ui_demo/server.mjs`:
- Around line 424-425: The request body in readRequestBody(req) is buffered
without any limit before JSON parsing, so add an early size cap in the /report
flow around readRequestBody/payload handling to reject oversized bodies before
they are fully accumulated. Update the logic near the payload parsing in
server.mjs so REPORT_MAX_REQUEST_CHARS is enforced while streaming or
immediately after reading each chunk, and keep the check tied to the existing
report request handling path.
---
Nitpick comments:
In `@examples/web_ui_demo/js/compose.js`:
- Line 65: The null-safety around regenBtn is inconsistent in compose.js: it is
treated as optional for addEventListener but later accessed directly in run()
and renderDone() via regenBtn.hidden. Fix this by making regenBtn part of the
required-elements guard alongside input, runBtn, stepsEl, and outputEl, or by
switching the hidden assignments in run() and renderDone() to optional chaining
so all reads/writes match the same lifecycle.
In `@examples/web_ui_demo/server.report.test.mjs`:
- Around line 111-112: The server.report.test.mjs port setup is deterministic
and can collide with local services or parallel runs, so update the test to
allocate an unused port before spawning the child instead of using the PID-based
formula. Use the existing test flow around the demo server launch and
waitForPort() to first reserve a free port, pass that port into the child
process, and keep the port selection logic centralized where demoPort is
currently derived.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2631f619-7163-43b9-904f-cea2d7d41271
📒 Files selected for processing (11)
docs/superpowers/plans/2026-07-03-agent-driven-grounded-reporting.mdexamples/web_ui_demo/.env.exampleexamples/web_ui_demo/README.mdexamples/web_ui_demo/css/app.cssexamples/web_ui_demo/index.htmlexamples/web_ui_demo/js/app.jsexamples/web_ui_demo/js/compose.jsexamples/web_ui_demo/report-bridge.mjsexamples/web_ui_demo/report-bridge.test.mjsexamples/web_ui_demo/server.mjsexamples/web_ui_demo/server.report.test.mjs
…unds/docs) - report-bridge: clamp each MCP tool result (MAX_TOOL_RESULT_CHARS) before appending to the running messages, so one broad payload can't blow the token budget or bloat the forced final compose turn (fail-closed at the module boundary, independent of the transport's own truncation). - report-bridge: report the ACTUAL rounds used after a budget break, not the configured max — the cost meter was over-reporting. - server: read the /report body with a bounded reader that stops accumulating and rejects once the 16 KiB cap is crossed, so an oversized payload is not fully buffered before rejection. - compose: flush the TextDecoder at stream end so a multi-byte char split across the final chunk can't corrupt the trailing NDJSON line. - README: reword the PDF/#352 note so no line begins with '#352' (was rendering as a Markdown heading). Pinned with regression tests (clampToolText + bounded-token runReport; actual rounds after break; oversized-body rejection). 17/17 green across both suites. Refs #353 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/web_ui_demo/server.report.test.mjs`:
- Around line 217-234: The fixed demoPort in the server.report test can collide
across runs because t.after calls child.kill() without waiting for the process
to fully exit. Update the server spawn/cleanup flow in server.report.test.mjs so
the child process termination is awaited before the test completes or before the
next spawn can reuse the same port; use the existing child, t.after, and
waitForPort flow to ensure the socket is released before proceeding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: de515f51-53d0-44e1-829a-02b05c98b155
📒 Files selected for processing (6)
examples/web_ui_demo/README.mdexamples/web_ui_demo/js/compose.jsexamples/web_ui_demo/report-bridge.mjsexamples/web_ui_demo/report-bridge.test.mjsexamples/web_ui_demo/server.mjsexamples/web_ui_demo/server.report.test.mjs
✅ Files skipped from review due to trivial changes (1)
- examples/web_ui_demo/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/web_ui_demo/js/compose.js
- examples/web_ui_demo/report-bridge.mjs
- examples/web_ui_demo/server.mjs
CodeRabbit review on #361: the integration tests spawn a demo server per test but t.after only kill()'d the child without awaiting its exit, so its listening socket could still be held when the file's next test binds — a port-race flake unrelated to what's under test. Extract shared spawnDemo/stopDemo helpers and have stopDemo await the child's 'exit' before closing the mock, so the socket is released before the next spawn. Refs #353 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks
This was referenced Jul 7, 2026
Closed
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The MCP contract is complete (ADR-044): every user-facing REST operation is a contract-bound MCP tool, and the demo's
server.mjsalready auto-derives its tool list fromtools/liston every chat. This PR is the demo's flagship demonstration of that — the agent driving grounded report composition — and it showcases SpecR's actual differentiator: deterministic-first, not RAG. The agent doesn't read spec blobs and summarize; it calls grounded tools, gets computed ground truth, and synthesizes a cited narrative.This is example/POC code under
examples/web_ui_demo/— no changes undersrc/, and the product still holds no LLM key (the harness lives in the demo bridge).What
A new Compose tab in the web UI demo:
/reportbridge (server.mjs+ new purereport-bridge.mjs): runs an OpenAI tool-calling loop over SpecR's MCP tools and streams progress back as newline-delimited JSON — onestepper grounded tool call (running → done/error), periodicusage, and a finaldonecarrying the composed narrative + deterministic citations.js/compose.js+ HTML/CSS): a request box (with example chips + a loaded-scope hint), a live grounding-step trace on the left, the composed report + a click-through Sources list on the right, a cost/scope meter, a read-only badge, and a disabled Download-PDF affordance._meta['specr/anchors']channel andaudit.showAnchor— no new anchor scheme, no backend change.Design decisions
Refs #353, notCloses #353: the demo-side reporting affordance is delivered; the PDF artifact remains for feat(generator): PDF artifact egress — render specs to PDF sharing the DOCX style + CSI numbering source-of-truth #352.readOnlyHint(ADR-045 tiers, surfaced on the wire as annotations). It physically cannot write/edit during composition. The internal__readOnlyflag is stripped before the OpenAI wire. Free-form Ask SpecR chat keeps the read+write tier unchanged._meta['specr/anchors'], deduped, and rendered as click-throughs — independent of what the model writes. Only the wording varies between runs, so Regenerate reproduces the same findings.httpand to parse with aReadableStreamreader; avoids SSE framing ceremony. (Non-goal: streaming infra beyond the demo's needs.)rounds · calls · ~tokensmeter so the "hundreds of DOCX" case can't run away.docs/adr/is the product/srcdecision register). These are demo/POC decisions; they live in this PR + the demo README instead.Testing
node --test examples/web_ui_demo/report-bridge.test.mjs— 9 tests green (read-only filter, step labels, dedupe/citations, token estimate, message build, loop + guardrails).node --test examples/web_ui_demo/server.report.test.mjs— black-box drives the full/reportNDJSON stream against a mock OpenAI + mock MCP; asserts step/usage/done events, deterministic citations, that write tools never reach the model, and that__readOnlyis stripped before the OpenAI wire./reportwith noOPENAI_API_KEYemits a single{type:'error', code:'no-key'}line; the panel shows the same "not configured" note as Ask SpecR.examples/web_ui_demo/is intentionally outside CI's lint/tsc/vitest scope (pnpm lint/testtargetsrc/only), so the demo'snode --testsuites run locally, not in CI.Note:
examples/web_ui_demo/must not requiresrc/changes — none are included.🤖 Co-authored by Claude Fable 5. Refs #353.
Summary by CodeRabbit
POST /report) NDJSON endpoint to power Compose with scoped, deterministic grounding output.