Repository navigation
Grok gRPC fallback, xAI dedupe, iOS canonical cards, dedupe - #306
jaywedgeworth22 wants to merge 2 commits into
Conversation
…use dedupe - Grok: x-xai-token-auth header, subscription_tier_display plan enrichment, and a best-effort grok.com gRPC-web credits fallback (documented wire contract, protobuf scanner with bounds) used only when the proxy publishes no percentage. - xAI/Grok: the same subscription pool reported under both keys keeps only the fresher window. - Copilot windows carry the monthly cadence (1m caption). - iOS: source-scope toggle, source badges/banners, per-window Updated stamps gone from the main list; canonical resolution only, fixed meter columns; source detail stays in Settings. - Muse Assist artwork deduped onto the Muse base (byte-identical).
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:
|
|
Task description is insufficient for business logic validation. 🤔 Insufficient Task ContextI found a task linked to this PR, but it only contains minimal information (title only, no description or acceptance criteria). To perform a meaningful business rules validation, I need more details. 🔍 What I need to validate:
💡 How to improve the task context:
|
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| // Muse Assist ships byte-identical artwork to Muse; one base serves both. | ||
| "muse-assist": "muse", |
There was a problem hiding this comment.
muse-assist artwork removed but bundle test still requires it
The bundle test still expects removed muse-assist artwork, so testTheResourceBundleCarriesArtwork fails. Remove muse-assist from the marks list in Tests/CodeCapsTests/PlatformLogoResourceTests.swift; PlatformLogo can continue mapping the key to muse.
Prompt for LLM
File Sources/CodeCaps/PlatformLogo.swift, lines 290-291:
muse-assist artwork removed but bundle test still requires it
This PR remaps the "muse-assist" logo key to "muse" and drops the muse-assist targets from `scripts/regenerate-provider-marks.py`, but no muse-assist artwork is left in `Sources/CodeCaps/Resources/ProviderMarks` (only muse.png / muse-color / muse-dark / muse-light), while `Tests/CodeCapsTests/PlatformLogoResourceTests.swift:61` still lists "muse-assist" in the marks that must exist in the resource bundle. `testTheResourceBundleCarriesArtwork` fails — `bundle.url(forResource: "muse-assist", withExtension:)` is nil for both "svg" and "png", so the assertion "no artwork for 'muse-assist' in the resource bundle" trips and the test suite goes red. Remove "muse-assist" from the `marks` list in that test (the loader still resolves the key through the new map to the shared muse artwork), or keep shipping a muse-assist file.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| case .bytes: | ||
| if let period = timestampPair(field.bytes) { | ||
| periodStart = period.start | ||
| periodEnd = period.end | ||
| } |
There was a problem hiding this comment.
Any length-delimited field is decoded as the current period, last match wins
Any unknown bytes field that resembles a period can overwrite the bounds and make a spent pool appear 100% remaining. Match timestampPair(field.bytes) only to Self.currentPeriodField, retain the first match, and require a recognized weekly or monthly duration before accepting it.
Prompt for LLM
File Sources/QuotaCore/GrokGrpcCredits.swift, lines 63-67:
Any length-delimited field is decoded as the current period, last match wins
The parser treats every top-level length-delimited field that decodes into exactly two Timestamp submessages as "the current period", and the last such match overwrites earlier ones, so any unknown field can invent period bounds (and, with them, an implicit 0% usage). The documented contract is that only schema-declared messages are recursively decoded and that unknown length-delimited fields stay opaque, so an unrelated period-like field (previous/next period, trial window) can supply a wrong resetAt/periodStart or make the implicit-zero branch report 0% used → "100% remaining" for a pool that is actually spent. Match the period against its documented field number instead of "any bytes field", stop at the first match instead of overwriting, and require the duration to be a recognized weekly/monthly span before accepting it.
Suggested code:
case .bytes:
guard field.number == Self.currentPeriodField else { continue }
if let period = timestampPair(field.bytes), periodStart == nil {
periodStart = period.start
periodEnd = period.end
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| case .fixed32: | ||
| // The only top-level floats in the config are usage | ||
| // percentages; product shares nest inside field 7 entries. | ||
| if percent == nil, let value = field.floatValue, | ||
| value.isFinite, (0...100).contains(value) { | ||
| percent = value | ||
| } |
There was a problem hiding this comment.
Out-of-range percentage on the wire is read as zero usage
An invalid published percentage is treated as absent, so the implicit-zero path can show a fully spent pool as untouched. Set sawPercentField when field.floatValue is present, keep percent only for valid values, and disable the implicit-zero fallback when a percentage was seen.
Prompt for LLM
File Sources/QuotaCore/GrokGrpcCredits.swift, lines 56-62:
Out-of-range percentage on the wire is read as zero usage
A percentage that is present on the wire but out of the 0...100 range (or non-finite) is dropped, leaving `percent` nil, which then lets the implicit-zero branch report 0% used. The documented rule for the inferred zero is "no percentage fields anywhere"; here a published percentage of e.g. 150 or NaN is silently converted into a reading of 100% remaining, so a fully exhausted pool is shown as untouched. Track the presence of a percentage field separately from its accepted value and refuse the implicit-zero path whenever any percentage field was seen.
Suggested code:
if let value = field.floatValue {
sawPercentField = true
if percent == nil, value.isFinite, (0...100).contains(value) {
percent = value
}
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| guard let (data, response) = try? await fetchJSON(request), | ||
| (200..<300).contains(response.statusCode) else { return nil } | ||
| return GrokGrpcCredits.parse(data, now: now()) |
There was a problem hiding this comment.
Grok gRPC authentication failures are silently swallowed
HTTP 401/403 and gRPC UNAUTHENTICATED failures are silently treated as missing data instead of rejected credentials. Introduce a typed authentication result in grokGrpcReading(token:) and GrokGrpcCredits.parse(data, now:), propagate it through the caller, and report GROK_TOKEN with only a masked first-8/last-4 preview.
Kody rule violation: Never mint provider API keys or expose secret values — secret hygiene is binding for every agent
Prompt for LLM
File Sources/QuotaCore/LocalQuotaReader.swift, lines 401-403:
Grok gRPC authentication failures are silently swallowed
The catch-all around the authenticated gRPC request and its optional parser result collapses HTTP 401/403 or gRPC status 16 (UNAUTHENTICATED) into a silent fallback miss. Rule 24 requires a rejected provider credential to stop and be reported using only a masked first-8/last-4 preview. Introduce a typed authentication-rejection result, propagate an error naming `GROK_TOKEN` with the masked preview, and reserve `nil` for non-authentication failures. This requires coordinated helper, caller, and parser-result changes rather than a local-line replacement.
Kody rule violation: [Never mint provider API keys or expose secret values — secret hygiene is binding for every agent](https://app.kodus.io/settings/code-review/global/kody-rules/3f722a39-4c80-41d0-b1fb-0151688e9f32?teamId=69360e3a-6b41-45d2-9e49-fde78fe49457)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if let tier = await grokSettingsTier(token: token) { | ||
| windows = windows.map { window in | ||
| var window = window | ||
| if window.planName == nil { window.planName = tier } | ||
| return window | ||
| } | ||
| } | ||
| // Best-effort grok.com gRPC fallback: only when the proxy answer | ||
| // published no percentage. Its percent fills the subscription | ||
| // window; the proxy's period bounds stay authoritative. | ||
| if windows.first?.remainingPercent == nil, | ||
| let grpc = await grokGrpcReading(token: token), | ||
| let index = windows.firstIndex(where: { $0.id.hasSuffix(":subscription") || $0.remainingPercent == nil }) { |
There was a problem hiding this comment.
Grok read adds two serialized HTTP calls that fire even when nothing to enrich
Each Grok refresh can add about eight seconds of serial latency even when neither optional request can enrich the result. In readGrok, request grokSettingsTier(token:) only when a window lacks planName, skip grokGrpcReading(token:) when windows is empty, and run eligible enrichments concurrently.
Prompt for LLM
File Sources/QuotaCore/LocalQuotaReader.swift, lines 352-364:
Grok read adds two serialized HTTP calls that fire even when nothing to enrich
`readGrok` now awaits up to two extra HTTP requests serially after the billing call — `/v1/settings` (2s budget) always, and the grok.com gRPC fallback (6s budget) whenever `windows.first?.remainingPercent == nil`, which is also true when `windows` is empty and there is nothing to fill. A single Grok refresh can block for ~8 extra seconds on top of the billing call, and both requests are wasted whenever the credits payload already carries a plan name or no window at all. Only issue the settings request when a window actually lacks a plan name, skip the gRPC request when `windows` is empty, and run the two optional enrichments concurrently instead of sequentially.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var window = windows[index] | ||
| window.remainingPercent = 100 - grpc.usedPercent | ||
| window.remainingUnknown = false | ||
| if window.resetAt == nil, let end = grpc.periodEnd { | ||
| window.resetAt = Self.isoString(end) | ||
| } | ||
| if window.periodStart == nil, let start = grpc.periodStart { | ||
| window.periodStart = Self.isoString(start) | ||
| } | ||
| windows[index] = window |
There was a problem hiding this comment.
gRPC fallback never re-derives status/isExhausted after filling the percent
A fallback-filled percentage leaves status, isExhausted, skip, and skipReason stale, so consumers miss an exhausted pool. Apply .normalizedForExport() to the mutated window after setting remainingPercent, resetAt, and periodStart before storing it.
Prompt for LLM
File Sources/QuotaCore/LocalQuotaReader.swift, lines 365-374:
gRPC fallback never re-derives status/isExhausted after filling the percent
The gRPC fallback writes `remainingPercent`/`remainingUnknown` onto a window that the `window(...)` helper already passed through `.normalizedForExport()`, but it never re-derives `status`, `isExhausted`, `skip` or `skipReason` from that new percentage (QuotaModels.swift:261-300 is the only code that derives them; `normalized()` at line 782 does not). A Grok subscription that only the fallback can fill leaves the app with `status == .unknown` and `isExhausted == false` even when the credits pool is at 0%, so `QuotaComponents.swift:545` never colors the row as exhausted and `QuotaPublisher.swift:332-333` publishes `"unknown"`/`false` for a dead pool. Re-run `.normalizedForExport()` on the mutated window before storing it, after the resetAt/periodStart assignments, so status/isExhausted/skip are derived from the percentage that was just filled in.
Suggested code:
var window = windows[index]
window.remainingPercent = 100 - grpc.usedPercent
window.remainingUnknown = false
if window.resetAt == nil, let end = grpc.periodEnd {
window.resetAt = Self.isoString(end)
}
if window.periodStart == nil, let start = grpc.periodStart {
window.periodStart = Self.isoString(start)
}
windows[index] = window.normalizedForExport()
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| func subscriptionIndex(_ key: String) -> Int? { | ||
| windows.firstIndex { window in | ||
| window.canonicalProviderKey == key | ||
| && (window.id.hasSuffix(":subscription") || window.id == "xai:subscription") |
There was a problem hiding this comment.
Suffix matching can misidentify the canonical subscription window
Suffix matching can classify grok:legacy:subscription as canonical and deduplicate the wrong row. Replace the suffix check on window.id with exact membership in ["grok:subscription", "xai:subscription"].
Kody rule violation: Avoid Substring Matching for Canonical Limit/Window Labels
Prompt for LLM
File Sources/QuotaCore/QuotaModels.swift, line 616:
Suffix matching can misidentify the canonical subscription window
The subscription-window check uses suffix matching, so an unrelated ID such as `grok:legacy:subscription` can be treated as canonical. Canonical limit and window identifiers must be matched exactly to prevent deduplicating the wrong row. Compare `window.id` against the exact set `{grok:subscription, xai:subscription}`.
Kody rule violation: [Avoid Substring Matching for Canonical Limit/Window Labels](https://app.kodus.io/settings/code-review/1370549454/kody-rules/60aefd5e-dadd-4523-8848-f0331c1073a7?teamId=69360e3a-6b41-45d2-9e49-fde78fe49457)
Suggested code:
&& ["grok:subscription", "xai:subscription"].contains(window.id)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| let normalizedWindows = Self.dedupedGrokSubscription(windows.map { $0.normalized() }) | ||
| var grouped = Dictionary<String, [QuotaWindow]>(minimumCapacity: normalizedWindows.count) | ||
| for window in normalizedWindows { | ||
| grouped[window.canonicalProviderKey, default: []].append(window) |
There was a problem hiding this comment.
Dedupe ignores providerGroups, so the dropped duplicate is re-added
The provider-group merge can re-add the fleet window that dedupedGrokSubscription removed, leaving duplicate Grok rows. Deduplicate each provider group’s normalized windows before merging them into seen.
Prompt for LLM
File Sources/QuotaCore/QuotaModels.swift, lines 637-640:
Dedupe ignores providerGroups, so the dropped duplicate is re-added
`dedupedGrokSubscription` is applied only to the top-level `windows` array, while the `providerGroups` merge that follows rebuilds `seen` from the already-deduped list, so the fleet `xai:subscription` window that the dedupe just dropped is re-added from its provider group. A response that carries the fleet reading in `providerGroups` (the additive manifest path) still yields two rows for the one Grok credits pool, which is exactly the disagreement this PR sets out to remove. Apply the same dedupe to the providerGroups merge, or compute the dropped window identities and skip them when filling absent IDs.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .minimumScaleFactor(0.75) | ||
| .multilineTextAlignment(.trailing) | ||
| .frame(width: 52, alignment: .trailing) | ||
| .frame(width: 32, alignment: .trailing) |
There was a problem hiding this comment.
iOS quota-card layout change lacks simulator screenshot evidence
The fixed-column sizing in CompanionContentView changes truncation, widths, alignment, and timestamps, so missing simulator evidence leaves a risky layout change unreviewed. Consult fleet recall with no simulator small iOS change, then attach or link a simulator screenshot of the canonical quota cards.
Kody rule violation: Verify UI changes with automated screenshots: Playwright for web, the hosted iOS build for small iPhone changes
Prompt for LLM
File ios/CodeCapsCompanion/App/Views/CompanionContentView.swift, line 916:
iOS quota-card layout change lacks simulator screenshot evidence
The added fixed-column sizing is part of a structural iOS quota-meter layout rewrite that changes caption truncation, column widths, alignment, and per-window timestamps. Rule 4 requires screenshot evidence for risky iOS layout-logic changes, but no CI-captured simulator screenshot or `xcrun simctl io booted screenshot` artifact is shown in the supplied PR material. Consult fleet recall with the exact query `no simulator small iOS change` to confirm this is not merely a small spacing tweak, then attach or link a simulator screenshot showing the revised canonical quota cards.
Kody rule violation: [Verify UI changes with automated screenshots: Playwright for web, the hosted iOS build for small iPhone changes](https://app.kodus.io/settings/code-review/global/kody-rules/5777a4dc-cfc2-49cf-9196-b9a77fa48691?teamId=69360e3a-6b41-45d2-9e49-fde78fe49457)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Mac build is red on the merge ref with 4 test failures. Evidence per failure, all traced to the canonical-cards rewrite in db0cde1 (neither main nor the branch touched the test files since the merge-base, so the tests still encode the pre-redesign structure):
Leaving the calls to your lane since this is an active redesign - flagging so the failures don't sit. The #304-era owner tests are the contract being asserted. |
Resolve the CopilotQuotaReader.swift conflict by taking main's line wrapping. Both sides carry identical arguments (token streams match after whitespace is removed); main only rewrapped the resetAt/window and remainingUnknown/status lines. The branch's Grok xAI work is untouched.
This branch has not been deployed
1 participant
Popup/plan fixes are in PR #304 (auto-merge armed). Folder-structure standardization is SWU-22; load-bearing paths (SPM/Xcode/release) intentionally unmoved.
Summary
This PR improves Grok quota completeness and consistency across local and fleet readings, adds a secondary Grok billing source, and simplifies the iOS quota experience to show one canonical reading per allowance.
Functional changes
Grok quota fallback
Grok/xAI subscription deduplication
groksubscription reading and fleetxai:subscriptionreading into one dashboard entry.occurredAttimestamp and leaves unrelated xAI windows and single-sided subscription data unchanged.Canonical iOS quota cards
Other quota and branding updates
Test coverage
GrokGrpcCreditsTestscovering framed and raw replies, product-share exclusion, active-period implicit zero, malformed input, period extraction, and the exact request frame.GrokXaiDedupeTestscovering fresher local and fleet readings, preservation of unrelated xAI windows, and single-sided subscription data.Outstanding review items
providerGroups; this can select the wrong duplicate or allow a dropped duplicate to be reintroduced.