Repository navigation
fix(logos): stop one undecodable mark from hiding every other bundled mark - #180
Conversation
… mark The owner has reported bundled provider logos going missing, being "fixed", and going missing again across roughly ten attempts (2026-10-06). Every previous fix was a theory about caching, and every one of them was verified in a process where the bundle resolved cleanly -- which is the one process the bug does not occur in. A green suite therefore never once proved this worked in the app, which is why it kept coming back. The screenshot that settled it: every bundled mark drew as the SF Symbol fallback at the same time, on the sidebar, Glance and the PiP, while the one platform with a custom mark *on disk* rendered correctly. Process-wide, not per-surface, and disk-backed marks never consult the bundle. So the defect was in bundle resolution, not in the artwork and not in a cache. The actual defect is structural: - `imageFromBundle` took the first candidate whose path *existed* and returned nil if that one would not decode. A single unreadable entry near the front of the list therefore masked every good copy behind it, permanently. Existence and decodability are now checked together, per candidate, and the search continues to the next location on a decode failure. - The candidate list is built fresh on every lookup instead of being derived from one pinned `Bundle` handle, so there is no cached answer left to go stale, and a bundle that becomes reachable later is found without any invalidation dance. - `currentBundle` no longer caches a handle it has not confirmed, and `invalidateCache` (the custom-mark import path) clears that handle too. Why clearing caches never fixed it: the file was never missing. It was present and undecodable, so every "clear the cache" attempt reproduced the identical failure at the identical entry. The fix had to be in the resolution order. Observability, which the bug badly needed: every failure path now logs which mark failed, whether it was absent or undecodable, and where it looked. Ten rounds of this were guesswork because there was no way to tell "no artwork for this key" from "artwork is there and unreadable". Tests: ProviderMarkResolutionTests covers the failure, not the success -- an undecodable earlier candidate must not mask a good later one, every mapped mark must resolve without any custom file on disk (so hasCustomMark cannot mask a broken bundle, the "only MiniMax works" shape), a failed lookup must not poison the next one, and 50 invalidate/lookup cycles must not degrade. Plus a PlatformLogoResourceTests case that marks still resolve after invalidation. swift test: 364 tests, 0 failures. Co-Authored-By: MiniMax Code <noreply@minimax.io>
Co-Authored-By: MiniMax Code <noreply@minimax.io>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7ad466ebc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ForEach(Array(targetRows.prefix(max(1, visible)))) { row in | ||
| pipRow(row, detail: detail) | ||
| } |
There was a problem hiding this comment.
When the user shortens the PiP so fewer rows fit than are pinned, this prefix removes every overflow row from the ScrollView content. The newly displayed “more pinned — scroll” instruction therefore cannot reveal those rows; scrolling reaches only the already-visible prefix and the message. Keep all targetRows in the scroll content while constraining only the viewport height.
Useful? React with 👍 / 👎.
| let windows = row.section.windows | ||
| .filter { !$0.window.isSupplementaryVideoQuota && !row.isMasked($0) } |
There was a problem hiding this comment.
For providers reported by multiple readers, this raw filter bypasses the existing glanceDrawableWindows and copy-suppression logic. In the live Grok Bot shape, DashboardService and gbu report the same weekly allowance while gbu can also emit an unreadable placeholder, so opening details renders two indistinguishable weekly readings plus an empty — row. Apply the same drawable-window and same-allowance filtering used by the expanded Glance details before rendering.
Useful? React with 👍 / 👎.
| if detailRow == nil { | ||
| closeButton | ||
| } else { | ||
| backButton |
There was a problem hiding this comment.
At resized widths around 130–150 points, the existing header breakpoint still shows the centered CodeCaps word, but detail mode now places the substantially wider chevron-and-“Back” control in an independent trailing ZStack layer. These layers overlap, obscuring both the title and control. Hide the title earlier in detail mode or reserve trailing width for the back button.
Useful? React with 👍 / 👎.
| guard let image = NSImage(contentsOf: candidate) ?? NSImage(contentsOfFile: candidate.path) else { | ||
| noteFailure("'\(filename)' exists at \(candidate.path) but would not decode; trying the next location") | ||
| continue |
There was a problem hiding this comment.
When a malformed image is recognized by NSImage but cannot actually produce pixels, this non-nil check accepts it, finalize caches it, and the search still stops before reaching a valid later candidate. The new test fixture's own comment notes that NSImage(contentsOf:) can return an object with no pixels, and the existing resource tests use tiffRepresentation to detect that state. Validate that the image rasterizes before returning so a corrupt earlier copy cannot continue hiding the usable bundled mark.
Useful? React with 👍 / 👎.
| .font(.system(size: 9, weight: .bold)) | ||
| .foregroundStyle(.secondary) | ||
| .frame(width: PipMetrics.cadenceWidth, alignment: .leading) | ||
| Text(snapshot.window.label.isEmpty ? "Quota" : snapshot.window.label) |
There was a problem hiding this comment.
For Antigravity rows, this displays the raw exported labels such as Gemini Models · 5-hour and Third-Party Models · Weekly, bypassing AntigravityDisplay.windowLabel, which the other UI surfaces use to present the approved Gemini and Third-Party names. Opening a PiP detail therefore reintroduces backend terminology and produces naming inconsistent with the row header; normalize the label before rendering it.
Useful? React with 👍 / 👎.
| if !full.isEmpty { | ||
| Text(full) | ||
| .font(.system(size: 9).monospacedDigit()) | ||
| .foregroundStyle(.secondary) | ||
| .lineLimit(1) | ||
| } |
There was a problem hiding this comment.
When the PiP is narrowed to a detail level where summary rows intentionally omit countdowns, the detail view still renders the full reset countdown because this condition checks only whether the string is empty and never consults detail.showsCountdown. At the 110-point minimum width, the fixed cadence and percentage plus this countdown leave too little room for the quota label, causing the body to truncate even though the responsive detail value is passed into this function. Gate the countdown on the width detail just as pipMeter does.
Useful? React with 👍 / 👎.
| let decodable = [goodURL, temp.appendingPathComponent("first/claude.svg")] | ||
| .filter { url in | ||
| guard FileManager.default.fileExists(atPath: url.path) else { return false } | ||
| return NSImage(contentsOf: url) != nil |
There was a problem hiding this comment.
This test neither uses the production resolver nor recreates the claimed candidate order: it places the good URL first and then filters every URL independently. It therefore remains green if imageFromBundle regresses to the old behavior of stopping at the first existing undecodable candidate. Invoke an injectable form of the actual candidate-walking code with the bad URL first and the good URL second so the central regression is genuinely covered.
Useful? React with 👍 / 👎.
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| /// missing, and cleared caches to fix a missing file. The file was present | ||
| /// and undecodable, so clearing caches changed nothing, and the search gave | ||
| /// up at the same entry every single time. | ||
| func testAnUndecodableEarlierCandidateDoesNotMaskAGoodLaterOne() throws { |
There was a problem hiding this comment.
Test regression escape: testAnUndecodableEarlierCandidateDoesNotMaskAGoodLaterOne filters a hardcoded URL array through NSImage(contentsOf:) != nil and never invokes the production loader, so the PlatformLogoImage.load → bundledImage → imageFromBundle → candidateURLs path is untested and the test still passes even if imageFromBundle regresses to the "first existing candidate, give up on decode failure" behavior the PR narrative says defeated prior fixes (the temp dir under NSTemporaryDirectory() is invisible to currentBundle(), Bundle.main.resourceURL, and the hardcoded CodeCaps_CodeCaps.bundle). Drive the assertion through the production loader by staging the undecodable file at a URL candidateURLs() will resolve (e.g. via a test seam that prepends it to the candidate list) and assert PlatformLogoImage.load(providerKey: "claude", style: .template) returns the good copy rather than nil.
Prompt for LLM
File Tests/CodeCapsTests/ProviderMarkResolutionTests.swift:
Line 80:
Test regression escape: `testAnUndecodableEarlierCandidateDoesNotMaskAGoodLaterOne` filters a hardcoded URL array through `NSImage(contentsOf:) != nil` and never invokes the production loader, so the `PlatformLogoImage.load` → `bundledImage` → `imageFromBundle` → `candidateURLs` path is untested and the test still passes even if `imageFromBundle` regresses to the "first existing candidate, give up on decode failure" behavior the PR narrative says defeated prior fixes (the temp dir under `NSTemporaryDirectory()` is invisible to `currentBundle()`, `Bundle.main.resourceURL`, and the hardcoded `CodeCaps_CodeCaps.bundle`). Drive the assertion through the production loader by staging the undecodable file at a URL `candidateURLs()` will resolve (e.g. via a test seam that prepends it to the candidate list) and assert `PlatformLogoImage.load(providerKey: "claude", style: .template)` returns the good copy rather than nil.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
The owner reported that no provider logo drew at all except the one custom mark on disk, and that repeated fixes had not moved it. Every bundled mark showed questionmark.square.dashed — the fallback glyph. Four prior attempts (#160, #167, #176, #180) all assumed a bundle-resolution failure. Measurement ruled that out: - All 17 marks are present and decode; every SVG rasterises to real pixels. - Bundle.url(forResource:) finds each one in the installed app. - PlatformLogoImage.load returns a valid image for all 18 provider keys. - Installed build 156 IS main HEAD, and already includes the #180 logo fix. - noteFailure had never logged in 48h, and that silence is meaningful: the app emits ~39k unified-log entries a day. So resolution worked and delivery did not. Two things were wrong with the delivery, both found by measuring rather than assuming: 1. The marks were SVGs, which come back as _NSSVGImageRep — a PRIVATE AppKit class. Three separate fixes had chased the wrong layer. 2. muse-code.png, muse.png and muse-assist.png shipped a baked-in opaque white background, so they drew as a white box on any non-white surface. Every mark now ships as three PNGs — <base>-color, <base>-light, <base>-dark — and appearance is chosen from data instead of the hardcoded monochromeKeys Set. That Set was the guesswork that made some marks invisible and others fine; it no longer decides which file gets loaded. Also fixed while in there: the Third-Party Antigravity pool resolved through templateResourceNames to a Muse mark, which was simply wrong. Both pools now map to their own star. muse-code is rasterised from the Meta vector because the shipped PNG was visibly soft at small sizes. muse-code coverage went from 100% (opaque white box) to 19% once the background was knocked out from the border inward, so white that belongs to the mark is preserved. The custom-mark path is untouched: an owner-supplied mark still wins over the bundled one, and still adapts to Light/Dark. A first pass at this merged .ccustom into the .standard case and broke exactly that path; the 6 failures it caused caught it. Verified: 366 tests pass, 3 skipped, 0 failures, plus 4 new ones asserting all 18 providers resolve in every style and appearance, every delivered mark is an NSBitmapImageRep rather than a private rep, none carries an opaque background, and Light and Dark are genuinely different files.
The owner reported that no provider logo drew at all except the one custom mark on disk, and that repeated fixes had not moved it. Every bundled mark showed questionmark.square.dashed — the fallback glyph. Four prior attempts (#160, #167, #176, #180) all assumed a bundle-resolution failure. Measurement ruled that out: - All 17 marks are present and decode; every SVG rasterises to real pixels. - Bundle.url(forResource:) finds each one in the installed app. - PlatformLogoImage.load returns a valid image for all 18 provider keys. - Installed build 156 IS main HEAD, and already includes the #180 logo fix. - noteFailure had never logged in 48h, and that silence is meaningful: the app emits ~39k unified-log entries a day. So resolution worked and delivery did not. Two things were wrong with the delivery, both found by measuring rather than assuming: 1. The marks were SVGs, which come back as _NSSVGImageRep — a PRIVATE AppKit class. Three separate fixes had chased the wrong layer. 2. muse-code.png, muse.png and muse-assist.png shipped a baked-in opaque white background, so they drew as a white box on any non-white surface. Every mark now ships as three PNGs — <base>-color, <base>-light, <base>-dark — and appearance is chosen from data instead of the hardcoded monochromeKeys Set. That Set was the guesswork that made some marks invisible and others fine; it no longer decides which file gets loaded. Also fixed while in there: the Third-Party Antigravity pool resolved through templateResourceNames to a Muse mark, which was simply wrong. Both pools now map to their own star. muse-code is rasterised from the Meta vector because the shipped PNG was visibly soft at small sizes. muse-code coverage went from 100% (opaque white box) to 19% once the background was knocked out from the border inward, so white that belongs to the mark is preserved. The custom-mark path is untouched: an owner-supplied mark still wins over the bundled one, and still adapts to Light/Dark. A first pass at this merged .ccustom into the .standard case and broke exactly that path; the 6 failures it caused caught it. Verified: 366 tests pass, 3 skipped, 0 failures, plus 4 new ones asserting all 18 providers resolve in every style and appearance, every delivered mark is an NSBitmapImageRep rather than a private rep, none carries an opaque background, and Light and Dark are genuinely different files.
#190) * fix(logos): ship every provider mark as colour/light/dark PNG triplets The owner reported that no provider logo drew at all except the one custom mark on disk, and that repeated fixes had not moved it. Every bundled mark showed questionmark.square.dashed — the fallback glyph. Four prior attempts (#160, #167, #176, #180) all assumed a bundle-resolution failure. Measurement ruled that out: - All 17 marks are present and decode; every SVG rasterises to real pixels. - Bundle.url(forResource:) finds each one in the installed app. - PlatformLogoImage.load returns a valid image for all 18 provider keys. - Installed build 156 IS main HEAD, and already includes the #180 logo fix. - noteFailure had never logged in 48h, and that silence is meaningful: the app emits ~39k unified-log entries a day. So resolution worked and delivery did not. Two things were wrong with the delivery, both found by measuring rather than assuming: 1. The marks were SVGs, which come back as _NSSVGImageRep — a PRIVATE AppKit class. Three separate fixes had chased the wrong layer. 2. muse-code.png, muse.png and muse-assist.png shipped a baked-in opaque white background, so they drew as a white box on any non-white surface. Every mark now ships as three PNGs — <base>-color, <base>-light, <base>-dark — and appearance is chosen from data instead of the hardcoded monochromeKeys Set. That Set was the guesswork that made some marks invisible and others fine; it no longer decides which file gets loaded. Also fixed while in there: the Third-Party Antigravity pool resolved through templateResourceNames to a Muse mark, which was simply wrong. Both pools now map to their own star. muse-code is rasterised from the Meta vector because the shipped PNG was visibly soft at small sizes. muse-code coverage went from 100% (opaque white box) to 19% once the background was knocked out from the border inward, so white that belongs to the mark is preserved. The custom-mark path is untouched: an owner-supplied mark still wins over the bundled one, and still adapts to Light/Dark. A first pass at this merged .ccustom into the .standard case and broke exactly that path; the 6 failures it caused caught it. Verified: 366 tests pass, 3 skipped, 0 failures, plus 4 new ones asserting all 18 providers resolve in every style and appearance, every delivered mark is an NSBitmapImageRep rather than a private rep, none carries an opaque background, and Light and Dark are genuinely different files. * fix(logos): resolve known iconHint keys via variantFile A hint equal to a provider base name like "claude" was passed to imageFromBundle as a filename, which always missed and logged "is mapped for ... but was not found". Route known keys through variantFile so the colour/light/dark PNG is loaded instead. --------- Co-authored-by: Jay Wedgeworth <jaywedgeworth22@users.noreply.github.com>
1 participant
Board 6a3f582a. Part 1 of 2 — the logo fix. The PiP visual changes from the same review round are in a follow-up.
Why this is different from the nine previous attempts
The owner has watched bundled provider logos go missing, get "fixed," and go missing again across roughly ten rounds. Every earlier fix was a theory about caching. Every earlier fix was verified by a passing test suite — in the one process where the bug does not happen.
The screenshot that settled the cause: every bundled mark drew as the SF Symbol fallback at the same time, across the sidebar, Glance and the PiP, while the single platform with a custom mark on disk rendered fine. Process-wide, not per-surface. Disk-backed custom marks never consult the resource bundle, which isolates the fault to bundle resolution.
Verified while investigating:
So the artwork is fine and the files are present. The defect is in the resolution order.
The actual defect
imageFromBundlechose the first candidate whose path existed, then returnednilif that one would not decode:A single unreadable entry near the front of the list therefore masked every good copy behind it, permanently. This is why clearing caches never helped: the file was never missing. It was present and undecodable, so every attempt reproduced the identical failure at the identical entry.
The fix
Bundlehandle, so there is no cached answer to go stale and a bundle that becomes reachable later is found with no invalidation dance.currentBundleno longer caches a handle it has not confirmed can serve an image;invalidateCache(the custom-mark import path) clears it too.Tests (the failure, not the success)
ProviderMarkResolutionTests:hasCustomMarkcannot mask a broken bundlePlus a
PlatformLogoResourceTestscase that marks still resolve after invalidation.swift buildclean;swift test364 tests, 0 failures.Honest limits
Native Mac UI is verified through code review and CI per the repo convention, and this bug is precisely one that a test suite in a clean process cannot fully demonstrate. If marks still go missing after this lands, the new
NSLoglines will name the exact file, path and reason — that log is the thing the last ten fixes lacked, and it is what makes the next attempt evidence-based instead of theoretical.Co-Authored-By: MiniMax Code noreply@minimax.io