Skip to content

Fix project favicon precedence - #93

Merged
Jay1 merged 4 commits into
mainfrom
jcode/ecc86491
Jun 11, 2026
Merged

Fix project favicon precedence#93
Jay1 merged 4 commits into
mainfrom
jcode/ecc86491

Conversation

@Jay1

@Jay1 Jay1 commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • prefer bounded project favicons over language metadata in the sidebar, with metadata fallback on probe or visible image failure
  • refresh missing or automatically detected project icon metadata on startup with a cap, while preserving manual metadata
  • update icon metadata docs for favicon priority and capped automatic refresh

Verification

  • snip bun run --cwd apps/web test src/components/ProjectSidebarIcon.test.tsx
  • snip safe-run --profile browser -- bun run --cwd apps/web test:browser src/components/ProjectSidebarIcon.browser.tsx
  • snip bun run --cwd apps/server test src/orchestration/Layers/OrchestrationEngine.test.ts
  • snip safe-run --profile build -- bun run --cwd apps/web typecheck
  • snip safe-run --profile build -- bun run --cwd apps/server typecheck
  • snip bunx oxfmt@0.52.0 --check apps/web/src/components/ProjectSidebarIcon.tsx apps/web/src/components/ProjectSidebarIcon.test.tsx apps/web/src/components/ProjectSidebarIcon.browser.tsx apps/server/src/orchestration/Layers/OrchestrationEngine.ts apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts docs/adr/0004-project-language-icons.md docs/architecture/jcode-linguist.md
  • GIT_MASTER=1 snip git diff --check
  • snip safe-run --profile build -- bun run --cwd apps/web build
  • snip safe-run --profile build -- bun run --cwd apps/server build
  • visual QA oracle passes: PASS/PASS

Summary by CodeRabbit

  • New Features

    • Startup refresh for stale or missing project icon metadata with a capped, bounded backfill; preserves manually-set icons.
    • Sidebar favicon probing with in-memory cache and fallback to persisted icon metadata; detects and reacts to favicon load/error.
  • Bug Fixes

    • More reliable persistence and refresh behavior across restarts; avoids unnecessary overwrites.
  • Tests

    • Expanded tests covering persistence, refresh cycles, capped backfill, and favicon preference/failure scenarios.
  • Documentation

    • Updated ADRs and architecture docs to reflect ownership, bounds, and renderer fallback behavior.

@github-actions github-actions Bot added size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 06d1ea12-7a19-415f-8ab3-4cbae4d1cff0

📥 Commits

Reviewing files that changed from the base of the PR and between b4a346e and df9caa7.

📒 Files selected for processing (1)
  • apps/server/src/orchestration/Layers/OrchestrationEngine.ts

📝 Walkthrough

Walkthrough

This PR implements provenance-aware icon metadata refresh on the server and client-side favicon probing with metadata fallback for project sidebar icons. The server tracks which projects have automatically-detected vs. manually-set icon metadata, uses this to skip unnecessary refresh on startup, and applies a configurable backfill limit. The client probes project favicons with caching and falls back to language metadata when unavailable.

Changes

Icon Metadata Provenance & Favicon Fallback

Layer / File(s) Summary
Server provenance infrastructure and helpers
apps/server/src/orchestration/Layers/OrchestrationEngine.ts, apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts
Introduces constants for startup backfill limits, helper functions to identify automatic icon commands and compare icon metadata for equality, a maintained Set of project ids with automatic icon provenance, and a recordProjectIconMetadataProvenance function that tracks automatic vs. manual metadata based on events. Test helpers add polling (waitForProjectIconMetadataValue) and reusable runtime factories (makePersistentOrchestrationRuntime) for persistence-focused scenarios.
Server icon metadata scheduling and startup backfill
apps/server/src/orchestration/Layers/OrchestrationEngine.ts
Updates scheduleProjectIconMetadataDetection to skip refresh for existing manual metadata, avoid updates when metadata matches, and use provenance-aware command IDs for refresh. Startup backfill now selects projects with missing or stale automatic metadata, applies a strict limit, sets the command prefix based on refresh type, and records provenance on committed/bootstrapped events.
Server persistence and startup scenarios
apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts
New test cases verify that stale automatic metadata is refreshed on startup with expected events, refresh occurs correctly across multiple restarts, manual metadata is preserved without resolver invocation, and startup backfill respects the configured limit while leaving uncapped projects unset.
Client favicon probing and caching
apps/web/src/components/ProjectSidebarIcon.tsx
Introduces FaviconProbeState union type, new ProjectPreferredFaviconIcon component that probes favicon existence with Image() load/error and caches results in projectFaviconPresence, and fallback ProjectLanguageIcon for standard glyph rendering. Updates ProjectFolderIcon to check the cache before probing and to invoke onFaviconError callback when the visible favicon fails.
Client icon rendering and test updates
apps/web/src/components/ProjectSidebarIcon.tsx, apps/web/src/components/ProjectSidebarIcon.test.tsx, apps/web/src/components/ProjectSidebarIcon.browser.tsx
ProjectSidebarIcon now delegates to ProjectPreferredFaviconIcon to probe and cache favicon presence. Browser test helper enhanced to simulate configurable load/error outcomes. Unit test assertions updated to check for favicon-preferred attribute. Browser test cases expanded to verify favicon preference when probing succeeds, fallback to icon metadata when probing fails, rechecking favicon on cwd change, fallback when favicon later errors, and Vue/wrapper layout scenarios with request counting.
Documentation updates
docs/adr/0004-project-language-icons.md, docs/architecture/jcode-linguist.md
ADR clarifies icon metadata as server-owned durable state with capped low-priority startup refresh for legacy/stale automatic metadata that must not overwrite manual metadata. Specifies automatic-detection provenance is required to intentionally revisit stale results. Architecture doc updates runtime boundaries: web UI must not scan repositories and may request bounded server favicon endpoint before falling back to persisted metadata.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Jay1/jcode#39: Orchestration changes for icon metadata provenance/backfill build directly on PR #39's initial project language icon orchestration setup.
  • Jay1/jcode#60: Both PRs target the icon metadata detection/refresh pipeline; this PR changes OrchestrationEngine's provenance/backfill behavior while PR #60 updates how iconMetadata is inferred by the resolver.
  • Jay1/jcode#58: Related frontend changes to ProjectSidebarIcon/favicon handling and tests that overlap with this PR's favicon probing/caching updates.

Suggested labels

size:XL

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. 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 'Fix project favicon precedence' clearly and concisely summarizes the main change—prioritizing project favicons in the sidebar component.
Description check ✅ Passed The PR description covers all required sections: Summary describes the three main changes, Verification lists comprehensive checks (tests, typecheck, formatting, build), and UI Changes/Reviewer Notes are addressed appropriately.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@apps/server/src/orchestration/Layers/OrchestrationEngine.ts`:
- Around line 676-677: During the reconcileReadModelAfterDispatchFailure
replay/publish path, call recordProjectIconMetadataProvenance with the event
before publishing so recovered automatic project.meta-updated events get the
correct provenance; specifically, add a
recordProjectIconMetadataProvenance(event) invocation in the
reconcileReadModelAfterDispatchFailure flow (the same place where
PubSub.publish/eventPubSub is used) so it mirrors the normal commit and
bootstrap replay behavior already present near
recordProjectIconMetadataProvenance and the bootstrap replay path.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 030bf1ee-d39c-4df7-83c7-a996cf0da962

📥 Commits

Reviewing files that changed from the base of the PR and between ab0c38c and b4a346e.

📒 Files selected for processing (7)
  • apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts
  • apps/server/src/orchestration/Layers/OrchestrationEngine.ts
  • apps/web/src/components/ProjectSidebarIcon.browser.tsx
  • apps/web/src/components/ProjectSidebarIcon.test.tsx
  • apps/web/src/components/ProjectSidebarIcon.tsx
  • docs/adr/0004-project-language-icons.md
  • docs/architecture/jcode-linguist.md

Comment thread apps/server/src/orchestration/Layers/OrchestrationEngine.ts
@Jay1
Jay1 merged commit b0d8490 into main Jun 11, 2026
8 checks passed
@Jay1
Jay1 deleted the jcode/ecc86491 branch June 11, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 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