Repository navigation
fix(quota): one payload verdict across surfaces (F1) + dedupe own push vs local copy (F2) - #322
jaywedgeworth22 wants to merge 13 commits into
Conversation
…local copy (F2) F1 - QuotaWindowVocabulary.isOwnDeviceWindow checked the local-mac: id prefix before the producer instance, while FleetOrigin.isOwnPush checks the instance first. One payload (local-mac:xai:subscription pushed by a foreign producer instance) read as this device's own on the phone and widget and as a copy on the Mac. The instance now decides first in both, so the surfaces cannot drift; prefix and source fallbacks are unchanged when no instance is present. F2 - QuotaResponse.platformSections keyed its dedupe on the raw window id, and only across provider-group additions. This Mac's plan-details window (local-mac:xai:plan-metadata) and its own push coming back (xai:plan-metadata) name one payload but survived as two rows. The key now strips the localDeviceWindowPrefix and top-level windows dedupe too. Regression tests pin both: the foreign-instance case against the prefix-first order, and the one-payload count against the raw-id key.
jaywedgeworth22
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| Autofix run https://github.com/Simple-With-Us/CodeCaps/actions/runs/40074626665 started for `sentry[bot]`'s tagged request. Owner/sentry request: exempt from the per-bot cap (owner rule 2026-10-20). |
|
Red-proof preserved from #321 at 5f2cb3b: https://github.com/Simple-With-Us/CodeCaps/actions/runs/40074352979/job/120814431133 . At 07:33:31Z both PlanDetailsDedupeTests assertions reported 2 rows instead of 1. At 07:33:35Z testAForeignProducerInstanceOutranksTheLocalMacPrefix failed XCTAssertFalse. The first corrective head 3cd6eca passed Mac tests but introduced the valid multi-producer collision finding above. The new fix moves dedupe to own-push adoption and adds actual refresh/file-poll assertions; new execution is still pending, so the old green is not current-head clearance. |
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
|
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 start-review |
This comment has been minimized.
This comment has been minimized.
|
Source comment uses one space instead of two after a sentence The new Kody rule violation: Sentence gap by surface: two ASCII spaces in files, U+00A0 plus a space in rendered copy and GitHub text Prompt for LLM |
|
PR description records four files, but the PR changes six files The verification record is stale: it claims four files while the PR changes six, including Kody rule violation: Preserve verification records Prompt for LLM |
|
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}
| } | ||
| let localKeys = Set(localWindows.map(key)) | ||
| return ownPush.filter { | ||
| !localProviders.contains($0.canonicalProviderKey) && !localKeys.contains(key($0)) |
There was a problem hiding this comment.
Provider-wide deduplication drops distinct pushed windows
Any local reading for a provider currently suppresses every pushed window from that provider, so distinct new windows never reach the merged result. Remove localProviders and deduplicate ownPush only against exact matching keys in localKeys.
Kody rule violation: Drop already-present rows when writing merged snapshot state
Prompt for LLM
File Sources/QuotaCore/QuotaModels.swift, line 436:
Provider-wide deduplication drops distinct pushed windows
The provider-wide `localProviders` check drops every pushed window for a provider whenever any local reading for that provider exists, so distinct new windows are not published. Deduplicate only against an exact matching window key; also remove the now-unused `localProviders` binding.
Kody rule violation: [Drop already\-present rows when writing merged snapshot state](https://app.kodus.io/settings/code-review/1370549454/kody-rules/3a2f861e-cca2-46f1-900a-390d056dc0a6?teamId=69360e3a-6b41-45d2-9e49-fde78fe49457)
Suggested code:
!localKeys.contains(key($0))
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This is separate from the nil-cadence classifier fix. Provider-wide local-reading precedence already exists in merge base 3938a6b in both MonitorModel merge paths and is preserved here. A local reading still owns its provider; without a local reading, an informative own push can replace its matching empty local copy. Foreign producer grouping is not deduplicated by this adoption helper.
Tests/QuotaCoreTests/PlanDetailsDedupeTests.swift:testALocalReadingStillOwnsItsProviderAcrossDistinctWindowIds pins that behavior. Exact-window-only suppression would reverse the existing provider policy, not fix a duplicate copy. That reversal was not implemented by this classifier correction.
Current-head 864bb58 review approved with no new findings: #322 (review) . This does not itself dispose of the older contrary request. Can you explicitly withdraw this request for the corrective scope, or identify a regression introduced here that still requires changing provider precedence? Keeping this thread unresolved pending specific disposition.
There was a problem hiding this comment.
Withdrawn for this corrective scope.
I did not find a regression in this change that requires removing provider-wide precedence:
Sources/QuotaCore/QuotaModels.swift:428-436preserves the existing rule that a local reading owns its provider.Tests/QuotaCoreTests/PlanDetailsDedupeTests.swift:46-53explicitly pins that behavior across distinct window IDs.- The corrective change only permits an informative own push to replace its matching empty local copy; it does not change provider-level precedence.
The suggested exact-window-only behavior would therefore reverse the existing provider policy rather than fix a duplicate-copy regression introduced here. If exact-only suppression is still desired, it should be handled as a separate policy change, not as part of this classifier correction.
This comment has been minimized.
This comment has been minimized.
|
Current-head dispositions for the 14:18Z Kody round:
The original red proof remains at https://github.com/Simple-With-Us/CodeCaps/actions/runs/40074352979/job/120814431133 . That earlier failure, the intermediate fix, the current regression results and this comment-only correction are separate records. No merge or review waiver is claimed here. |
|
Corrective head for the three 976e38f findings: 15d9d83.
|
This comment has been minimized.
This comment has been minimized.
|
Correction to the pending native validation record: 15d9d83 failed to compile its test because the added assertion accessed private MonitorModel.localWindows. No native tests or red mutations executed at that head. 15d9d83 fixes only the test: it checks the actual AntigravityQuotaGroups.normalize result on the injected fixture, then checks the public response.windows retains the exact local ID after each actual merge path. Production visibility and quota behavior are unchanged. Native baseline, both differential mutations and restored-green execution remain pending; no clearance is claimed. |
| return ownPush.filter { | ||
| // A local reading still owns its provider. An empty local copy | ||
| // must not hide an informative returning push with the same key. | ||
| !localProviders.contains($0.canonicalProviderKey) |
There was a problem hiding this comment.
Provider-wide filter drops distinct pushed windows for the same provider
The provider-wide check in adoptedOwnPush suppresses every pushed window for a provider with any local reading, so distinct new rows never enter the merged snapshot. Compare exact windowKey values and suppress only a push when the matching local window has hasReading; remove the now-unused localProviders.
Also found in:
Tests/QuotaCoreTests/PlanDetailsDedupeTests.swift:53-53
Kody rule violation: Drop already-present rows when writing merged snapshot state
Prompt for LLM
File Sources/QuotaCore/QuotaModels.swift, line 438:
Provider-wide filter drops distinct pushed windows for the same provider
Rule 38 requires that distinct new rows are still written while only rows the consumer already holds are suppressed. The provider-wide check `!localProviders.contains($0.canonicalProviderKey)` rejects every own push for a provider as soon as any local reading exists for that provider, so an own push carrying a different window id (a distinct new row) is never adopted into the merged snapshot state. Deduplicate against the exact (provider, window id) key instead: suppress the own push only when a local window with the same key already carries a reading, and let other windows for the same provider through. The `localProviders` binding then becomes unused and can be dropped.
**Also found in:**
- `Tests/QuotaCoreTests/PlanDetailsDedupeTests.swift:53-53`
Kody rule violation: [Drop already\-present rows when writing merged snapshot state](https://app.kodus.io/settings/code-review/1370549454/kody-rules/3a2f861e-cca2-46f1-900a-390d056dc0a6?teamId=69360e3a-6b41-45d2-9e49-fde78fe49457)
Suggested code:
// Only an exact local reading for the same window suppresses an own
// push; distinct windows under the same provider stay adoptable.
!localWindows.contains { local in
local.hasReading && key(local) == key($0)
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
There was a problem hiding this comment.
This is separate from the nil-cadence classifier fix. Provider-wide local-reading precedence already exists in merge base 3938a6b in both MonitorModel merge paths and is preserved here. A local reading still owns its provider; without a local reading, an informative own push can replace its matching empty local copy. Foreign producer grouping is not deduplicated by this adoption helper.
Tests/QuotaCoreTests/PlanDetailsDedupeTests.swift:testALocalReadingStillOwnsItsProviderAcrossDistinctWindowIds pins that behavior. Exact-window-only suppression would reverse the existing provider policy, not fix a duplicate copy. That reversal was not implemented by this classifier correction.
Current-head 864bb58 review approved with no new findings: #322 (review) . This does not itself dispose of the older contrary request. Can you explicitly withdraw this request for the corrective scope, or identify a regression introduced here that still requires changing provider precedence? Keeping this thread unresolved pending specific disposition.
There was a problem hiding this comment.
Withdrawn for this corrective scope.
Provider-wide local-reading precedence is deliberate in Sources/QuotaCore/QuotaModels.swift: an own push is excluded when its provider already has a local reading. That existing policy is pinned by Tests/QuotaCoreTests/PlanDetailsDedupeTests.swift:testALocalReadingStillOwnsItsProviderAcrossDistinctWindowIds, which expects a distinct pushed window to be rejected when the provider has a local reading.
I found no regression from this change requiring exact-window-only suppression. Applying that request would reverse the preserved provider policy and could leave two readings for the same provider in the merged result.
Therefore, the older provider-precedence request is withdrawn for the F1/F2 corrective scope, and provider precedence remains unchanged.
|
@kody start-review |
|
Native differential execution now verified at exact 54ee1fe, Swift 6.4 on arm64 macOS 27.2, in a disposable detached worktree:
This verifies the scoped corrections for 4367393440 and 4367393578. The earlier Antigravity fixture coverage overstatement remains corrected; it is not retroactively treated as a differential proof. The first corrective head's private-access compile failure remains recorded separately. Current-head hosted Mac build/tests passed at https://github.com/Simple-With-Us/CodeCaps/actions/runs/40105707538/job/120906842523 and iOS compile passed at https://github.com/Simple-With-Us/CodeCaps/actions/runs/40105707538/job/120907744237 . The selected differential run is not a claim that it ran the entire suite. Current-head Kody review and remaining findings are separate gates, not waived by these results. |
|
Further same-key F2 correction in 1a8a2ad: both observable merge paths now omit an empty local copy only when an actually adopted own push with the same canonical window key carries a reading. The private collector-local windows and provider precedence are unchanged. The adoption documentation, exact replacement unit cases and both integration-path assertions were updated. This addresses the duplicate-payload part of 4367553220; it does not claim to restore local-only usage history or visually qualify Console/Glance rendering. Native baseline, adoption/dedupe/replacement differential mutations, current-head CI and review remain pending. The 54ee native proof remains evidence for that earlier head only. |
|
Native three-mutation verification at exact 1a8a2ad is complete (Swift 6.4, arm64 macOS 27.2). Baseline and restored runs each passed 11 selected tests: six PlanDetailsDedupeTests and five FleetRowSourceTests. All three disposable-worktree mutations compiled and executed all 11 tests, exiting 1 for the expected assertions:
The source was restored, selected tests passed again, and the isolated worktree was removed after a clean diff check. This supports the narrow duplicate-payload replacement correction for 4367553220. It does not claim local-only history recovery, pixel-verified Console/Glance rendering, a full-suite differential run or current-head Kody clearance. 4367553130 repeats the broader provider-precedence request in 4367393338. That behavior predates this PR; removing it changes local-versus-pushed provider selection, so it remains outside this same-payload correction rather than being described as nonexistent. The existing evidence-backed scope disagreement stands. Current hosted Mac build/test is green, iOS compile is still running as of this read, and current-head Kody review is not yet present. No merge clearance is claimed. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@kody start-review |
This comment has been minimized.
This comment has been minimized.
|
Single-space sentence gap in test source comment Plain-text source comments flag unclear sentence boundaries. Add a second space after Kody rule violation: Sentence gap by surface: two ASCII spaces in files, U+00A0 plus a space in rendered copy and GitHub text Prompt for LLM |
|
@kody review |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Changes
local-mac:ID prefix. A foreign installation's same-named payload stays foreign.Verification
Native Swift 6.4, arm64 macOS 27.2:
This is selected regression and build evidence, not a full-suite runtime, live collector, UI, history-recovery or installed-app claim. Tests used temporary homes and synthetic fixtures with network and owner-home access denied.
Current-head Kody review: #322 (review)
Boundaries and remaining disposition