feat(ai-tools): draft/ai-tools workflow viewer#217
Conversation
Implements client-side support for the draft/ai-tools v0.4 spec: AI bots stream their workflow state (thinking, tool calls, results) as TAGMSG carrying a +obby.world/ai-tools JSON envelope; we render those workflows in a floating tray pinned to the top-right of the chat area, scoped to the current channel. Each workflow is its own card -- spinner + bot nick + dropdown chevron when collapsed, expands to show a Claude-Code-style timeline of steps (colored dot accent per type, content rendered in a monospace box for both string fragments and tool-call args). Multiple bots → multiple stacked cards, so workflow telemetry never eats into chat real-estate. Control signals (cancel / approve / reject) round-trip via TAGMSG to the bot's nick. Pending-approval steps surface inline buttons inside the card; the card header has a Stop button while running and a dismiss-X after the workflow reaches a terminal state. Includes: - aiTools.ts: decoder/encoder + TS types matching the spec - src/store/handlers/aiTools.ts: TAGMSG/CHANMSG/USERMSG → workflow state machine; string-content fragments concatenate so the ai-tools/content-stream batch reassembles correctly even when processed message-by-message - AiToolsCard + AiToolsTray: the floating UI - ChatArea wires the tray in, scoped to the selected target - draft/ai-tools added to the CAP REQ list - Tests: 21 new (decoder edge cases + handler state transitions) - Translations: all 18 supported locales
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds AI workflow support: typed protocol encode/decode and IRC escaping, new AiToolsCard UI showing collapsible workflow steps with approvals, AiToolsTray that filters/sorts workflows by server/channel and is mounted in ChatArea, header history popover and message pill integration, and translations across many locales. ChangesAI Tools Workflow Visualization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Automated deployment preview for the PR in the Cloudflare Pages. |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/locales/fr/messages.po (1)
54-57: 🏗️ Heavy liftUse ICU pluralization instead of
step(s)in translatable text.
msgid "{0} step(s) awaiting approval"is not fully localizable and will produce awkward grammar in multiple locales. Please switch the source string to ICU plural form and regenerate catalogs.🤖 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 `@src/locales/fr/messages.po` around lines 54 - 57, Replace the non-ICU source string msgid "{0} step(s) awaiting approval" with an ICU pluralized source such as using a named variable (e.g. {count}) and plural rules, update the usage in the component src/components/ui/AiToolsCard.tsx to pass a numeric "count" prop instead of positional {0}, and then regenerate the PO catalogs so msgstr entries use the plural form; ensure the PO entry and component consistently use the same variable name (count) and that the ICU plural covers singular and other cases.
🤖 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 `@src/components/ui/AiToolsCard.tsx`:
- Around line 170-212: The header currently nests action <button>s inside the
outer toggle <button>, causing invalid HTML; replace the outer <button> that
uses onToggle and aria-expanded with a focusable element (e.g., <div> or <button
role="button">) that acts as the collapse toggle (keep onToggle, aria-expanded
and workflowHeaderIcon usage on this element) and move the dismiss and Stop
action buttons (handlers dismiss(workflow.serverId, workflow.id) and onCancel)
outside of that toggle element so they are siblings rather than children; remove
the e.stopPropagation() workaround and ensure the new toggle retains keyboard
accessibility (role and tabIndex/keypress handling) and visual styling
consistent with the original classes.
- Around line 47-53: Add localization to the Step component: import/use
useLingui by adding const { t } = useLingui() inside the Step component and
update the headerLabel useMemo to return localized strings for the fallbacks
(replace "Thinking" with t`Thinking` and "Text" with t`Text`) while keeping
existing logic for step.label, step.type, and step.tool intact.
In `@src/lib/aiTools.ts`:
- Around line 148-164: The switch over character variable c that escapes IRC tag
values omits NUL (\0), so add a case for "\0" that appends "\\0" (e.g., case
"\0": out += "\\0"; break;) to the escaping logic; also update the complementary
unescape routine (the function that handles sequences like "\\n", "\\r", "\\s",
"\\:") to recognize "\\0" and convert it back to "\0" so round-trip
encoding/decoding is complete.
- Around line 84-133: The decoder accepts any string for enum fields and
force-casts them, allowing invalid values through; update decodeAiToolsValue to
explicitly validate enum values before casting: in the "workflow" case check
that obj.state is one of AiWorkflowState (and similarly validate
obj.trigger/name if you have constrained sets), in the "step" case validate
obj.type is a member of AiStepType and obj.state is a member of AiStepState, and
in the "action" case validate obj.action is a member of AiActionType; implement
small type-guard helpers or lookup Sets and return null when a value is not in
the allowed enum set instead of force-casting (apply these checks around the
construction of AiWorkflowMessage, AiStepMessage, and AiActionMessage).
In `@src/locales/en/messages.po`:
- Around line 53-57: The localized catalog (messages.po) is out of sync with
extracted strings (e.g. the msgid "{0} step(s) awaiting approval" used by
AiToolsCard.tsx); run the i18n pipeline to regenerate and commit updated
catalogs by executing `npm run i18n:extract && npm run i18n:compile`, verify
`git diff --exit-code src/locales/en/messages.po` is clean, and commit the
resulting files so CI no longer fails (repeat for the other affected msgid
occurrences).
In `@src/locales/nl/messages.mjs`:
- Line 1: The serialized NL locale contains typos in user-facing strings; locate
the entries with keys "PPqfdA" (currently "Kanaelconfiguratie-instellingen
openen") and "SkZcl+" (currently "Kies een vooraf ingesteld
floodbeveililingsprofiel. Deze profielen bieden evenwichtige
beveiligingsinstellingen voor verschillende toepassingen." where
"floodbeveililingsprofiel" is misspelled) and correct the Dutch text (e.g.,
"Kanaalconfiguratie-instellingen openen" and "floodbeveiligingsprofiel"), then
update the NL source locale and regenerate the translation catalog so the
serialized messages.mjs is rebuilt with the corrected strings.
---
Nitpick comments:
In `@src/locales/fr/messages.po`:
- Around line 54-57: Replace the non-ICU source string msgid "{0} step(s)
awaiting approval" with an ICU pluralized source such as using a named variable
(e.g. {count}) and plural rules, update the usage in the component
src/components/ui/AiToolsCard.tsx to pass a numeric "count" prop instead of
positional {0}, and then regenerate the PO catalogs so msgstr entries use the
plural form; ensure the PO entry and component consistently use the same
variable name (count) and that the ICU plural covers singular and other cases.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1ef2bf0-1cf2-4cf2-ba16-d78ae194a06b
📒 Files selected for processing (48)
src/components/layout/ChatArea.tsxsrc/components/ui/AiToolsCard.tsxsrc/components/ui/AiToolsTray.tsxsrc/lib/aiTools.tssrc/lib/irc/IRCClient.tssrc/locales/cs/messages.mjssrc/locales/cs/messages.posrc/locales/de/messages.mjssrc/locales/de/messages.posrc/locales/en/messages.mjssrc/locales/en/messages.posrc/locales/es/messages.mjssrc/locales/es/messages.posrc/locales/fi/messages.mjssrc/locales/fi/messages.posrc/locales/fr/messages.mjssrc/locales/fr/messages.posrc/locales/it/messages.mjssrc/locales/it/messages.posrc/locales/ja/messages.mjssrc/locales/ja/messages.posrc/locales/ko/messages.mjssrc/locales/ko/messages.posrc/locales/nl/messages.mjssrc/locales/nl/messages.posrc/locales/pl/messages.mjssrc/locales/pl/messages.posrc/locales/pt/messages.mjssrc/locales/pt/messages.posrc/locales/ro/messages.mjssrc/locales/ro/messages.posrc/locales/ru/messages.mjssrc/locales/ru/messages.posrc/locales/sv/messages.mjssrc/locales/sv/messages.posrc/locales/tr/messages.mjssrc/locales/tr/messages.posrc/locales/uk/messages.mjssrc/locales/uk/messages.posrc/locales/zh-TW/messages.mjssrc/locales/zh-TW/messages.posrc/locales/zh/messages.mjssrc/locales/zh/messages.posrc/store/handlers/aiTools.tssrc/store/handlers/index.tssrc/store/index.tstests/lib/aiTools.test.tstests/store/aiTools.test.ts
| switch (obj.msg) { | ||
| case "workflow": { | ||
| if (typeof obj.id !== "string" || typeof obj.state !== "string") | ||
| return null; | ||
| const m: AiWorkflowMessage = { | ||
| msg: "workflow", | ||
| id: obj.id, | ||
| state: obj.state as AiWorkflowState, | ||
| }; | ||
| if (typeof obj.name === "string") m.name = obj.name; | ||
| if (typeof obj.trigger === "string") m.trigger = obj.trigger; | ||
| if (typeof obj["cancelled-by"] === "string") | ||
| m["cancelled-by"] = obj["cancelled-by"] as string; | ||
| return m; | ||
| } | ||
| case "step": { | ||
| if ( | ||
| typeof obj.wid !== "string" || | ||
| typeof obj.sid !== "string" || | ||
| typeof obj.type !== "string" || | ||
| typeof obj.state !== "string" | ||
| ) | ||
| return null; | ||
| const m: AiStepMessage = { | ||
| msg: "step", | ||
| wid: obj.wid, | ||
| sid: obj.sid, | ||
| type: obj.type as AiStepType, | ||
| state: obj.state as AiStepState, | ||
| }; | ||
| if (typeof obj.tool === "string") m.tool = obj.tool; | ||
| if (typeof obj.label === "string") m.label = obj.label; | ||
| if (obj.content !== undefined) m.content = obj.content; | ||
| if (typeof obj.truncated === "boolean") m.truncated = obj.truncated; | ||
| return m; | ||
| } | ||
| case "action": { | ||
| if (typeof obj.action !== "string" || typeof obj.target !== "string") | ||
| return null; | ||
| const m: AiActionMessage = { | ||
| msg: "action", | ||
| action: obj.action as AiActionType, | ||
| target: obj.target, | ||
| }; | ||
| if (typeof obj.content === "string") m.content = obj.content; | ||
| return m; | ||
| } | ||
| default: | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Reject unknown enum values during decode.
decodeAiToolsValue currently accepts any string for state, type, and action (Lines 86–91, 103–112, 121–126) and then force-casts. That bypasses schema validation and can leak invalid protocol states into the store/UI.
Suggested fix
+const WORKFLOW_STATES = new Set<AiWorkflowState>([
+ "start",
+ "thinking",
+ "running",
+ "complete",
+ "failed",
+ "cancelled",
+]);
+const STEP_TYPES = new Set<AiStepType>([
+ "thinking",
+ "tool-call",
+ "tool-result",
+ "text",
+]);
+const STEP_STATES = new Set<AiStepState>([
+ "start",
+ "running",
+ "pending-approval",
+ "complete",
+ "failed",
+ "cancelled",
+]);
+const ACTION_TYPES = new Set<AiActionType>(["cancel", "approve", "reject", "steer"]);
+
switch (obj.msg) {
case "workflow": {
- if (typeof obj.id !== "string" || typeof obj.state !== "string")
+ if (
+ typeof obj.id !== "string" ||
+ typeof obj.state !== "string" ||
+ !WORKFLOW_STATES.has(obj.state as AiWorkflowState)
+ )
return null;
const m: AiWorkflowMessage = {
msg: "workflow",
id: obj.id,
- state: obj.state as AiWorkflowState,
+ state: obj.state as AiWorkflowState,
};
@@
case "step": {
if (
typeof obj.wid !== "string" ||
typeof obj.sid !== "string" ||
typeof obj.type !== "string" ||
- typeof obj.state !== "string"
+ typeof obj.state !== "string" ||
+ !STEP_TYPES.has(obj.type as AiStepType) ||
+ !STEP_STATES.has(obj.state as AiStepState)
)
return null;
@@
case "action": {
- if (typeof obj.action !== "string" || typeof obj.target !== "string")
+ if (
+ typeof obj.action !== "string" ||
+ typeof obj.target !== "string" ||
+ !ACTION_TYPES.has(obj.action as AiActionType)
+ )
return null;🤖 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 `@src/lib/aiTools.ts` around lines 84 - 133, The decoder accepts any string for
enum fields and force-casts them, allowing invalid values through; update
decodeAiToolsValue to explicitly validate enum values before casting: in the
"workflow" case check that obj.state is one of AiWorkflowState (and similarly
validate obj.trigger/name if you have constrained sets), in the "step" case
validate obj.type is a member of AiStepType and obj.state is a member of
AiStepState, and in the "action" case validate obj.action is a member of
AiActionType; implement small type-guard helpers or lookup Sets and return null
when a value is not in the allowed enum set instead of force-casting (apply
these checks around the construction of AiWorkflowMessage, AiStepMessage, and
AiActionMessage).
| switch (c) { | ||
| case "\\": | ||
| out += "\\\\"; | ||
| break; | ||
| case ";": | ||
| out += "\\:"; | ||
| break; | ||
| case " ": | ||
| out += "\\s"; | ||
| break; | ||
| case "\r": | ||
| out += "\\r"; | ||
| break; | ||
| case "\n": | ||
| out += "\\n"; | ||
| break; | ||
| default: |
There was a problem hiding this comment.
Complete IRC tag escaping for NUL (\0).
The function comment says this mirrors the IRC escape/unescape behavior, but Line 148–164 omits \0. If payload content contains NUL, round-tripping is incomplete.
Suggested fix
case "\n":
out += "\\n";
break;
+ case "\0":
+ out += "\\0";
+ break;
default:
out += c;🤖 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 `@src/lib/aiTools.ts` around lines 148 - 164, The switch over character
variable c that escapes IRC tag values omits NUL (\0), so add a case for "\0"
that appends "\\0" (e.g., case "\0": out += "\\0"; break;) to the escaping
logic; also update the complementary unescape routine (the function that handles
sequences like "\\n", "\\r", "\\s", "\\:") to recognize "\\0" and convert it
back to "\0" so round-trip encoding/decoding is complete.
Replaces the flat JSON.stringify <pre> dump with a key→value chip tree. Primitives become inline colored chips (green strings, cyan numbers, purple bools); nested objects/arrays nest under a tinted left rule with their entries laid out one-per-row. Much easier to scan tool-call args once they get more than a couple of fields, which the user noticed when the bot started passing structured options through.
'Workflow history' (button title) and '{0} step(s)' (list row summary)
across all 18 non-English locales.
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/locales/fr/messages.po (1)
749-752:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFill in the missing French translation before committing.
The
msgstrentries are empty for "Workflow history". As per coding guidelines, all translations in.pofiles must be completed before committing. Suggested translation:#: src/components/ui/AiToolsHistoryButton.tsx #: src/components/ui/AiToolsHistoryButton.tsx msgid "Workflow history" -msgstr "" +msgstr "Historique des workflows"🤖 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 `@src/locales/fr/messages.po` around lines 749 - 752, The French translation for msgid "Workflow history" is missing in the .po file; open the entry with msgid "Workflow history" (referenced by ChannelSettingsModal.tsx) and set an appropriate msgstr (e.g., "Historique du flux de travail") to complete the translation, save the .po, and run any localization lint/validation before committing.
🤖 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 `@src/components/message/AiToolsMessagePill.tsx`:
- Around line 72-76: The current Trans usage in AiToolsMessagePill.tsx renders
"View workflow ({stepCount} steps)" which won't pluralize correctly; update the
conditional that uses available, stepCount and Trans to use the i18n plural
helper (pass count={stepCount} and an i18nKey that includes singular/plural
forms) so the message is internationalized correctly across locales — locate the
JSX fragment referencing available, stepCount and Trans and replace it with a
plural-aware Trans invocation or t('key', { count: stepCount }) using the same
key in your translation files.
In `@src/components/ui/AiToolsHistoryButton.tsx`:
- Line 132: Replace the inline "(s)" text with the i18n pluralization helper so
translations handle language-specific plural rules; in AiToolsHistoryButton.tsx
update the Trans usage that currently renders "{w.steps.length} step(s)" to use
the i18n plural/pluralization API (driven by w.steps.length) and an appropriate
i18n key so the rendered string is chosen via plural rules rather than a manual
"(s)". Target the Trans usage around w.steps.length in the AiToolsHistoryButton
component and ensure the new i18n key/translation entries include singular and
plural forms for all locales.
In `@src/locales/de/messages.po`:
- Around line 54-57: The German .po file has empty translations for the msgid
"{0} step(s)" used by AiToolsHistoryButton.tsx (and duplicated at the later
block); update the msgstr entries to a proper German translation (e.g., a
localized plural-aware form such as "{0} Schritt" / "{0} Schritte" according to
your i18n system) for both occurrences, ensure the translation matches the
placeholder "{0}", and re-run npm run i18n:extract / your i18n workflow to
verify no other non-English .po files have empty msgstr entries before
committing.
In `@src/locales/es/messages.po`:
- Around line 54-57: The Spanish PO has empty translations for the new AI Tools
strings (msgid "{0} step(s)" referenced by
src/components/ui/AiToolsHistoryButton.tsx and another identical entry later),
so populate the corresponding msgstr entries with correct Spanish
plural/placeholder-aware translations (e.g., " {0} paso(s)" or a proper
pluralized form like "1 paso" vs "{0} pasos" depending on your project's
pluralization conventions) for both occurrences; ensure the placeholder {0} is
preserved and that all non-English .po files are updated similarly before
committing.
In `@src/locales/fr/messages.po`:
- Around line 54-57: The French translation for the msgid "{0} step(s)" is
missing in src/locales/fr/messages.po; update the msgstr for that entry to the
correct French translation (for example "Étape(s) : {0}" or " {0} étape(s)"
depending on plural styling used in the file) so the entry for msgid "{0}
step(s)" in the file is no longer empty.
In `@src/locales/it/messages.po`:
- Around line 54-57: The Italian locale file has empty translations for the new
AI workflow strings (msgid "{0} step(s)" and msgid "Workflow history")
referenced from src/components/ui/AiToolsHistoryButton.tsx; update messages.po
by providing proper Italian translations for those msgid entries (and any other
empty msgstr entries noted around lines 2749-2753) so they no longer render
untranslated, then run npm run i18n:extract and verify all non-English .po files
have no remaining empty msgstr entries before committing.
In `@src/locales/ja/messages.po`:
- Around line 54-57: The Japanese .po file has empty msgstr entries for newly
added UI strings (e.g. msgid "{0} step(s)" referenced in
src/components/ui/AiToolsHistoryButton.tsx and additional empty entries around
lines 2749-2752), so the UI is not localized; update each empty msgstr with the
correct Japanese translations (keep pluralization/placeholder semantics intact,
e.g. translate "{0} step(s)" into an appropriate Japanese string using the {0}
placeholder), and ensure all other non-English .po files are similarly filled
before committing.
In `@src/locales/ko/messages.po`:
- Around line 54-57: The PO file has empty translations for the AI-tools UI
strings — fill the msgstr for "{0} step(s)" and "Workflow history" (and any
other empty msgstr entries referenced by the extractor) in the Korean
messages.po so the AiToolsHistoryButton.tsx label and the workflow label are
localized; after updating the msgstr entries run npm run i18n:extract to verify
no missing translations remain and commit the updated messages.po.
In `@src/locales/pl/messages.po`:
- Around line 54-57: Fill the missing Polish translations in
src/locales/pl/messages.po for the new AI workflow strings: set msgstr for msgid
"{0} step(s)" and for msgid "Workflow history" (and any other empty msgstr
entries around lines 2749-2753) with the correct Polish phrases, then re-run the
extraction/validation step (npm run i18n:extract) to ensure no other non-English
.po files contain empty msgstr entries before committing.
In `@src/locales/pt/messages.po`:
- Around line 54-57: The pt/messages.po file has empty msgstr entries for new UI
strings (notably the msgid "{0} step(s)" used by
src/components/ui/AiToolsHistoryButton.tsx and other entries around lines
2749-2753), so update pt/messages.po by providing proper Portuguese translations
for each empty msgstr (including the "{0} step(s)" string), then run npm run
i18n:extract to ensure no other non-English .po files have empty msgstr entries
and verify the translated strings render correctly in the app.
In `@src/locales/ru/messages.po`:
- Around line 54-57: Two translations are missing: fill the empty msgstr for
msgid "{0} step(s)" and for msgid "Workflow history" in
src/locales/ru/messages.po by adding proper Russian translations (e.g., use
correct plural handling for "{0} step(s)" and a natural Russian phrase for
"Workflow history"), then re-run the i18n extraction/validation (npm run
i18n:extract) to ensure no other empty msgstr entries remain; locate these by
searching for the exact msgid strings "{0} step(s)" and "Workflow history" in
the file and replace the empty msgstr "" entries with the appropriate Russian
text.
In `@src/locales/sv/messages.po`:
- Around line 54-57: The Swedish locale file is missing translations for the
workflow strings; fill the empty msgstr entries for msgid "{0} step(s)" and
msgid "Workflow history" in src/components/ui/AiToolsHistoryButton.tsx (and the
other occurrences around lines 2749-2753) with proper Swedish translations
(e.g., use plural-aware phrasing like "{0} steg" and "Arbetsflödets historik" or
equivalent), then run npm run i18n:extract to ensure no other non-English .po
files contain empty msgstr entries before committing.
---
Outside diff comments:
In `@src/locales/fr/messages.po`:
- Around line 749-752: The French translation for msgid "Workflow history" is
missing in the .po file; open the entry with msgid "Workflow history"
(referenced by ChannelSettingsModal.tsx) and set an appropriate msgstr (e.g.,
"Historique du flux de travail") to complete the translation, save the .po, and
run any localization lint/validation before committing.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44b51c91-9a30-4eaa-a827-1e6a4f66a502
📒 Files selected for processing (45)
src/components/layout/ChatHeader.tsxsrc/components/message/AiToolsMessagePill.tsxsrc/components/message/MessageItem.tsxsrc/components/ui/AiToolsCard.tsxsrc/components/ui/AiToolsHistoryButton.tsxsrc/locales/cs/messages.mjssrc/locales/cs/messages.posrc/locales/de/messages.mjssrc/locales/de/messages.posrc/locales/en/messages.mjssrc/locales/en/messages.posrc/locales/es/messages.mjssrc/locales/es/messages.posrc/locales/fi/messages.mjssrc/locales/fi/messages.posrc/locales/fr/messages.mjssrc/locales/fr/messages.posrc/locales/it/messages.mjssrc/locales/it/messages.posrc/locales/ja/messages.mjssrc/locales/ja/messages.posrc/locales/ko/messages.mjssrc/locales/ko/messages.posrc/locales/nl/messages.mjssrc/locales/nl/messages.posrc/locales/pl/messages.mjssrc/locales/pl/messages.posrc/locales/pt/messages.mjssrc/locales/pt/messages.posrc/locales/ro/messages.mjssrc/locales/ro/messages.posrc/locales/ru/messages.mjssrc/locales/ru/messages.posrc/locales/sv/messages.mjssrc/locales/sv/messages.posrc/locales/tr/messages.mjssrc/locales/tr/messages.posrc/locales/uk/messages.mjssrc/locales/uk/messages.posrc/locales/zh-TW/messages.mjssrc/locales/zh-TW/messages.posrc/locales/zh/messages.mjssrc/locales/zh/messages.posrc/store/handlers/aiTools.tssrc/store/index.ts
✅ Files skipped from review due to trivial changes (11)
- src/locales/ja/messages.mjs
- src/locales/sv/messages.mjs
- src/locales/nl/messages.mjs
- src/locales/ro/messages.mjs
- src/locales/fr/messages.mjs
- src/locales/fi/messages.po
- src/locales/en/messages.po
- src/locales/nl/messages.po
- src/locales/fi/messages.mjs
- src/locales/ro/messages.po
- src/locales/cs/messages.po
🚧 Files skipped from review as they are similar to previous changes (10)
- src/locales/cs/messages.mjs
- src/locales/pt/messages.mjs
- src/locales/de/messages.mjs
- src/locales/it/messages.mjs
- src/locales/es/messages.mjs
- src/locales/en/messages.mjs
- src/locales/pl/messages.mjs
- src/locales/ru/messages.mjs
- src/locales/ko/messages.mjs
- src/components/ui/AiToolsCard.tsx
CollapsibleMessage renders as a block-level div, so the inline-flex pill stacked above the message body rather than sitting next to it. Float-left makes the body wrap around the chip, prepending it to the first line as intended.
Extends the workflow message schema with an optional `prompt` field that bots can use to ship a short truncated copy of the trigger message. The card renders it in italic muted text under the workflow name so users see what was asked without scrolling back to the trigger PRIVMSG. Field is optional -- bots that don't include it are unaffected, the card just omits the line. Plumbed end-to-end: lib/aiTools.ts decoder, AiWorkflow store type, applyWorkflowUpdate merge, AiToolsCard render.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/components/ui/AiToolsCard.tsx (2)
144-150:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize fallback header labels in
Step.Line 148 and Line 149 still return raw English strings, so these fallbacks won’t be translated.
Suggested fix
const Step: React.FC<{ step: AiStep; isFirst: boolean; isLast: boolean; }> = ({ step, isFirst, isLast }) => { + const { t } = useLingui(); const accent = STEP_TYPE_ACCENT[step.type] ?? STEP_TYPE_ACCENT.text; const headerLabel = useMemo(() => { if (step.label) return step.label; if (step.type === "tool-call" || step.type === "tool-result") return step.tool ? `${step.tool}` : step.type; - if (step.type === "thinking") return "Thinking"; - return "Text"; - }, [step.label, step.tool, step.type]); + if (step.type === "thinking") return t`Thinking`; + return t`Text`; + }, [step.label, step.tool, step.type, t]);As per coding guidelines
src/**/*.{ts,tsx}: Usettemplate literal from@lingui/macrofor simple strings outside JSX (inside render functions) and for variables/interpolation, but never at module scope.🤖 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 `@src/components/ui/AiToolsCard.tsx` around lines 144 - 150, The fallback headerLabel in AiToolsCard.tsx returns raw English strings; update the useMemo so the "Thinking" and "Text" fallbacks use the t template literal from `@lingui/macro` (e.g., t`Thinking`, t`Text`) instead of plain strings, and add an import for { t } from '@lingui/macro' if it's not already present; keep the dependency array ([step.label, step.tool, step.type]) unchanged and ensure you only use t inside the render/function scope (not at module scope).
348-395:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid nested
<button>elements in the header.Line 348 wraps Line 374 and Line 387 buttons inside another button. That’s invalid interactive markup and can break keyboard/focus behavior.
Suggested structural fix
- <button - type="button" - onClick={onToggle} - className="w-full flex items-center gap-2.5 px-3 py-2.5 hover:bg-discord-dark-400/60 transition-colors text-left" - aria-expanded={!workflow.collapsed} - > + <div className="w-full flex items-center gap-2.5 px-3 py-2.5 hover:bg-discord-dark-400/60 transition-colors"> + <button + type="button" + onClick={onToggle} + className="flex-1 min-w-0 flex items-center gap-2.5 text-left" + aria-expanded={!workflow.collapsed} + > <span className="w-6 h-6 flex items-center justify-center shrink-0"> {workflowHeaderIcon(workflow.state)} </span> <div className="flex-1 min-w-0"> @@ <span className="text-discord-text-muted shrink-0 ml-1"> {workflow.collapsed ? ( <FaChevronDown className="text-xs" /> ) : ( <FaChevronUp className="text-xs" /> )} </span> - </button> + </button> + {isTerminal && ( + <button + type="button" + onClick={() => dismiss(workflow.serverId, workflow.id)} + className="text-discord-text-muted hover:text-white p-1 rounded" + aria-label={t`Dismiss`} + > + <FaTimes className="text-xs" /> + </button> + )} + {!isTerminal && ( + <button + type="button" + onClick={onCancel} + className="text-discord-text-muted hover:text-red-400 px-1.5 py-0.5 text-[10px] uppercase tracking-wide rounded" + aria-label={t`Cancel workflow`} + > + <Trans>Stop</Trans> + </button> + )} + </div>🤖 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 `@src/components/ui/AiToolsCard.tsx` around lines 348 - 395, The header currently nests two interactive elements (the dismiss and stop buttons) inside the outer clickable button in AiToolsCard.tsx which breaks accessibility; refactor the outer <button> into a non-nested interactive container (either replace the outer element with a <div> or <header> that has role="button", tabIndex={0}, onClick/onKeyDown handlers and preserves aria-expanded and the onToggle behavior) or move the inner dismiss/stop buttons outside of the outer clickable element so the inner handlers (dismiss(workflow.serverId, workflow.id) and onCancel) are direct children of the header container; keep workflowHeaderIcon(workflow.state), sender display, aria-expanded value, and keyboard activation semantics (onKeyDown handling for Enter/Space) intact so focus and accessibility are preserved.
🤖 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 `@src/locales/fi/messages.po`:
- Around line 805-808: The translation for msgid "Dismiss" in the localization
entry used by AiToolsCard.tsx currently uses the same Finnish word as msgid
"Reject" ("Hylkää"); change the msgstr for "Dismiss" to a distinct Finnish term
(for example "Sulje" or "Ohita") while keeping "Reject" as "Hylkää", and update
the other duplicate block for "Reject" so the two actions read differently in
the UI (ensure you edit the entries for msgid "Dismiss" and msgid "Reject" in
messages.po accordingly).
---
Duplicate comments:
In `@src/components/ui/AiToolsCard.tsx`:
- Around line 144-150: The fallback headerLabel in AiToolsCard.tsx returns raw
English strings; update the useMemo so the "Thinking" and "Text" fallbacks use
the t template literal from `@lingui/macro` (e.g., t`Thinking`, t`Text`) instead
of plain strings, and add an import for { t } from '@lingui/macro' if it's not
already present; keep the dependency array ([step.label, step.tool, step.type])
unchanged and ensure you only use t inside the render/function scope (not at
module scope).
- Around line 348-395: The header currently nests two interactive elements (the
dismiss and stop buttons) inside the outer clickable button in AiToolsCard.tsx
which breaks accessibility; refactor the outer <button> into a non-nested
interactive container (either replace the outer element with a <div> or <header>
that has role="button", tabIndex={0}, onClick/onKeyDown handlers and preserves
aria-expanded and the onToggle behavior) or move the inner dismiss/stop buttons
outside of the outer clickable element so the inner handlers
(dismiss(workflow.serverId, workflow.id) and onCancel) are direct children of
the header container; keep workflowHeaderIcon(workflow.state), sender display,
aria-expanded value, and keyboard activation semantics (onKeyDown handling for
Enter/Space) intact so focus and accessibility are preserved.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1166c30-0994-44fc-bfd3-baebbe85cb4d
📒 Files selected for processing (45)
src/components/layout/ChatArea.tsxsrc/components/message/AiToolsMessagePill.tsxsrc/components/message/MessageItem.tsxsrc/components/ui/AiToolsCard.tsxsrc/lib/aiTools.tssrc/locales/cs/messages.mjssrc/locales/cs/messages.posrc/locales/de/messages.mjssrc/locales/de/messages.posrc/locales/en/messages.mjssrc/locales/en/messages.posrc/locales/es/messages.mjssrc/locales/es/messages.posrc/locales/fi/messages.mjssrc/locales/fi/messages.posrc/locales/fr/messages.mjssrc/locales/fr/messages.posrc/locales/it/messages.mjssrc/locales/it/messages.posrc/locales/ja/messages.mjssrc/locales/ja/messages.posrc/locales/ko/messages.mjssrc/locales/ko/messages.posrc/locales/nl/messages.mjssrc/locales/nl/messages.posrc/locales/pl/messages.mjssrc/locales/pl/messages.posrc/locales/pt/messages.mjssrc/locales/pt/messages.posrc/locales/ro/messages.mjssrc/locales/ro/messages.posrc/locales/ru/messages.mjssrc/locales/ru/messages.posrc/locales/sv/messages.mjssrc/locales/sv/messages.posrc/locales/tr/messages.mjssrc/locales/tr/messages.posrc/locales/uk/messages.mjssrc/locales/uk/messages.posrc/locales/zh-TW/messages.mjssrc/locales/zh-TW/messages.posrc/locales/zh/messages.mjssrc/locales/zh/messages.posrc/store/handlers/aiTools.tssrc/store/index.ts
✅ Files skipped from review due to trivial changes (12)
- src/locales/en/messages.mjs
- src/locales/de/messages.mjs
- src/locales/fi/messages.mjs
- src/locales/nl/messages.mjs
- src/locales/ja/messages.mjs
- src/locales/ru/messages.po
- src/locales/fr/messages.po
- src/locales/ko/messages.po
- src/locales/es/messages.po
- src/locales/cs/messages.po
- src/locales/en/messages.po
- src/locales/de/messages.po
🚧 Files skipped from review as they are similar to previous changes (13)
- src/locales/it/messages.mjs
- src/locales/ko/messages.mjs
- src/locales/ru/messages.mjs
- src/locales/fr/messages.mjs
- src/locales/es/messages.mjs
- src/locales/cs/messages.mjs
- src/locales/pt/messages.mjs
- src/components/message/AiToolsMessagePill.tsx
- src/locales/pl/messages.mjs
- src/components/layout/ChatArea.tsx
- src/lib/aiTools.ts
- src/locales/ja/messages.po
- src/locales/pt/messages.po
| #: src/components/ui/AiToolsCard.tsx | ||
| msgid "Dismiss" | ||
| msgstr "Hylkää" | ||
|
|
There was a problem hiding this comment.
Disambiguate “Dismiss” vs “Reject” translations.
Line 806 (“Dismiss”) and Line 1978 (“Reject”) both resolve to “Hylkää”, which makes two different actions read the same in UI controls.
Suggested fix
#: src/components/ui/AiToolsCard.tsx
msgid "Dismiss"
-msgstr "Hylkää"
+msgstr "Sulje"Also applies to: 1977-1980
🤖 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 `@src/locales/fi/messages.po` around lines 805 - 808, The translation for msgid
"Dismiss" in the localization entry used by AiToolsCard.tsx currently uses the
same Finnish word as msgid "Reject" ("Hylkää"); change the msgstr for "Dismiss"
to a distinct Finnish term (for example "Sulje" or "Ohita") while keeping
"Reject" as "Hylkää", and update the other duplicate block for "Reject" so the
two actions read differently in the UI (ensure you edit the entries for msgid
"Dismiss" and msgid "Reject" in messages.po accordingly).
Going from 60s to 5s means the previous 'fade only in the last 10s' logic would start with the card already half-transparent, so switch the opacity easing to span the full countdown (1.0 -> 0.15) instead.
Live workflows now claim a chat slot the moment the start TAGMSG lands. The slot shows the workflow name + a live preview of the latest step with a spinner, then morphs in place into the bot's final PRIVMSG (carrying the workflow pill) once it arrives -- no row jump. Historical (chathistory-replayed) workflows no longer pop floating tray cards on channel join; they remain in the workflow history popover and the inline pill on the original message.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/components/message/AiToolsPlaceholderBody.tsx`:
- Around line 35-40: The fallback labels "Thinking" and "Text" are hardcoded at
module scope in stepLabel(AiStep) preventing localization; update the code so
these strings are produced inside a component/function body where the lingui t
macro can be used. Either move stepLabel into the component that renders
AiToolsPlaceholderBody (so it can call t`Thinking` and t`Text`) or change
stepLabel to accept localized fallback strings (e.g., stepLabel(step,
thinkingLabel, textLabel)) and call it with t`...` from the component; ensure
any usage of step.tool/tool-call/tool-result still returns the tool or type as
before.
In `@src/components/ui/AiToolsCard.tsx`:
- Around line 306-314: The useEffect that counts down (references secondsLeft,
paused, dismiss, workflow.serverId, workflow.id) should not include the unstable
store action ref dismiss in its dependency array; remove dismiss from the array
and add the ESLint/React Hooks suppression comment immediately above the
useEffect (e.g. // eslint-disable-next-line react-hooks/exhaustive-deps) to
prevent linting errors while keeping secondsLeft, paused, workflow.serverId and
workflow.id as dependencies so the countdown logic remains correct.
- Around line 321-330: The auto-scroll check runs after DOM updates so it misses
cases where new content grows past the 24px tolerance; fix by tracking the
previous scrollHeight and computing "wasAtBottom" against that value before
deciding to auto-scroll. Add a ref like prevScrollHeightRef and update it
before/after renders (or switch the effect to useLayoutEffect) and replace the
atBottom computation to use prevScrollHeightRef.current - (el.scrollTop +
el.clientHeight) <= SCROLL_TOLERANCE; if wasAtBottom then set el.scrollTop =
el.scrollHeight, and finally set prevScrollHeightRef.current = el.scrollHeight;
keep references to bodyRef, workflow.collapsed and workflow.steps.length to
locate the logic in AiToolsCard.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: defdb157-72de-4c8d-b0d6-fa66044a41ee
📒 Files selected for processing (48)
src/components/message/AiToolsMessagePill.tsxsrc/components/message/AiToolsPlaceholderBody.tsxsrc/components/message/MessageItem.tsxsrc/components/ui/AiToolsCard.tsxsrc/components/ui/AiToolsHistoryButton.tsxsrc/components/ui/AiToolsTray.tsxsrc/locales/cs/messages.mjssrc/locales/cs/messages.posrc/locales/de/messages.mjssrc/locales/de/messages.posrc/locales/en/messages.mjssrc/locales/en/messages.posrc/locales/es/messages.mjssrc/locales/es/messages.posrc/locales/fi/messages.mjssrc/locales/fi/messages.posrc/locales/fr/messages.mjssrc/locales/fr/messages.posrc/locales/it/messages.mjssrc/locales/it/messages.posrc/locales/ja/messages.mjssrc/locales/ja/messages.posrc/locales/ko/messages.mjssrc/locales/ko/messages.posrc/locales/nl/messages.mjssrc/locales/nl/messages.posrc/locales/pl/messages.mjssrc/locales/pl/messages.posrc/locales/pt/messages.mjssrc/locales/pt/messages.posrc/locales/ro/messages.mjssrc/locales/ro/messages.posrc/locales/ru/messages.mjssrc/locales/ru/messages.posrc/locales/sv/messages.mjssrc/locales/sv/messages.posrc/locales/tr/messages.mjssrc/locales/tr/messages.posrc/locales/uk/messages.mjssrc/locales/uk/messages.posrc/locales/zh-TW/messages.mjssrc/locales/zh-TW/messages.posrc/locales/zh/messages.mjssrc/locales/zh/messages.posrc/store/handlers/aiTools.tssrc/store/handlers/index.tssrc/store/index.tssrc/types/index.ts
✅ Files skipped from review due to trivial changes (14)
- src/locales/de/messages.mjs
- src/locales/pt/messages.mjs
- src/locales/en/messages.mjs
- src/locales/ro/messages.mjs
- src/locales/ru/messages.mjs
- src/locales/nl/messages.mjs
- src/locales/ko/messages.mjs
- src/locales/fr/messages.mjs
- src/locales/fr/messages.po
- src/locales/ja/messages.po
- src/locales/nl/messages.po
- src/locales/de/messages.po
- src/locales/it/messages.po
- src/locales/fi/messages.po
🚧 Files skipped from review as they are similar to previous changes (15)
- src/components/ui/AiToolsTray.tsx
- src/locales/it/messages.mjs
- src/components/message/MessageItem.tsx
- src/locales/es/messages.mjs
- src/locales/fi/messages.mjs
- src/locales/pl/messages.mjs
- src/locales/ro/messages.po
- src/components/message/AiToolsMessagePill.tsx
- src/locales/es/messages.po
- src/locales/cs/messages.mjs
- src/locales/ja/messages.mjs
- src/locales/en/messages.po
- src/locales/pt/messages.po
- src/components/ui/AiToolsHistoryButton.tsx
- src/locales/ko/messages.po
| function stepLabel(step: AiStep): string { | ||
| if (step.label) return step.label; | ||
| if (step.type === "tool-call" || step.type === "tool-result") | ||
| return step.tool ?? step.type; | ||
| if (step.type === "thinking") return "Thinking"; | ||
| return "Text"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/message/AiToolsPlaceholderBody.tsxRepository: ObsidianIRC/ObsidianIRC
Length of output: 3112
Localize the fallback step labels.
stepLabel() hardcodes "Thinking" and "Text" at module scope, preventing these user-visible fallback labels from being localized. Move the label selection into the component body (or refactor to pass localized strings) so these strings can use t from @lingui/macro.
Per coding guidelines: src/**/*.{ts,tsx} must use t template literal for simple strings, never at module scope—move strings inside function bodies if needed.
🤖 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 `@src/components/message/AiToolsPlaceholderBody.tsx` around lines 35 - 40, The
fallback labels "Thinking" and "Text" are hardcoded at module scope in
stepLabel(AiStep) preventing localization; update the code so these strings are
produced inside a component/function body where the lingui t macro can be used.
Either move stepLabel into the component that renders AiToolsPlaceholderBody (so
it can call t`Thinking` and t`Text`) or change stepLabel to accept localized
fallback strings (e.g., stepLabel(step, thinkingLabel, textLabel)) and call it
with t`...` from the component; ensure any usage of
step.tool/tool-call/tool-result still returns the tool or type as before.
…holder Two issues with the in-chat placeholder flow: 1. When the bot's tagged PRIVMSG morphs the placeholder, we were not resolving the +reply tag, so the morphed row had no reply quote block (whereas an untagged reply landed via the normal path with quote intact). Now resolveReplyMessage + extractMentions run on the morph path too. 2. If the bot's final PRIVMSG doesn't carry the workflow tag (only a terminal-state TAGMSG is sent), the morph path never fires and the placeholder is stuck on a "Thinking…" spinner while the bot's actual reply lands as a separate row. On terminal-state TAGMSG, remove the still-pending placeholder so the bot's reply can land normally with full reply context.
The tray filters out historical workflows so chathistory replay doesn't pop a wall of cards on channel join, but the inline pill's reopen action was leaving `historical` set -- meaning even after explicit click the card stayed filtered out and nothing happened. Clear it on reopen so an explicit view request always surfaces the card.
The pill used to sit in a flex column to the left of the body, which pushed the entire message content sideways and made the reply look visually off-axis from neighbouring messages. Strip it to a bare clickable icon (step count + name already live on the floating card and history popover) and absolutely-position it in the avatar gutter so the body keeps its natural alignment.
…g from count Two changes to how workflow steps are presented: 1. Display: a tool-call and its matching tool-result now render as a single row -- tool name in the header, then IN (call args) and OUT (result content) stacked, matching the Claude Code "Bash → Commit and push" pattern. Pairing is FIFO by tool name; an orphan result (no preceding call) still renders. Thinking / text rows are unchanged. 2. Counting: countableSteps() now powers the inline pill tooltip and the history popover label. It excludes thinking (model muttering, not work) and counts a tool-call/result pair as one step instead of two.
The auto-scroll effect only fired when the user was already at the bottom, which never happens on initial expand (scrollTop is 0). Track a "has-done-initial-scroll" flag per expansion: force-scroll to the bottom the first time the body mounts/expands, then fall back to the at-bottom check for subsequent step updates so a user scrolled-up to review earlier output isn't yanked away.
…check The previous approach checked "is the user at the bottom?" inside the content-update effect, but by then the new content had already grown scrollHeight -- the old scrollTop no longer qualified as bottom, so we never auto-scrolled even when the user was parked there. Track stickiness via the scroll event instead: whenever the user (or we) scrolls, recompute whether they're at the bottom and stash that into a ref. On content update, honour the flag. Reset to sticky on collapse so a fresh expansion always lands on the latest content.
|
link in description is broken. Do/should we have a repo for the specs? |
Summary
Client-side implementation of
draft/ai-toolsv0.4. AI bots stream their workflow state — thinking, tool calls, results — as TAGMSG carrying a+obby.world/ai-toolsJSON envelope. We render those workflows in a floating tray pinned to the top-right of the chat area, scoped to the current channel.UX
Protocol
draft/ai-toolsadded to CAP REQ+obby.world/ai-toolstag value (already IRC-unescaped byparseMessageTags; we just JSON.parse + schema-validate)ai-tools/content-streambatch reassembles correctly even when processed message-by-messageTest plan
tests/lib/aiTools.test.ts— 12 decoder/encoder cases (workflow / step / action, malformed JSON, missing fields, IRC escape round-trip, content-stream concat)tests/store/aiTools.test.ts— 9 handler cases (state transitions, idempotent step updates, dismiss flag preserves state)draft-ai-toolsmodule on obby.t3ks.com, exercise a running botSummary by CodeRabbit
New Features
Localization