Skip to content

refactor(web): pass the project record to ProjectFavicon so icons cannot drift - #10714

Merged
t3dotgg merged 1 commit into
mainfrom
refactor/project-favicon-object
Sep 8, 2026
Merged

refactor(web): pass the project record to ProjectFavicon so icons cannot drift#10714
t3dotgg merged 1 commit into
mainfrom
refactor/project-favicon-object

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Sep 8, 2026

Copy link
Copy Markdown
Member

Follow-up to #10712. `ProjectFavicon` took five loose props, and any call site could hand it a display label as the title. The automatic icon is derived from the title, so one wrong string changed the icon on that surface only. That is exactly how the command palette drifted from the sidebar.

`ProjectFavicon` now takes one `project` prop typed as a Pick of the project record (environment, workspace root, title, favicon path, icon override). Every call site passes the record itself, so the saved title and icon override always travel together. Sidebar rows, drafts, the chat header, the command palette subtitle, the PR filter menu, and the archive panel drop their per-field plumbing and pass the project through. Sidebar goes from four per-field lookup maps to one.

Behavior is unchanged where a project exists. Rows with no matching project record used to render a generic icon from an empty title. They now render no icon.

Verified with the web typecheck, lint on the touched files, and the favicon, command palette, chat header, and PR filter test files.

Created with Claude Fable 5.1 in Claude Code.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Project icons and metadata are now handled consistently using complete project details across chat, sidebars, command menus, pull request filters, and settings.
    • Project breadcrumbs, archived threads, and filter options continue to display the correct project names, paths, and icons.
  • Tests
    • Updated project icon coverage to reflect the unified project data handling.

ProjectFavicon took five loose props (environment, cwd, title, favicon
path, icon override). Any call site could pass a display label as the
title and change the automatic icon, which is how the command palette
drifted from the sidebar.

ProjectFavicon now takes one project prop typed as a Pick of the
project record. Every call site passes the record itself. Sidebar rows,
drafts, the chat header, the command palette subtitle, the PR filter
menu, and the archive panel drop their per-field plumbing and pass the
project through instead.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.6 KiB −18 B (−0.1%) 15.1 KiB
Codex Thread snapshot wire 7.0 KiB 7.0 KiB −2 B (−0.0%) 7.3 KiB
Codex Live turn WebSocket wire 6.6 KiB 6.6 KiB −16 B (−0.2%) 7.8 KiB
Codex Live turn WebSocket decoded 57.1 KiB 57.1 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 10 10 0 (0.0%) 21
Claude Total thread wire 13.6 KiB 13.6 KiB +12 B (+0.1%) 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.0 KiB +2 B (+0.0%) 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 6.6 KiB +10 B (+0.1%) 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 57.9 KiB +44 B (+0.1%) 66.4 KiB
Claude Live turn messages 9 10 +1 (+11.1%) 21

Baseline: d7a59c6 · PR result: e05ab14 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at e05ab14

Macroscope's review found this PR approvable — This is a focused web refactor and low-risk UI consistency fix: ProjectFavicon now receives the canonical project data so saved titles, favicon paths, and icon overrides cannot drift between surfaces. Existing project behavior is preserved aside from correcting mismatched automatic icons, with focused tests covering the component behavior.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8de29b69-393a-45dd-995c-082a592acc8b

📥 Commits

Reviewing files that changed from the base of the PR and between d7a59c6 and e05ab14.

📒 Files selected for processing (12)
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/components/CommandPalette.tsx
  • apps/web/src/components/LegacySidebar.tsx
  • apps/web/src/components/ProjectFavicon.test.tsx
  • apps/web/src/components/ProjectFavicon.tsx
  • apps/web/src/components/Sidebar.tsx
  • apps/web/src/components/ThreadCommandSubtitle.tsx
  • apps/web/src/components/chat/ChatHeader.tsx
  • apps/web/src/components/pullRequest/PullRequestListFilters.tsx
  • apps/web/src/components/settings/ProjectSettingsPanel.tsx
  • apps/web/src/components/settings/ProjectsSettings.tsx
  • apps/web/src/components/settings/SettingsPanels.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

ProjectFavicon now accepts a consolidated project object. Chat, sidebar, command palette, pull request filters, and settings components pass full project records instead of separate metadata fields. Tests use the new typed contract.

Changes

Project metadata consolidation

Layer / File(s) Summary
Consolidated favicon contract
apps/web/src/components/ProjectFavicon.tsx, apps/web/src/components/ThreadCommandSubtitle.tsx, apps/web/src/components/chat/ChatHeader.tsx, apps/web/src/components/ChatView.tsx, apps/web/src/components/ProjectFavicon.test.tsx
ProjectFaviconProject groups project metadata. ProjectFavicon, ThreadCommandSubtitle, and ChatHeader use project objects. Tests update all component calls and assertions.
Sidebar and command-palette wiring
apps/web/src/components/Sidebar.tsx, apps/web/src/components/CommandPalette.tsx, apps/web/src/components/LegacySidebar.tsx
Project lookup maps now store full project records. Sidebar rows, tooltips, search results, command-palette items, and workspace resolution use those records.
Filters and settings consumers
apps/web/src/components/pullRequest/PullRequestListFilters.tsx, apps/web/src/components/settings/ProjectSettingsPanel.tsx, apps/web/src/components/settings/ProjectsSettings.tsx, apps/web/src/components/settings/SettingsPanels.tsx
Filter options and settings views pass project objects to ProjectFavicon. Archived project records retain the full project shape.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to e05ab

Project favicon rendering now receives cohesive project records across the application, preserving existing behavior for known projects while omitting icons when no project is available. No current merge-blocking risk remains.

Suggested reviewers: saphid, stienswout, juliusmarminge

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 12 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main refactor: passing the project record to ProjectFavicon to prevent icon drift.
Description check ✅ Passed The description explains what changed, why it changed, affected call sites, behavior differences, and verification steps. It does not include the template headings or checklist, but the required subst…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/project-favicon-object

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

@t3dotgg
t3dotgg merged commit eb11506 into main Sep 8, 2026
22 checks passed
@t3dotgg
t3dotgg deleted the refactor/project-favicon-object branch September 8, 2026 10:29
alexito4 added a commit to alexito4/t3code that referenced this pull request Sep 8, 2026
Upstream's pingdotgg#10714 changed ProjectFavicon to take a project record instead
of flattened props, to stop icons drifting from the saved title/favicon.
This page's own ProjectFavicon call wasn't a textual conflict during the
merge (upstream never touched this feature-only file), so it silently
broke instead of failing the build until now.
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 8, 2026
## What's Changed
* feat(chat): attach files to question answers by @shivamhwp in pingdotgg/t3code#9871
* feat(desktop): refresh macOS installer with aurora artwork by @saphid in pingdotgg/t3code#10632
* fix(server): give completed turns a full session idle window by @StiensWout in pingdotgg/t3code#10689
* feat(web): add pull request merge defaults by @Bil0000 in pingdotgg/t3code#8088
* fix(usage): keep account columns aligned across limit rows by @juliusmarminge in pingdotgg/t3code#10690
* fix(web): chat text no longer shows through a 1px gap under composer banners by @vitalyiegorov in pingdotgg/t3code#10635
* refactor(server): classify runtime exports by @juliusmarminge in pingdotgg/t3code#10274
* refactor(server): classify orchestration exports by @juliusmarminge in pingdotgg/t3code#10275
* refactor(server): classify service exports by @juliusmarminge in pingdotgg/t3code#10276
* refactor(server): classify telemetry exports by @juliusmarminge in pingdotgg/t3code#10277
* refactor(server): classify provider exports by @juliusmarminge in pingdotgg/t3code#10278
* refactor(server): classify source control exports by @juliusmarminge in pingdotgg/t3code#10279
* refactor(server): classify source control registry API by @juliusmarminge in pingdotgg/t3code#10280
* refactor(server): classify preview toolkit exports by @juliusmarminge in pingdotgg/t3code#10281
* ci(knip): enforce server exports by @juliusmarminge in pingdotgg/t3code#10282
* feat(web): add previous/next turn navigation in minimap by @UtkarshUsername in pingdotgg/t3code#8531
* fix(web): stop the settings sidebar shifting when switching pages by @t3dotgg in pingdotgg/t3code#10705
* fix(web): copy terminal selection with Ctrl+Insert by @iamshadmantaqi in pingdotgg/t3code#8541
* fix(web): show the same project icon in the command palette as everywhere else by @t3dotgg in pingdotgg/t3code#10712
* fix(web): stop sidebar rows flashing and shifting on click by @t3dotgg in pingdotgg/t3code#10713
* refactor(web): pass the project record to ProjectFavicon so icons cannot drift by @t3dotgg in pingdotgg/t3code#10714

## New Contributors
* @iamshadmantaqi made their first contribution in pingdotgg/t3code#8541

**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260908.1387...v0.0.41-nightly.20260908.1400

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260908.1400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant