Skip to content

chore(mobile): name the two project favicon caches by their job - #13160

Merged
juliusmarminge merged 3 commits into
mainfrom
chore/mobile-name-project-favicon-cache-modules
Sep 23, 2026
Merged

juliusmarminge merged 3 commits into
mainfrom
chore/mobile-name-project-favicon-cache-modules

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

The problem

Mobile shipped two different projectFaviconCache.ts files with the same basename and different jobs: src/components/projectFaviconCache.ts tracked in-flight favicon image requests, while src/lib/projectFaviconCache.ts was the SQLite-backed persistent cache. The mobile singleton was also named projectFaviconCache, the same name as the shared client-runtime factory it calls. Navigation and agents routinely landed on the wrong file (mobile audit item #9 flagged this collision).

The fix

Pure renames plus two module doc comments — no behavior changes:

  • src/components/projectFaviconCache.ts (+ test) → src/lib/projectFaviconRequests.ts, since it is request bookkeeping, not a cache, and has no React in it. Added a module comment saying it pairs with the database cache.
  • src/lib/projectFaviconCache.ts (+ test) → src/lib/projectFaviconDatabaseCache.ts, and its singleton projectFaviconCache → projectFaviconDatabaseCache, so it no longer shadows the createProjectFaviconCache factory name from @t3tools/client-runtime/project-favicon-cache.
  • Updated the four import sites (ProjectFavicon.tsx, state/assets.ts, state/client-cache-state.ts, connection/environment-cache-store.ts). The public function names in the request module are unchanged.

Left alone deliberately: the client-runtime package file (cross-app contract, not flagged in the audit) and every other file in the audit's broader naming findings — one collision per PR.

Verification

  • vp test run src/lib/projectFaviconRequests.test.ts src/lib/projectFaviconDatabaseCache.test.ts — 9 passed (both suites cover the renamed modules directly).
  • tsc --noEmit for apps/mobile — clean.
  • vp lint apps/mobile/src — no new findings on touched files (the one ProjectFavicon.tsx warning is pre-existing on untouched lines).

Not applicable: screenshots — no visual or runtime change; the diff is renames, identifier updates, and comments.

Model: Claude Opus 5 via Codex, working inside T3 Code.


Devin Review

Summary by CodeRabbit

  • Refactor

    • Reorganized mobile project favicon caching and request handling into clearer, dedicated components.
    • Updated environment loading, asset handling, and cache invalidation to use the revised favicon cache structure.
    • Preserved existing favicon storage, image-loading, and caching behavior.
  • Documentation

    • Clarified favicon request tracking and persistence responsibilities.
  • Tests

    • Updated test references and descriptions to reflect the revised favicon cache organization without changing coverage.

The in-flight request registry and the SQLite-backed cache both shipped as
projectFaviconCache.ts, and the mobile singleton reused the shared factory's
name. Rename the registry to lib/projectFaviconRequests, the persistence
adapter to lib/projectFaviconDatabaseCache, and its singleton to
projectFaviconDatabaseCache so each file says what it does.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 22, 2026
@github-actions

github-actions Bot commented Sep 22, 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.5 KiB 13.5 KiB −26 B (−0.2%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +4 B (+0.1%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −30 B (−0.5%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.3 KiB 56.3 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 10 10 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +5 B (+0.0%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB −1 B (−0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB +6 B (+0.1%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 57.0 KiB 0 B (0.0%) 66.4 KiB ✅
Claude Live turn messages 9 9 0 (0.0%) 21 ✅

Baseline: bed0b4c · PR result: bfcf42c · 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: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

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

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 22, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at bfcf42c

Macroscope's review found this PR approvable — This is a self-contained mechanical rename that separates favicon request bookkeeping from the database-backed cache without changing their implementations, storage keys, or call-site behavior. Tests and all mobile references were updated accordingly, with no product-default or static-analysis configuration changes.

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

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: e25f3e62-7b8b-4f35-942b-f6c33b05a5d3

📥 Commits

Reviewing files that changed from the base of the PR and between c325978 and bfcf42c.

📒 Files selected for processing (1)
  • apps/mobile/src/lib/projectFaviconRequests.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/mobile/src/lib/projectFaviconRequests.ts

Limit details: You’ve used all 10 included reviews currently available.


📝 Walkthrough

Walkthrough

The mobile project favicon cache is separated into request and database-cache modules. The database-backed cache is renamed to projectFaviconDatabaseCache, and related environment, asset, client-cache, component, and test references are updated.

Changes

Project favicon cache

Layer / File(s) Summary
Cache module separation
apps/mobile/src/lib/projectFaviconDatabaseCache.ts, apps/mobile/src/lib/projectFaviconRequests.ts, apps/mobile/src/lib/*test.ts
The database cache export is renamed to projectFaviconDatabaseCache. Request documentation and test imports reflect the separate modules.
Environment cache wiring
apps/mobile/src/connection/environment-cache-store.ts
Environment cache loading hydrates the renamed database cache. Environment clearing uses its clearEnvironment method.
Consumer updates
apps/mobile/src/components/ProjectFavicon.tsx, apps/mobile/src/state/assets.ts, apps/mobile/src/state/client-cache-state.ts
Favicon request helpers and database-cache consumers use the updated module names and export. Global and environment-scoped cache clearing use projectFaviconDatabaseCache.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to bfcf4

The rename and documentation updates preserve favicon request and database-cache behavior, so the change is mergeable with normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: naming the two mobile project favicon modules by their distinct responsibilities.
Description check ✅ Passed The description clearly explains the problem, the rename-based fix, the unchanged behavior, affected modules, verification steps, and why UI screenshots are not applicable. It does not use the templat…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 8 files.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

Correct the request-registry comment: it tracks active URLs and loaded keys,
it does not coalesce loads. Merge the database cache comment so the
client_cache/Settings note stays visible next to the export.
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 22, 2026 23:50

Dismissing prior approval to re-evaluate c325978

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All clear

Posted via Macroscope — Effect Service Conventions

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 22, 2026
The registry keeps an entry alive while any view stays mounted, not only
during the network load, and its current-URL bookkeeping is per cache key.
Comment-only.
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 22, 2026 23:55

Dismissing prior approval to re-evaluate bfcf42c

@juliusmarminge
juliusmarminge merged commit 3409651 into main Sep 23, 2026
23 checks passed
@juliusmarminge
juliusmarminge deleted the chore/mobile-name-project-favicon-cache-modules branch September 23, 2026 00:20
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 23, 2026
## What's Changed
* chore(mobile): drop dead nitro-markdown tgz override and @expo/metro-runtime by @juliusmarminge in pingdotgg/t3code#13148
* feat(web): show settings scope as a sentence at the top of the page by @juliusmarminge in pingdotgg/t3code#13139
* refactor(web): move settings scope pickers into breadcrumbs by @Yash-Singh1 in pingdotgg/t3code#13165
* feat(auth): share provider sign-in flows and credential bindings by @juliusmarminge in pingdotgg/t3code#12983
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13161
* chore(mobile): name the two project favicon caches by their job by @juliusmarminge in pingdotgg/t3code#13160
* revert(mobile): git sheets back to Platform.OS ternaries (un-guarded uniwind variants broke both platforms) by @juliusmarminge in pingdotgg/t3code#13169
* docs(mobile): document the two mobile routes that intentionally skip deep links by @juliusmarminge in pingdotgg/t3code#13164
* refactor(mobile): break module cycles with focused extractions by @juliusmarminge in pingdotgg/t3code#13151
* fix(server): generate PR diffs from branch changes by @Yash-Singh1 in pingdotgg/t3code#13170
* fix(web): preserve nested scroll behavior in chat timeline by @Yash-Singh1 in pingdotgg/t3code#13167
* test(web): cover usage model ordering without static markup by @flamboh in pingdotgg/t3code#13104
* fix(desktop): find linuxbrew node for the WSL backend by @CodyRay in pingdotgg/t3code#7827
* chore(models): use GPT-6 Luna for text generation by @extoci in pingdotgg/t3code#13115
* fix(mobile): keep ordinary offline outbox failures out of console.warn by @juliusmarminge in pingdotgg/t3code#13144
* feat(providers): check remote compatibility ranges by @juliusmarminge in pingdotgg/t3code#13130
* chore(lint): keep mobile theme escape-hatch allowlist honest by @juliusmarminge in pingdotgg/t3code#13146
* fix(web): the pull request badge reads at the meta size again by @juliusmarminge in pingdotgg/t3code#13175
* fix(mobile): uniwind platform variants stay guarded on both platforms by @juliusmarminge in pingdotgg/t3code#13172
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13185
* refactor(mobile): remaining className platform ternaries become class variants by @juliusmarminge in pingdotgg/t3code#13188
* fix(web): align provider emails without clipping by @Derpedyea in pingdotgg/t3code#13174
* perf(mobile): recycle the default v2 home list and scope the snooze minute tick by @juliusmarminge in pingdotgg/t3code#13149
* refactor(mobile): retire the legacy grouped thread list by @juliusmarminge in pingdotgg/t3code#13183
* fix(server): background PR checks spend less GitHub quota by @juliusmarminge in pingdotgg/t3code#13189
* fix(server): background PR sync reads summaries in batches by @juliusmarminge in pingdotgg/t3code#13198
* fix(server): GitHub PR lookups stop probing owner-qualified heads by @juliusmarminge in pingdotgg/t3code#13200
* chore(mobile): clear the legacy-list deletion fallout by @juliusmarminge in pingdotgg/t3code#13203

## New Contributors
* @CodyRay made their first contribution in pingdotgg/t3code#7827

**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260922.2123...v0.0.43-nightly.20260923.2135

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

Labels

size:M 30-99 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