Skip to content

fix(desktop): skip approval small-print when reason repeats the badge label - #412

Merged
itsskofficial merged 4 commits into
existence-master:mainfrom
TanmayNileshShinde:fix/approval-card-duplicate-badge-label
Oct 17, 2026
Merged

itsskofficial merged 4 commits into
existence-master:mainfrom
TanmayNileshShinde:fix/approval-card-duplicate-badge-label

Conversation

@TanmayNileshShinde

@TanmayNileshShinde TanmayNileshShinde commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Skips the small-print reason line in ApprovalCard.tsx when approval.reason matches the badge label (risk.label) after trimming and case-insensitive comparison, avoiding duplicate lines such as [Signs up] badge followed by "Signs up" in the card body.

Closes #409

Changes

  • Adds pure helper approvalCardReason in desktop/src/features/chat/approvalRisk.ts that returns null when reason matches badgeLabel (trimmed, case-insensitive).
  • Uses approvalCardReason in desktop/src/features/chat/ApprovalCard.tsx so the card falls back to its own line (meta.done using ...) or to nothing when the reason repeats the badge label.
  • Adds unit tests in desktop/tests/approvalRisk.test.ts verifying identical and case/whitespace variations are skipped while distinct reasons are preserved.

Verification

  • npm test in desktop/: all 69 unit tests passing.
  • npm run typecheck in desktop/: passes with 0 errors.
  • npm run build in desktop/: builds cleanly with 0 errors.

Summary by CodeRabbit

  • Bug Fixes
    • Approval cards now show a distinct approval reason when available, while avoiding repeated text when the reason matches the risk label.
    • When no distinct reason is available, the card falls back to the tool’s completion description and name. Command and purpose text continue to take precedence.

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@github-actions github-actions Bot added the area: desktop Electron app and UI label Oct 17, 2026
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b6b0cb78-6176-4fbd-a6e2-209a806524dc
📥 Commits

Reviewing files that changed from the base of the PR and between 24dfffb and ef10360.

📒 Files selected for processing: 3
  • desktop/src/features/chat/ApprovalCard.tsx
  • desktop/src/features/chat/approvalRisk.ts
  • desktop/tests/approvalRisk.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The approval card now filters its reason before displaying small print. It omits empty reasons and reasons that match the badge label after trimming and case normalization. Other reasons remain, followed by the existing tool metadata fallback.

Changes: Approval reason display

Layer / File(s) Summary
Filter and render approval reason
desktop/src/features/chat/approvalRisk.ts, desktop/src/features/chat/ApprovalCard.tsx, desktop/tests/approvalRisk.test.ts
approvalCardReason returns null for empty reasons or reasons matching the badge label after trimming and case normalization. The card uses the result after command and purpose text, then falls back to the tool completion description and name. Tests cover matching, distinct, and empty reasons.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to ef103

The approval card no longer repeats a reason that only restates its badge label, and distinct reasons still show. No merge-blocking risk was found.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title accurately and specifically describes the main change: suppressing approval small-print when the reason repeats the badge label.
Linked Issues check Passed Issue #409 requires suppressing the small-print reason when it matches the badge label after trimming and case-insensitive comparison, with fallback to the card completion line or no line, plus an aut…
Out of Scope Changes check Passed The pull request changes only the approval-card reason selection, its focused helper, and unit tests for issue #409. The refactoring preserves command and purpose precedence and supports the requested…
Full details: Linked Issues check

Explanation

Issue #409 requires suppressing the small-print reason when it matches the badge label after trimming and case-insensitive comparison, with fallback to the card completion line or no line, plus an automated test. approvalCardReason implements this behavior. ApprovalCard.tsx uses it and renders the fallback only when available. desktop/tests/approvalRisk.test.ts covers matching, normalization, and distinct reasons. The screenshot criterion is a manual check and is not a coding requirement.

Full details: Out of Scope Changes check

Explanation

The pull request changes only the approval-card reason selection, its focused helper, and unit tests for issue #409. The refactoring preserves command and purpose precedence and supports the requested fallback behavior. No unrelated product behavior or unrelated files are changed.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
⚔️ Resolve merge conflicts: 💡
  • Resolve merge conflict in branch fix/approval-card-duplicate-badge-label
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the badge with care
It trims the reason shown below
If both repeat, one line stays
Distinct words still have room to show
The card keeps its fallback close
Then hops away through fields of green

Comment @coderabbitai help to get the list of available commands.

@TanmayNileshShinde

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@TanmayNileshShinde

Copy link
Copy Markdown
Contributor Author

recheck

github-actions Bot added a commit that referenced this pull request Oct 17, 2026
@itsskofficial itsskofficial reopened this Oct 17, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 17, 2026
@itsskofficial

Copy link
Copy Markdown
Contributor

Thanks @TanmayNileshShinde, and welcome! Nice quick pick-up of #409. It now conflicts with main because #367 changed the same approval card files after you opened this, and GitHub won't run CI on a conflicting PR. I'm merging main into your branch for you, then testing it in the real app. I'll follow up with a review.

@itsskofficial

Copy link
Copy Markdown
Contributor

Thanks @TanmayNileshShinde, nice clean fix! #367 and #404 changed the same files after you opened this, so I merged main into your branch. I also:

  • moved your check onto the new resolveWording label;
  • added one test that runs a Subscribe and a Delete account click through it;
  • added changes/409.fixed.md, which CI now requires.

I tested it in the built app with a real engine: the free Subscribe card shows "Signs up" once, and the Delete account card shows "Deletes" once.

The PR got locked by our CLA bot when I closed and reopened it to start CI. Sorry about that; it's unlocked now and #434 stops that from happening again. Merging once the CLA check re-runs. Thanks for the contribution!

@itsskofficial
itsskofficial added this pull request to the merge queue Oct 17, 2026
Merged via the queue into existence-master:main with commit bd412fa Oct 17, 2026
9 of 10 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: desktop Electron app and UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Approval card small print repeats the badge ("Signs up" twice)

2 participants