Unified input attachments with extensible plugin system#42
Conversation
Replace dual image/quote state management in ChatInput with a unified
InputAttachment model backed by a plugin registry, mirroring the
existing message plugin architecture.
- InputAttachment is now a generic { kind, data } envelope
- useInputAttachments hook manages lifecycle and registry-delegated cleanup
- InputAttachmentPlugin interface: renderPreview, toSendContribution, cleanup
- Built-in plugins for image, file, and quote kinds
- inputAttachmentRegistry merges builtins + app plugins (same pattern as pluginRegistry)
- ChatInput delegates preview rendering and send conversion to the registry
- addInputAttachment callback threaded through PluginViewProps → PluginRenderer →
MessageRow → ChatViewport → page.tsx so message plugins can inject attachments
- Demo context_chip plugin: renders Attach button in chat, adds prompt_context
to compose bar on click, renders as styled chip in sent user messages
- UserTextWithQuotes detects [context: label] headers in quoted blocks
and renders them as document-style cards instead of plain blockquotes
- Plugin system docs updated with full Input Attachment Plugins section
- 32 new tests across 4 test files covering hook, registry, all plugins,
context chip rendering, and demo mode integration
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust and extensible plugin system for managing input attachments in the chat composer. By unifying image and quote handling under a generic Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In ChatInput,
hasContentrecomputestoSendContributionfor every attachment on each render and only checks forimages, so text-only attachments (e.g. quotes orprompt_context) don't count as content; consider tracking a simplehasContentfulAttachmentsflag in the hook or extending the plugin interface with a cheaphasContent/kindcheck instead of callingtoSendContributionin render. - The
InputAttachmentParseResulttype ininputAttachmentTypes.tsis currently unused; either wire it into the plugin flow (e.g. for validating attachmentdatalike message plugins do) or remove it to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ChatInput, `hasContent` recomputes `toSendContribution` for every attachment on each render and only checks for `images`, so text-only attachments (e.g. quotes or `prompt_context`) don't count as content; consider tracking a simple `hasContentfulAttachments` flag in the hook or extending the plugin interface with a cheap `hasContent`/`kind` check instead of calling `toSendContribution` in render.
- The `InputAttachmentParseResult` type in `inputAttachmentTypes.ts` is currently unused; either wire it into the plugin flow (e.g. for validating attachment `data` like message plugins do) or remove it to avoid confusion.
## Individual Comments
### Comment 1
<location path="components/ChatInput.tsx" line_range="345-349" />
<code_context>
const isPill = scrollPhase === "pill";
- const hasContent = !!value.trim() || attachments.length > 0;
+ const hasContent = !!value.trim() || attachments.some((att) => {
+ const plugin = inputAttachmentRegistry.get(att.kind);
+ if (!plugin) return false;
+ const c = plugin.toSendContribution(att.data);
+ return !!c.images?.length;
+ });
</code_context>
<issue_to_address>
**issue (bug_risk):** Send-button `hasContent` ignores text-only attachment contributions.
The current `hasContent` check only looks at `c.images?.length`, so attachments that contribute only `textPrefix` (e.g. `quote`, `prompt_context`) won’t enable the send button when the textarea is empty. But `submit()` still sends when `attachments.length > 0`, so the button state can disagree with what’s actually sendable.
To align the UI with the submit behavior, consider treating any non-empty send contribution as content, for example:
```ts
const hasContent = !!value.trim() || attachments.some((att) => {
const plugin = inputAttachmentRegistry.get(att.kind);
if (!plugin) return false;
const c = plugin.toSendContribution(att.data);
return !!c.images?.length || !!c.textPrefix?.trim();
});
```
</issue_to_address>
### Comment 2
<location path="lib/demoMode.ts" line_range="693" />
<code_context>
return RESPONSES.weather;
if (lower.includes("plugin") || lower.includes("widget"))
return buildPluginDemoResponse();
+ if (lower.includes("attach") || lower.includes("context chip"))
+ return buildAttachDemoResponse();
if (lower.includes("pause") || lower.includes("approval"))
</code_context>
<issue_to_address>
**issue (bug_risk):** Demo keyword matching doesn’t align with the documented `context` trigger.
The help text lists `attach / context` as triggers, but the matcher only looks for `"attach"` and `"context chip"`, so typing just `context` won’t match. To keep behavior consistent with the docs, please include plain `"context"` in the check, e.g.:
```ts
if (lower.includes("attach") || lower.includes("context chip") || lower.includes("context"))
return buildAttachDemoResponse();
```
If you’re concerned about collisions with other demos, you could tighten the matching logic (e.g., word boundaries or more specific phrases).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
components/ChatInput.tsx
Outdated
| const hasContent = !!value.trim() || attachments.some((att) => { | ||
| const plugin = inputAttachmentRegistry.get(att.kind); | ||
| if (!plugin) return false; | ||
| const c = plugin.toSendContribution(att.data); | ||
| return !!c.images?.length; |
There was a problem hiding this comment.
issue (bug_risk): Send-button hasContent ignores text-only attachment contributions.
The current hasContent check only looks at c.images?.length, so attachments that contribute only textPrefix (e.g. quote, prompt_context) won’t enable the send button when the textarea is empty. But submit() still sends when attachments.length > 0, so the button state can disagree with what’s actually sendable.
To align the UI with the submit behavior, consider treating any non-empty send contribution as content, for example:
const hasContent = !!value.trim() || attachments.some((att) => {
const plugin = inputAttachmentRegistry.get(att.kind);
if (!plugin) return false;
const c = plugin.toSendContribution(att.data);
return !!c.images?.length || !!c.textPrefix?.trim();
});![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a robust and extensible input attachment plugin system, mirroring the existing message plugin architecture. This is a significant architectural improvement that centralizes attachment handling, making it easier to add new types of content to the compose bar. The changes are well-documented in docs/plugin-system.md and thoroughly tested with new unit tests, covering the core functionality of the new hooks, plugins, and registry. The integration into the ChatInput and MessageRow components is well-executed, and the demo mode updates provide a clear example of the new system in action.
components/ChatInput.tsx
Outdated
| const hasContent = !!value.trim() || attachments.some((att) => { | ||
| const plugin = inputAttachmentRegistry.get(att.kind); | ||
| if (!plugin) return false; | ||
| const c = plugin.toSendContribution(att.data); | ||
| return !!c.images?.length; |
There was a problem hiding this comment.
The hasContent variable currently checks if there's trimmed text or if any attachment contributes images. However, attachments like the context_chip (prompt_context) contribute only a textPrefix and no images. In such cases, if the input field is empty and only a prompt_context attachment is present, hasContent will evaluate to false.
This leads to the send button being disabled, preventing the user from sending a message that consists solely of a text-prefix-only attachment. The submit function's guard (if (!t && attachments.length === 0) return;) correctly allows submission if there are attachments, but the UI's isActive logic (which depends on hasContent) incorrectly disables the button.
To fix this, hasContent should reflect whether there is any content to send, which includes text in the input field OR any attachments, regardless of whether they contribute images or text prefixes.
const hasContent = !!value.trim() || attachments.length > 0;
- Simplify hasContent to treat any attachment as sendable content, fixing send button staying disabled for text-only attachments - Add "context" keyword to demo mode matcher to match documented triggers - Remove unused InputAttachmentParseResult type
1 participant
Summary
InputAttachmentmodel backed by a plugin registry, mirroring the existing message plugin architectureInputAttachmentPlugininterface (renderPreview,toSendContribution,cleanup) with a registry combining built-in + app pluginsaddInputAttachmentthrough the message plugin rendering chain so message plugins can inject compose-bar attachmentscontext_chipmessage plugin that attaches context to the compose bar, rendering as a styled card in sent user messagesdocs/plugin-system.mdTest plan
pnpm build— type-check + production build passespnpm test— 245 tests pass (32 new)make build-web— iOS web build succeeds>prefix + image contentattachin demo mode → context chip appears → click Attach → chip appears in compose bar → send → styled card in user bubble