[Enhancement] - End of Message Response

[leekahung]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update
• Infrastructure
• Maintenance

Description

This PR adds a done chunk to signal stream completion. The backend appends {"type": "done"} after all content blocks, and the frontend uses it to trigger the onDone callback rather than relying on the stream closing.

Also fixes the letter panel never appearing: custom stream chunks from generate_letter were being wrapped as NonStandardContentBlock(type="non_standard") instead of passed through directly, so letter content was silently dropped before reaching the frontend.

Another bug fix was the Generate Letter button not showing the message window. The main cause was the route for the Link set in InitializationForm was set to "letter" instead of "/letter", causing the route to jump to /chat/letter instead of /letter, the proper route for Generate Letter. Included a new unit test to cover the Generate Letter bug.

Related Tickets & Documents

• Related Issue End of Stream Message #309
• Closes End of Stream Message #309

QA Instructions, Screenshots, Recordings

Please replace this line with instructions on how to test your changes, a note on the devices and browsers this has been tested on, as well as any relevant images for UI changes.

No visual changes, but the plumbing for the new chunk for "Done" should appear in our backend debugging logs following the other response chunks and at the very end.

Added/updated tests?

• Yes
• No, and this is why: please replace this line with details on why tests have not been included
• I need help with writing tests

Documentation

• If this PR changes the system architecture, Architecture.md has been updated

[optional] Are there any post deployment tasks we need to perform?

[leekahung]

@claude pr-check

[leekahung]

@claude, pr-check, I've just finished resolving a merge conflict

[claude]

Claude finished @leekahung's task in 1m 35s —— View job

---

PR Review: End of Message Response

• Gather context
• Read changed files
• Review backend changes
• Review frontend changes
• Review tests
• Post final review

---

The PR is clean and well-structured. The core fix — passing custom tool chunks through directly instead of wrapping them in NonStandardContentBlock — is the right call and unblocks letter generation. The done chunk mechanism is a solid improvement over relying on stream close for completion signaling.

A few observations:

Minor issues

1. Misleading debug log (chat.py:77): The done_chunk is appended using the same log template as stream blocks — "Received content_block: {done_chunk}" — but the done chunk is not received from the stream; it's synthesized locally. Something like "Sending done chunk" would be clearer.
   Fix this →

2. Test placement (test_chat.py:48): test_done_chunk_serializes_correctly lives in TestClassifyBlocks but tests DoneChunk schema serialization, not _classify_blocks. It fits better in TestDispatchRequest (which tests the HTTP layer that actually emits the done chunk) or as a standalone schema test.
   Fix this →

3. Dropped-connection UX (Letter.tsx:83-88): The setTimeout for clearing the spinner fires regardless of whether onDone was called. On a dropped connection the user sees the spinner disappear (success-looking behaviour) with no error message — only a console.warn. This is a deliberate trade-off per the updated test, but worth noting in case a follow-up issue wants better error surface here.

What's solid

• The cast(ContentBlock, chunk) fix in langchain_chat_manager.py:189 is the key bug fix — the previous NonStandardContentBlock wrapping swallowed letter content silently.
• useLetterContent type guard (chunk): chunk is LetterChunk is a nice cleanup.
• Test coverage is thorough: onDone called / not called, done chunk not appended to message content, letter chunk still appended, dropped connection warning.
• The system prompt addition ("Letter content must always be passed to the generate_letter tool") is a good defensive layer on top of the code fix.
• Architecture.md is updated to match the new streamText signature.

None of the above are blockers. Good to merge after addressing the log message if desired.

[leekahung]

@claude pr-check

[claude]

Claude finished @leekahung's task in 1m 35s —— View job

---

PR Review: End of Message Response

• Gather context
• Read changed files
• Review backend changes
• Review frontend changes
• Review tests
• Post final review

---

This is a clean PR. The previous review's two flagged issues (log message wording, test placement) have both been addressed. Here's the full fresh pass:

What's fixed

• cast(ContentBlock, chunk) in langchain_chat_manager.py:120 — the core bug fix. The previous NonStandardContentBlock wrapping silently swallowed letter content; passing the dict through directly means _classify_blocks can now match on "letter" correctly.
• Route fix "letter" → "/letter" in InitializationForm.tsx:257 — absolute path now navigates to /letter instead of /chat/letter.
• done chunk appended after stream close — cleaner than relying on stream-end for completion signaling.

Minor observations (non-blocking)

1. cast(ContentBlock, chunk) in langchain_chat_manager.py:120 — ContentBlock is a TypedDict from LangChain, so this is a runtime no-op cast. It works because {"type": "letter", "content": "..."} is structurally compatible. Worth noting in a comment that this relies on the tool emitting a dict with the exact keys _classify_blocks expects — a future tool change could silently break it. Low risk for now.

2. Architecture.md code snippet — the updated snippet uses // ... to abbreviate error-handling and null-reader blocks (lines ~355–395). This makes the example non-runnable as documentation. The removed sections are meaningful (null reader path, catch block) — either restore them or note explicitly that it's pseudocode. Minor documentation nit.
   Fix this →

3. Spinner cleared regardless of outcome in Letter.tsx:88-91 — setTimeout(() => setIsGenerating(false), 500) fires whether cleanCompletion is true or false, so the spinner clears on error too. The error message is already set before the timeout, so the UX is fine — user sees the error message with no spinner, which is reasonable. Just flagging it as a conscious choice if someone wonders why there's no persistent error state.

What's solid

• _classify_blocks simplification — removing the non_standard indirection makes the case match readable and the data flow obvious.
• (chunk): chunk is LetterChunk type guard in useLetterContent.tsx — proper narrowing instead of a cast.
• onDone callback design over return value — avoids the caller needing to inspect a boolean | undefined return to determine success.
• console.warn on stream-end without done chunk — dropped connections are now detectable in browser devtools.
• Test coverage is thorough: onDone called / not called, done chunk not in message content, letter chunk still appended, dropped connection warning, and the new routing regression test for href="/letter".
• System prompt guardrail ("Letter content must always be passed to the generate_letter tool") is a good defensive layer.

No blockers. Good to merge.

[yangm2]

Since #304 already merged a fix for the backend piece of the letter type ... I'm not sure why this change is necessary in this PR.

[yangm2]

I don't think this is necessary. This code is working, unless you're saying that it breaks something else. Plus it's not clear that this type-cast makes sense.

[leekahung]

Think some of the behavior that broke the build came pre-merge conflict resolution.

Patched to the code to use the destructure pattern and included a few debug loggers to better show what might be happening in development, I'll see what I can revert

[yangm2]

I think we'll need this destructuring pattern when we use a similar approach for other tools (e.g. citation type).

[leekahung]

I'll see if I can incorporate this pattern to capture content blocks coming from tools.

[leekahung]

@claude pr-check

[claude]

Claude finished @leekahung's task in 1m 55s —— View job

---

PR Review: End of Message Response

• Gather context
• Read changed files
• Review backend changes
• Review frontend changes
• Review tests
• Post final review

---

All of yangm2's review feedback has been addressed. Here's the current state:

Addressed feedback

• DoneChunk → EndOfStreamChunk ✓
• Letter constraint moved to generate_letter docstring (out of system_prompt.md) ✓
• Architecture.md code block removed — replaced with a single prose paragraph ✓
• non_standard destructuring pattern added to _classify_blocks (chat.py:32-45) — extensible for future tool types (citations, etc.) ✓
• langchain_chat_manager.py wraps custom chunks in NonStandardContentBlock rather than passing raw dicts as ContentBlock ✓

What's solid

• The NonStandardContentBlock wrapping + inner-match in _classify_blocks is clean. Adding a new tool chunk type (e.g. citation) now only requires adding a case "citation" inside the non_standard branch — no changes elsewhere.
• test_streaming_custom_chunk_yields_non_standard_block test docstring is accurate: it validates the contract that langchain_chat_manager wraps custom chunks in NonStandardContentBlock so _classify_blocks can identify their origin.
• test_non_standard_letter_block_routed_correctly and test_non_standard_unknown_inner_type_is_skipped round-trip the full two-layer routing in _classify_blocks.
• Route fix ("letter" → "/letter") is minimal and correct; the regression test in InitializationForm.test.tsx pins it.
• streamText done-chunk handling is clean: receivedDone flag + onDone?.() on receipt, console.warn on stream-end without it.

One minor observation (non-blocking)

EndOfStreamChunk (schema.py:21) emits {"type": "done"} on the wire. The class name and the wire-format value now diverge slightly — if the wire format ever changes to "end_of_stream" for consistency, both streamHelper.ts:67 and the frontend types would need updating. Not an issue for this PR, just worth keeping in mind if the protocol ever gets versioned.

No blockers. The PR is in good shape to merge.

[yangm2]

Python looks good. Someone else should review the frontend.