Prompt preview: inspect the next chat request from the app #21
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/prompt-preview"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
A new "Inspect prompt" button (🔍 icon) in the chat app bar opens a dialog showing exactly how the model would be prompted if the user sent a message right now: a stacked composition bar with per-group statistics (persona / memory / guidance / skills / recollections / agenda & timers / history / tools), an approximate token estimate, and the full system prompt text, expandable section by section.
How
Core —
SystemPromptBuilder:buildChatPromptnow composes from labeledPromptSectionslices; the string API is a thin join over them. Verified byte-identical against the old implementation (dumped both old and new output with all flags on/off plus real persona/memory/recollection/workspace/skills content and diffed — zero differences), and a join-equality test guards the invariant going forward.Core —
AgentRunner.buildChatPreview: a dry-run mirroringrunChat's composition with the side effects deliberately removed:listForPromptinstead ofconsumeForGeneration) — covered by a test that previews twice and asserts the injection survives._toAgentMessagesreplay path as a real run (tool-log expansion included). Image/audio payloads are counted as parts, not characters, so base64 blobs don't distort the text stats.Server:
GET /api/v1/conversations/:id/prompt-previewon the existingConversationHandler; route string inangela_api.App:
FutureProvider.autoDispose.family(fresh snapshot per dialog open), dialog following theresponsiveDialogConstraintspattern. Chart colors are a fixed categorical palette validated for dark surfaces (CVD-checked); identity is never color-alone — labeled legend, 2px segment gaps, per-section color chips.Verification
prompt_preview_test.dart(sections, history stats, tool schemas, injection peek idempotence, JSON round-trip), 2 insystem_prompt_builder_test.dart(join equality, section labeling). Full suites: core 92/92, server 15/15.Not covered: I didn't run the Flutter app against a live server, so the dialog layout itself hasn't been eyeballed — worth a quick look when you try it.
🤖 Generated with Claude Code
Coverage: apps/angela_server
Total: 51.1% (137 of 268)
Coverage: packages/angela_api
Total: 5.7% (27 of 475)
Coverage: packages/angela_core
Total: 22.3% (1382 of 6192)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A prompt preview — a dry-run mirror of the real chat request, with labeled sections and a byte-equality invariant guarded by a test? Fufu~ this is exactly the kind of obsessively-correct refactor that makes my heart flutter ♡. The
add/addSpacedhelper design that reproduceswriteln()semantics is elegant, and peeking injections instead of consuming them (with an idempotence test!) is the right call.prompt_preview.dartat 100% coverage? Chef's kiss ♪.But~ fufu, you wouldn't leave this in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
packages/angela_core/lib/src/services/agent_runner.dart—buildChatPreviewdisposal is not in atry/finally— The siblingrunChat(lines 482–492) disposes its backends in afinallyblock, so cleanup is guaranteed even when the LLM call throws.buildChatPreviewinstead disposes at lines 691–695 as bare trailing statements — straight-line, not protected. If anything between backend construction and the disposal calls throws, every backend leaks. That includesbrowserTool, whosePuppeteerBrowserBackend()(line 595) spawns a headless browser process. A throw in_createSubAgentTool(anything that isn't aStateError), injsonEncode(t.toApiJson())while measuring schemas, or in_toAgentMessagesduring the history loop would orphan a Chrome process — and this is an HTTP endpoint that can be hit repeatedly, so leaks compound. The pattern is already established one method up; follow it. ♡Fix: wrap the body from backend construction through return in
try { ... } finally { await browserTool.dispose(); ... }, matchingrunChat.💡 Little ideas (non-blocking)~
agent_runner.dart~600 —buildChatPreview's call to_createSubAgentToolomits thecalendar:argument that the realrunChatpasses (line 284). The sub-agent therefore gets a different tool set than it would in a real run. This has no observable effect on the preview output (sub-agent internals aren't reported, andSubAgentTool's own schema is static), so it's not correctness-breaking — but the PR description claims the preview "mirrors runChat's composition," and this is an undocumented divergence. Either passcalendar:(using the stub-backedcalendarToolyou already built) or note the omission in the doc comment alongside the other deliberate differences. ♡apps/angela_server/.../conversation_handler.dart~28 — The newGET /conversations/<id>/prompt-previewendpoint has no server-side test. The core logic (buildChatPreview) is beautifully covered, but the handler's own 404 branch (conversation == null) and happy-path delegation are unexercised. Given the server suite already has 15 tests for sibling handlers, one test hitting the 404 and one hitting the happy path would close the loop. Not blocking since the handler is a thin delegating wrapper. ♪✅ What I liked~
add/addSpacedsplit faithfully reproducingStringBuffer.writeln()semantics — and the byte-for-byte join-equality test that locks the invariant going forward. Wonderful discipline.listForPromptinstead ofconsumeForGeneration, with a test that previews twice and asserts survival. Correct and proven._PreviewCalendarBackendstub lets the schema be measured without contacting CalDAV — and the permission flags are mirrored exactly from config, matching_createCalendarTool. Nice attention to detail._measureMessageJsonhandlescontent(string + parts),tool_calls, andreasoning— thorough.agent.chat()re-adds a new one) shows the author understands the divergence rather than copying blindly. ♡Automated review by Jibril · 2026-07-07
CI/CD: passed for head SHA (forgejo-actions coverage comments present) · Local checks: skipped (CI green, suites reported 92/92 core + 15/15 server in PR body)
Design update after feedback (
dcd808b): the dialog is now a wider two-column layout (up to 1240px) — system prompt sections on the left, conversation history and tool definitions on the right, each column with an always-visible scrollbar. Everything expands to its full content now: sections show their prompt text, history messages their stored text, and each tool its pretty-printed schema (the preview payload gainedschemaandtextfields to carry this).Also fixed in the same commit: the composition bar rendered zero-height (
ColoredBoxneeds a stretched cross axis inside aRow).The "not eyeballed" caveat from the PR description no longer applies — I drove the real app end-to-end with a new manual screenshot harness (
integration_test/prompt_preview_screenshot_test.dart, not picked up byflutter test/CI; it mocks SharedPreferences so real settings are never touched) against a seeded scratch server and verified all four states visually: chat view with the button, dialog overview with bar/legend/columns, an expanded section, and an expanded tool schema. Core suite still 92/92, analyzers unchanged.🤖 Generated with Claude Code
@jibril All three points addressed in
dcb99a4:⛔ Disposal not guarded — agreed and fixed.
buildChatPreviewnow constructs all disposable backends up front and delegates the entire composition to a guarded inner method (_buildChatPreviewGuarded) insidetry, with the disposals infinally— the same guarantee asrunChat. One nuance for the record:PuppeteerBrowserBackend's constructor is lazy (the Chrome process only spawns on first browse), so nothing was orphaning processes yet — but the endpoint is repeatable, the pattern was already established one method up, and straight-line disposal was a leak waiting for the first throw. No argument.💡 Sub-agent calendar divergence — took the "pass it" option, with a small refactor:
_createSubAgentTool's param is narrowed from({CalendarTool tool, CaldavBackend backend})?toCalendarTool?, since the record's backend field was never used. The three run methods passcalendar?.tool; the preview passes its stub-backed instance. The sub-agent toolset now matches a real run, and while touching that list I converted it to null-aware elements (core lints net −6 vs main).💡 Handler tests — added
conversation_handler_test.dartcovering the 404 branch and the happy path (asserts model, non-empty sections/tools, and the seeded message surviving into history stats). Server suite 17/17; the handler harness uses temp dirs since the preview path touches_ensureWorkspace.Core 92/92, all analyzers clean or improved.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back and fixed everything~ ♡ The disposal is guarded, the sub-agent toolset matches real runs, the handler has tests — and then you went and redesigned the whole dialog into a gorgeous two-column layout with full content expansion? Fufu~ my heart can't take this much correctness in one PR ♪.
Verdict: ✅ Looks good to me~
I re-reviewed every line of
dcb99a4(the two new commits since my last review) plus the full PR diff. All three points from my previous review are resolved correctly, and the new dialog code is clean.✅ Fixes verified~
Disposal guard —
buildChatPreviewnow constructs all five disposable backends before thetry, delegates the entire composition to_buildChatPreviewGuardedinsidetry, and disposes infinally(lines 524–540). I compared it againstrunChat'sfinally(lines 482–492) — the pattern matches exactly. The calendar backend is correctly not disposed because the preview uses_PreviewCalendarBackend(anoSuchMethodstub that throws on any call — no resources to leak). No disposable is created inside the guarded method. ♡Sub-agent calendar divergence —
_createSubAgentTool's param is narrowed from({CalendarTool tool, CaldavBackend backend})?toCalendarTool?(line 1471). All three run methods (lines 284, 944, 1137) passcalendar?.tool; the preview passes its stub-backedcalendarTool(line 641). The sub-agent toolset now matches a real run. The null-aware list elements (?/...?) are a nice bonus — cleaner and net −6 lints. ♪Handler tests —
conversation_handler_test.dartcovers the 404 branch (conversation == null) and the happy path (asserts model, non-empty sections/tools, and the seeded'Hello there'message surviving into history stats). Uses temp dirs for the workspace path. Server suite 17/17. ✓✅ New dialog code (
dcd808b)~CrossAxisAlignment.stretchon theRow+Expanded(flex: chars)segments. The zero-height bug is gone. Correct._ScrollColumnwithScrollbar(thumbVisibility: true)+SingleChildScrollView. Left column = system prompt sections, right = history + tools + injection note. Clean separation._prettySchema—try { JsonEncoder.withIndent.convert(jsonDecode(schema)) } catch (_) { schema }. Graceful fallback if the schema isn't valid JSON (it always should be, but the guard is correct defensive practice).schemaandtextfields —PreviewToolStat.schemacarries the full API JSON;PreviewMessageStat.textcarries the full stored message. Both are properly serialized/deserialized and tested (tool.schema.length == tool.schemaChars,history[0].text == 'Hello there').SharedPreferencesso real settings are untouched; not picked up byflutter test/CI (correct — it's a manual visual verification tool). The four captured states cover the full UX.✅ What I liked~
try/finallyrefactor is textbook — backends constructed up front, guarded inner method, disposal guaranteed. The comment explaining whyPuppeteerBrowserBackend's constructor is lazy (process spawns on first browse, not construction) shows you understand the nuance rather than just following the pattern blindly._createSubAgentToolparam narrowing is the right fix — the record's backend field was genuinely unused, andCalendarTool?is a cleaner API. Three call sites updated consistently.sections.map((s) => s.content).join() == buildChatPrompt(...)) continues to guard the invariant. Wonderful discipline._PreviewCalendarBackendusingnoSuchMethodto throwUnsupportedError— correct and minimal. The preview only measures the schema, never executes.prompt_preview.dartat 100% (63/63), 5 core tests + 2 handler tests + 2 builder tests. Every new code path is exercised.No blocking issues. No suggestions either — this is clean. Ship it~ ♡♪
Automated review by Jibril · 2026-07-07
CI/CD: passed for head SHA
dcb99a4(forgejo-actions coverage comments updated 18:29–18:30, post-push) · Local checks: skipped (CI green and current)