✨(front) add co2 tooltip - #590
Merged
Merged
Conversation
|
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:
WalkthroughAdds CO2 impact extraction from message annotations, synchronizes assistant annotations into chat state, renders a responsive energy indicator in messages, and adds coverage for utilities and UI interactions. It also updates feedback button styling and the changelog. ChangesCO2 Energy Indicator Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AiSdk as "`@ai-sdk/react`"
participant useChat
participant ChatState as "Chat state"
participant MessageItem
participant MessageEnergyIndicator
participant ImpactCO2 as "ImpactCO2 widget"
AiSdk->>useChat: onFinish(message with annotations)
useChat->>ChatState: update latest assistant annotations
useChat->>AiSdk: invoke original callback
ChatState->>MessageItem: render updated message
MessageItem->>MessageEnergyIndicator: pass co2ImpactKg
MessageEnergyIndicator->>ImpactCO2: inject widget script and data-search
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
elvoisin
force-pushed
the
evoisin/feature-co2
branch
from
July 6, 2026 13:10
565f7e3 to
f1d2e24
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@CHANGELOG.md`:
- Around line 99-101: The unreleased changelog section contains a duplicated
bullet entry, so remove the repeated item and keep only one copy of the note.
Update the changelog entries in the affected section so the same release item
does not appear twice, preserving the other unique bullets around the duplicate.
In
`@src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`:
- Line 12: Remove the unused Box import from MessageEnergyIndicator so the
component only imports what it actually uses; keep Text if it is referenced and
leave the rest of the component unchanged. Use the MessageEnergyIndicator
component as the locator when editing the import list.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3858fea4-d093-4d55-ba00-f776c447bb57
⛔ Files ignored due to path filters (2)
src/frontend/apps/conversations/src/assets/icons/uikit-custom/arrow-up-right.svgis excluded by!**/*.svgsrc/frontend/apps/conversations/src/assets/icons/uikit-custom/leaves.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
CHANGELOG.mdsrc/frontend/apps/conversations/src/custom-next.d.tssrc/frontend/apps/conversations/src/features/chat/api/useChat.tsxsrc/frontend/apps/conversations/src/features/chat/components/Chat.tsxsrc/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageItem.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsxsrc/frontend/apps/conversations/src/features/chat/fixtures/fakeCo2Message.tssrc/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts
💤 Files with no reviewable changes (1)
- src/frontend/apps/conversations/src/custom-next.d.ts
|
|
||
| import ArrowUpRightIcon from '@/assets/icons/uikit-custom/arrow-up-right.svg'; | ||
| import LeavesIcon from '@/assets/icons/uikit-custom/leaves.svg'; | ||
| import { Box, Text } from '@/components'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused Box import.
Box isn't referenced in this component, so strict lint/noUnusedLocals checks will fail if they are enabled.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 12-12: Remove this unused import of 'Box'.
🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`
at line 12, Remove the unused Box import from MessageEnergyIndicator so the
component only imports what it actually uses; keep Text if it is referenced and
leave the rest of the component unchanged. Use the MessageEnergyIndicator
component as the locator when editing the import list.
Source: Linters/SAST tools
elvoisin
force-pushed
the
evoisin/feature-co2
branch
2 times, most recently
from
July 6, 2026 14:25
f296ef8 to
0e0462e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsx (1)
526-552: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the
co2_impact <= 0boundary.The util's contract only surfaces
co2ImpactKgwhenco2_impact > 0. Current tests only check "annotation present with positive value" vs. "annotation absent entirely" — a regression that renders the indicator forco2_impact: 0or a negative value wouldn't be caught here.🧪 Suggested additional test
+ it('does not render MessageEnergyIndicator when co2_impact is zero or negative', async () => { + await act(async () => { + renderWithProviders( + <MessageItem + {...defaultProps} + message={{ + ...defaultProps.message, + annotations: [{ co2_impact: 0 }], + }} + />, + ); + }); + + expect( + screen.queryByTestId('message-energy-indicator'), + ).not.toBeInTheDocument(); + });Based on learnings from
getMessageCo2Impact.ts, which looks for an annotation object withco2_impact: numberand returns that value only if it's > 0.🤖 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/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsx` around lines 526 - 552, The energy indicator tests only cover a positive CO2 annotation and no annotation, so add a boundary case in MessageItem.test.tsx for MessageItem/MessageEnergyIndicator where annotations include co2_impact values of 0 and a negative number, and assert the indicator is not rendered. Use the existing renderWithProviders and screen.queryByTestId('message-energy-indicator') pattern so the suite matches the getMessageCo2Impact contract that only surfaces co2ImpactKg when co2_impact > 0.src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx (1)
18-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using the current i18n locale for number formatting.
toLocaleString(undefined, ...)falls back to the browser's default locale, which may not match the app's active i18next language, causing inconsistent number formatting (decimal separators, etc.) versus the rest of the localized UI.🌐 Optional improvement
-const formatCo2Impact = (kgCo2eq: number): string => { - return `${(kgCo2eq * 1000).toLocaleString(undefined, { +const formatCo2Impact = (kgCo2eq: number, locale?: string): string => { + return `${(kgCo2eq * 1000).toLocaleString(locale, { maximumFractionDigits: 2, })} g CO₂eq`; };And pass
i18n.languagefrom the component when callingformatCo2Impact.🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx` around lines 18 - 22, The number formatting in formatCo2Impact currently relies on toLocaleString(undefined, ...) and may not match the app’s active i18next language. Update MessageEnergyIndicator so the formatter uses the current i18n locale instead of the browser default, and pass i18n.language from the component when calling formatCo2Impact to keep numeric separators consistent with the rest of the localized UI.
🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`:
- Around line 62-86: The Prettier failure comes from inconsistent JSX
indentation in MessageEnergyIndicator’s leftActions/rightActions blocks.
Reformat the Button contents so the spacing matches the project style,
especially the Know more text and the OK button nesting. Use the
MessageEnergyIndicator component as the target and run the file through
Prettier-compatible formatting so CI passes.
---
Nitpick comments:
In
`@src/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsx`:
- Around line 526-552: The energy indicator tests only cover a positive CO2
annotation and no annotation, so add a boundary case in MessageItem.test.tsx for
MessageItem/MessageEnergyIndicator where annotations include co2_impact values
of 0 and a negative number, and assert the indicator is not rendered. Use the
existing renderWithProviders and
screen.queryByTestId('message-energy-indicator') pattern so the suite matches
the getMessageCo2Impact contract that only surfaces co2ImpactKg when co2_impact
> 0.
In
`@src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`:
- Around line 18-22: The number formatting in formatCo2Impact currently relies
on toLocaleString(undefined, ...) and may not match the app’s active i18next
language. Update MessageEnergyIndicator so the formatter uses the current i18n
locale instead of the browser default, and pass i18n.language from the component
when calling formatCo2Impact to keep numeric separators consistent with the rest
of the localized UI.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 27b30117-a032-45ed-9aed-cff3ee5ec7eb
⛔ Files ignored due to path filters (2)
src/frontend/apps/conversations/src/assets/icons/uikit-custom/arrow-up-right.svgis excluded by!**/*.svgsrc/frontend/apps/conversations/src/assets/icons/uikit-custom/leaves.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
CHANGELOG.mdsrc/frontend/apps/conversations/src/features/chat/api/useChat.tsxsrc/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageItem.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsxsrc/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts
- src/frontend/apps/conversations/src/features/chat/api/useChat.tsx
- src/frontend/apps/conversations/src/features/chat/components/MessageItem.tsx
- src/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsx
elvoisin
force-pushed
the
evoisin/feature-co2
branch
from
July 7, 2026 14:41
0e0462e to
46bda7b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx (1)
18-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNumber formatting ignores app's active language.
toLocaleString(undefined, ...)falls back to the browser/runtime locale, not the app's i18n language selected viauseTranslation. This can cause the CO2 value's decimal separator/grouping to be inconsistent with the rest of the localized UI.♻️ Proposed fix
-const formatCo2Impact = (kgCo2eq: number): string => { - return `${(kgCo2eq * 1000).toLocaleString(undefined, { - maximumFractionDigits: 2, - })} g CO₂eq`; -}; +const formatCo2Impact = (kgCo2eq: number, locale: string): string => { + return `${(kgCo2eq * 1000).toLocaleString(locale, { + maximumFractionDigits: 2, + })} g CO₂eq`; +};Then pass
i18n.languagefromuseTranslation()at the call site.🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx` around lines 18 - 22, The number formatting in formatCo2Impact is using the runtime default locale, so it can differ from the app’s selected language. Update MessageEnergyIndicator to get i18n from useTranslation and pass i18n.language into the toLocaleString call at the formatCo2Impact call site so the CO2 value matches the active app locale.
🤖 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/frontend/apps/conversations/src/features/chat/components/__tests__/MessageEnergyIndicator.test.tsx`:
- Around line 45-53: The desktop tooltip test in MessageEnergyIndicator is
asserting tooltip text before the Tooltip content is actually mounted. Update
the test to trigger the tooltip via hover or focus on the Carbon impact trigger
returned by getByLabelText('Carbon impact') before calling getByText for the CO₂
content, so the assertion targets the mounted tooltip content reliably.
---
Nitpick comments:
In
`@src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`:
- Around line 18-22: The number formatting in formatCo2Impact is using the
runtime default locale, so it can differ from the app’s selected language.
Update MessageEnergyIndicator to get i18n from useTranslation and pass
i18n.language into the toLocaleString call at the formatCo2Impact call site so
the CO2 value matches the active app locale.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3d1d47d8-6dad-4e0a-b521-4ec2fb6c6301
⛔ Files ignored due to path filters (2)
src/frontend/apps/conversations/src/assets/icons/uikit-custom/arrow-up-right.svgis excluded by!**/*.svgsrc/frontend/apps/conversations/src/assets/icons/uikit-custom/leaves.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
CHANGELOG.mdsrc/frontend/apps/conversations/src/features/chat/api/useChat.tsxsrc/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageItem.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageEnergyIndicator.test.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsxsrc/frontend/apps/conversations/src/features/chat/utils/__tests__/getMessageCo2Impact.test.tssrc/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts
✅ Files skipped from review due to trivial changes (3)
- src/frontend/apps/conversations/src/features/chat/utils/tests/getMessageCo2Impact.test.ts
- src/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/frontend/apps/conversations/src/features/chat/api/useChat.tsx
- src/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsx
- src/frontend/apps/conversations/src/features/chat/components/tests/MessageItem.test.tsx
- src/frontend/apps/conversations/src/features/chat/components/MessageItem.tsx
elvoisin
force-pushed
the
evoisin/feature-co2
branch
from
July 9, 2026 14:22
46bda7b to
bb9b6b5
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (5)
src/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts (1)
13-17: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUnnecessary
.map()allocation before.find().The
.map()creates an intermediate array solely to cast each element toCo2Annotation. The cast can be applied inline in the.find()predicate, avoiding the extra allocation.♻️ Proposed refactor
- const impact = annotations - ?.map((annotation) => annotation as Co2Annotation) - .find( - (annotation) => typeof annotation.co2_impact === 'number', - )?.co2_impact; + const impact = annotations + ?.find( + (annotation) => + typeof (annotation as Co2Annotation).co2_impact === 'number', + ) + ?.co2_impact as number | undefined;🤖 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/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts` around lines 13 - 17, The `getMessageCo2Impact` logic is allocating an unnecessary intermediate array by calling `.map()` before `.find()`. Update the `impact` computation to apply the `Co2Annotation` cast inline within the `.find()` callback instead of mapping first, so the existing `annotations` iterable is searched directly without extra allocation.src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx (2)
61-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist constant definitions outside the component.
IMPACT_CO2_SCRIPT_URLandIMPACT_CO2_TYPEare immutable constants but are defined inside the component body, causing re-allocation on every render. Move them above the component.♻️ Proposed refactor
+const IMPACT_CO2_SCRIPT_URL = 'https://impactco2.fr/iframe.js'; +const IMPACT_CO2_TYPE = 'comparateur/etiquette-animee'; + const formatCo2Impact = (kgCo2eq: number): string => { return `${(kgCo2eq * 1000).toLocaleString(undefined, { maximumFractionDigits: 2, })} g CO₂eq`; }; @@ - const IMPACT_CO2_SCRIPT_URL = 'https://impactco2.fr/iframe.js'; - const IMPACT_CO2_TYPE = 'comparateur/etiquette-animee'; - const widgetContainerRef = useCallback(🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx` around lines 61 - 62, Hoist the immutable IMPACT_CO2_SCRIPT_URL and IMPACT_CO2_TYPE definitions out of MessageEnergyIndicator so they are not recreated on every render. Move these constants to module scope above the component definition, and keep the component logic using the same symbols without changing behavior.
72-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
.datasetoversetAttributefor data attributes.SonarCloud flags this: using the
datasetproperty is the idiomatic DOM API fordata-*attributes and is more concise.♻️ Proposed refactor
const script = document.createElement('script'); script.src = IMPACT_CO2_SCRIPT_URL; - script.setAttribute('data-name', 'impact-co2'); - script.setAttribute('data-type', IMPACT_CO2_TYPE); - script.setAttribute('data-search', dataSearch); + script.dataset.name = 'impact-co2'; + script.dataset.type = IMPACT_CO2_TYPE; + script.dataset.search = dataSearch; container.appendChild(script);🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx` around lines 72 - 74, The MessageEnergyIndicator component is setting data-* attributes with setAttribute; refactor the script setup to use the element’s dataset property instead. Update the DOM manipulation near the impact-co2 script initialization so the data-name, data-type, and data-search values are assigned through dataset on the same script element, preserving the existing values and behavior.Source: Linters/SAST tools
src/frontend/apps/conversations/src/pages/globals.css (2)
367-370: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
!importanton button gap may be avoidable.Using
gap: 7px !importantforces this value globally for all--with-icon--rightbuttons and makes it hard to override per-component. If the base gap comes from a design system token, consider overriding via a higher-specificity selector or adjusting the token instead.🤖 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/frontend/apps/conversations/src/pages/globals.css` around lines 367 - 370, The `.c__button--with-icon--right` rule is forcing the button spacing with `gap: 7px !important`, which makes component-level overrides difficult. Remove the `!important` and adjust the styling through a more specific selector or the relevant design token used by the button styles so `gap` can be overridden normally.
140-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the shared tooltip override
.c__tooltipis already defined in the shared Cunningham stylesheet, so this rule changes padding and hides the React Aria arrow for every tooltip using that class. If this is only meant for the energy indicator, scope it to a feature-specific selector instead of overriding the shared one. The!importantalso makes future overrides harder.🤖 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/frontend/apps/conversations/src/pages/globals.css` around lines 140 - 147, The `.c__tooltip` override in the conversations globals stylesheet is affecting the shared Cunningham tooltip class, including padding and the React Aria arrow for every tooltip using that selector. Update the styles to target a feature-specific wrapper or component scope for the energy indicator instead of the shared `.c__tooltip` class, and remove the `!important` usage so the shared tooltip styling remains overridable.
🤖 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.
Nitpick comments:
In
`@src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`:
- Around line 61-62: Hoist the immutable IMPACT_CO2_SCRIPT_URL and
IMPACT_CO2_TYPE definitions out of MessageEnergyIndicator so they are not
recreated on every render. Move these constants to module scope above the
component definition, and keep the component logic using the same symbols
without changing behavior.
- Around line 72-74: The MessageEnergyIndicator component is setting data-*
attributes with setAttribute; refactor the script setup to use the element’s
dataset property instead. Update the DOM manipulation near the impact-co2 script
initialization so the data-name, data-type, and data-search values are assigned
through dataset on the same script element, preserving the existing values and
behavior.
In
`@src/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts`:
- Around line 13-17: The `getMessageCo2Impact` logic is allocating an
unnecessary intermediate array by calling `.map()` before `.find()`. Update the
`impact` computation to apply the `Co2Annotation` cast inline within the
`.find()` callback instead of mapping first, so the existing `annotations`
iterable is searched directly without extra allocation.
In `@src/frontend/apps/conversations/src/pages/globals.css`:
- Around line 367-370: The `.c__button--with-icon--right` rule is forcing the
button spacing with `gap: 7px !important`, which makes component-level overrides
difficult. Remove the `!important` and adjust the styling through a more
specific selector or the relevant design token used by the button styles so
`gap` can be overridden normally.
- Around line 140-147: The `.c__tooltip` override in the conversations globals
stylesheet is affecting the shared Cunningham tooltip class, including padding
and the React Aria arrow for every tooltip using that selector. Update the
styles to target a feature-specific wrapper or component scope for the energy
indicator instead of the shared `.c__tooltip` class, and remove the `!important`
usage so the shared tooltip styling remains overridable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dd83df19-22a5-4897-b627-c83708afbd24
⛔ Files ignored due to path filters (2)
src/frontend/apps/conversations/src/assets/icons/uikit-custom/arrow-up-right.svgis excluded by!**/*.svgsrc/frontend/apps/conversations/src/assets/icons/uikit-custom/leaves.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
CHANGELOG.mdsrc/frontend/apps/conversations/src/features/chat/api/useChat.tsxsrc/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageItem.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageEnergyIndicator.test.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsxsrc/frontend/apps/conversations/src/features/chat/utils/__tests__/getMessageCo2Impact.test.tssrc/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.tssrc/frontend/apps/conversations/src/pages/globals.css
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (6)
- src/frontend/apps/conversations/src/features/chat/utils/tests/getMessageCo2Impact.test.ts
- src/frontend/apps/conversations/src/features/chat/components/tests/MessageItem.test.tsx
- src/frontend/apps/conversations/src/features/chat/components/tests/MessageEnergyIndicator.test.tsx
- src/frontend/apps/conversations/src/features/chat/api/useChat.tsx
- src/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsx
- src/frontend/apps/conversations/src/features/chat/components/MessageItem.tsx
3 tasks
elvoisin
force-pushed
the
evoisin/feature-co2
branch
3 times, most recently
from
July 10, 2026 08:52
1f330aa to
21b8848
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts (1)
8-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
mapbeforefind.
annotationsis already typed asCo2Annotation[]from the cast on line 6, so mapping each element toCo2Annotationagain is a no-op. Callfinddirectly.♻️ Proposed refactor
const impact = annotations - ?.map((annotation) => annotation as Co2Annotation) - .find( + ?.find( (annotation) => typeof annotation.co2_impact === 'number', )?.co2_impact;🤖 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/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts` around lines 8 - 12, Remove the redundant map cast before find in getMessageCo2Impact, and call find directly on the already typed annotations array while preserving the existing co2_impact number check and result handling.src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx (1)
72-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
datasetinstead ofsetAttributeto clear the SonarCloud failures.SonarCloud flags lines 72–74 as failures. Switching to the
datasetAPI keeps behavior identical and unblocks the check.♻️ Proposed fix
- script.setAttribute('data-name', 'impact-co2'); - script.setAttribute('data-type', IMPACT_CO2_TYPE); - script.setAttribute('data-search', dataSearch); + script.dataset.name = 'impact-co2'; + script.dataset.type = IMPACT_CO2_TYPE; + script.dataset.search = dataSearch;As per static analysis hints (SonarCloud: "Prefer
.datasetoversetAttribute(…)").🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx` around lines 72 - 74, Replace the three setAttribute calls in the script setup with assignments to script.dataset.name, script.dataset.type, and script.dataset.search, preserving the existing values and behavior.Source: Linters/SAST tools
🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`:
- Around line 22-26: Update formatCo2Impact to accept an app locale parameter
and pass that locale to toLocaleString instead of undefined. At the impactLabel
call site, provide the locale derived from i18n.resolvedLanguage, preserving the
existing formatting options and translated widget language.
---
Nitpick comments:
In
`@src/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`:
- Around line 72-74: Replace the three setAttribute calls in the script setup
with assignments to script.dataset.name, script.dataset.type, and
script.dataset.search, preserving the existing values and behavior.
In
`@src/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.ts`:
- Around line 8-12: Remove the redundant map cast before find in
getMessageCo2Impact, and call find directly on the already typed annotations
array while preserving the existing co2_impact number check and result handling.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f8f6c4d6-9fc8-4147-9c52-030358006dd5
⛔ Files ignored due to path filters (2)
src/frontend/apps/conversations/src/assets/icons/uikit-custom/arrow-up-right.svgis excluded by!**/*.svgsrc/frontend/apps/conversations/src/assets/icons/uikit-custom/leaves.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
CHANGELOG.mdsrc/frontend/apps/conversations/src/features/chat/api/useChat.tsxsrc/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsxsrc/frontend/apps/conversations/src/features/chat/components/MessageItem.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageEnergyIndicator.test.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsxsrc/frontend/apps/conversations/src/features/chat/utils/__tests__/getMessageCo2Impact.test.tssrc/frontend/apps/conversations/src/features/chat/utils/__tests__/impactCo2.test.tssrc/frontend/apps/conversations/src/features/chat/utils/getMessageCo2Impact.tssrc/frontend/apps/conversations/src/features/chat/utils/impactCo2.tssrc/frontend/apps/conversations/src/pages/globals.css
✅ Files skipped from review due to trivial changes (4)
- src/frontend/apps/conversations/src/features/chat/utils/tests/impactCo2.test.ts
- CHANGELOG.md
- src/frontend/apps/conversations/src/pages/globals.css
- src/frontend/apps/conversations/src/features/chat/components/FeedbackButtons.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/frontend/apps/conversations/src/features/chat/utils/tests/getMessageCo2Impact.test.ts
- src/frontend/apps/conversations/src/features/chat/api/useChat.tsx
- src/frontend/apps/conversations/src/features/chat/components/tests/MessageItem.test.tsx
- src/frontend/apps/conversations/src/features/chat/components/MessageItem.tsx
Comment on lines
+22
to
+26
| const formatCo2Impact = (kgCo2eq: number): string => { | ||
| return `${(kgCo2eq * 1000).toLocaleString(undefined, { | ||
| maximumFractionDigits: 2, | ||
| })} g CO₂eq`; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Format the CO₂ number using the active app language.
toLocaleString(undefined, …) uses the runtime/browser default locale rather than the user's selected i18n language, so the number in impactLabel (e.g. decimal/grouping separators) can diverge from the surrounding translated text and from the widget language you already derive from i18n.resolvedLanguage. Pass the app locale through.
🌐 Proposed fix
-const formatCo2Impact = (kgCo2eq: number): string => {
- return `${(kgCo2eq * 1000).toLocaleString(undefined, {
- maximumFractionDigits: 2,
- })} g CO₂eq`;
-};
+const formatCo2Impact = (kgCo2eq: number, locale?: string): string => {
+ return `${(kgCo2eq * 1000).toLocaleString(locale, {
+ maximumFractionDigits: 2,
+ })} g CO₂eq`;
+};And at the call site:
- const impactLabel = t('This request: {{co2}}', {
- co2: formatCo2Impact(co2ImpactKg),
- });
+ const impactLabel = t('This request: {{co2}}', {
+ co2: formatCo2Impact(co2ImpactKg, i18n.language),
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const formatCo2Impact = (kgCo2eq: number): string => { | |
| return `${(kgCo2eq * 1000).toLocaleString(undefined, { | |
| maximumFractionDigits: 2, | |
| })} g CO₂eq`; | |
| }; | |
| const formatCo2Impact = (kgCo2eq: number, locale?: string): string => { | |
| return `${(kgCo2eq * 1000).toLocaleString(locale, { | |
| maximumFractionDigits: 2, | |
| })} g CO₂eq`; | |
| }; |
🤖 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/frontend/apps/conversations/src/features/chat/components/MessageEnergyIndicator.tsx`
around lines 22 - 26, Update formatCo2Impact to accept an app locale parameter
and pass that locale to toLocaleString instead of undefined. At the impactLabel
call site, provide the locale derived from i18n.resolvedLanguage, preserving the
existing formatting options and translated widget language.
elvoisin
force-pushed
the
evoisin/feature-co2
branch
3 times, most recently
from
July 10, 2026 09:24
ccec60c to
90cb29e
Compare
add co2 tooltip on assistant messages + modal
elvoisin
force-pushed
the
evoisin/feature-co2
branch
from
July 17, 2026 14:31
90cb29e to
554ff4a
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
co2_impactannotation is available.