refactor(site/src/pages/AgentsPage): clean up RenderBlock types and dead fields#23175
Merged
Conversation
b372920 to
0e3cc85
Compare
677c711 to
08421e5
Compare
Base automatically changed from
refactor/chat-message-part-discriminated-union
to
main
March 18, 2026 09:27
f88b532 to
4e9c9ae
Compare
DanielleMaywood
approved these changes
Mar 18, 2026
Comment on lines
85
to
-117
| return ( | ||
| <div className="w-full"> | ||
| {hasText ? ( | ||
| <button | ||
| type="button" | ||
| aria-expanded={isOpen} | ||
| aria-controls={id} | ||
| className="flex items-center gap-2 bg-transparent border-0 p-0 text-content-secondary transition-colors hover:text-content-primary cursor-pointer" | ||
| onClick={() => setIsOpen((prev) => !prev)} | ||
| > | ||
| {labelContent} | ||
| <ChevronDownIcon | ||
| className={cn( | ||
| "h-3 w-3 shrink-0 text-content-secondary transition-transform", | ||
| isOpen ? "rotate-0" : "-rotate-90", | ||
| )} | ||
| /> | ||
| </button> | ||
| ) : ( | ||
| <div className="flex items-center gap-2 text-content-secondary transition-colors hover:text-content-primary"> | ||
| {labelContent} | ||
| </div> | ||
| )} | ||
| {isOpen && hasText ? ( | ||
| <div id={id} className="mt-1.5"> | ||
| <Response | ||
| className="text-[11px] text-content-secondary" | ||
| urlTransform={urlTransform} | ||
| > | ||
| {displayText} | ||
| </Response> | ||
| </div> | ||
| ) : null} |
Contributor
There was a problem hiding this comment.
Assuming you've verified we haven't lost any useful functionality here, I'm happy
Member
Author
There was a problem hiding this comment.
I didn't find any issues, this should all be dead code (e.g. reasoning title never set).
kylecarbs
approved these changes
Mar 18, 2026
…ead fields RenderBlock's file-reference variant used camelCase field names (fileName, startLine, endLine) while the API sends snake_case. The parser destructured and renamed every field. The file variant was defined inline duplicating ChatFilePart. Replace both inline definitions with direct references to the generated ChatFilePart and ChatFileReferencePart types. The file-reference parser case becomes a pass-through. The file case now constructs a typed object instead of casting Record<string, unknown>. Remove dead fields confirmed by Kyle: - title on thinking/reasoning blocks (never populated by backend) - text on file-reference blocks (abandoned file comment feature) Without title, ReasoningDisclosure's disclosure button path was dead code. Removed the button, its useState, and ChevronDownIcon. The component now renders inline text or a streaming placeholder. Cascading dead code: asOptionalTitle, appendParsedTextBlock wrapper, appendStreamTextBlock alias, createBlock helper (identical branches), joinText parameter (no production caller). Refs #23168
4e9c9ae to
77fe83d
Compare
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
PR 2 in a 3-PR series to re-architect
ChatMessagePartfrom a flat interface into a discriminated union.RenderBlockhad inline type definitions that duplicated the generatedChatFilePartandChatFileReferencePart, a camelCase divergence in thefile-reference variant (
fileName,startLine,endLinevs the API'ssnake_case), and two dead fields (
titleon thinking blocks,textonfile-reference blocks).
Replace the inline definitions with direct references to the generated
types. The file-reference parser case becomes a pass-through instead of
destructuring and renaming every field. The file case now constructs a
typed object instead of casting
Record<string, unknown>, which brokeafter
ChatFilePartgainedreadonlyfields from the codegen.Remove dead fields Kyle confirmed:
titleon reasoning: "a janky remnant of an attempt I made to enhance reasoning blocks"texton file-reference: "It was a 'file comment' but I thought that was too broad and lazy"Without
title,ReasoningDisclosure's!title && hasTextguard alwaystook the early return (inline text rendering). The disclosure button, its
useState, and theChevronDownIconimport were all dead. Removed them.The component now has two paths: inline text when content exists, streaming
placeholder otherwise.
Cascading cleanup:
asOptionalTitle(only consumer was title extraction),appendParsedTextBlock(pass-through wrapper after title removal),appendStreamTextBlockalias,createBlockhelper (identical branchesafter title removal), unused
joinTextparameter onappendTextBlock.Refs #23168
🤖 This PR was created with the help of AI, and has been reviewed by a human. 🏂🏻