Skip to content

CodexAIModelCatalog: Fix sort comparator for app hang - #813

Open
sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/codex-sort-app-hang
Open

CodexAIModelCatalog: Fix sort comparator for app hang#813
sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/codex-sort-app-hang

Conversation

@sentry

@sentry sentry Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

This PR addresses an app hang issue (REPOPROMPT-J0) caused by a Strict Weak Ordering violation in the sort comparator within CodexAIModelCatalog.swift.

Previously, the options.sorted closure in CodexAIModelCatalog.swift checked for leftBase == rightBase before evaluating lhs.isDefault != rhs.isDefault. This ordering could create a sorting cycle (A < B, B < C, C < A) under specific conditions: when two items shared the same base but differed in their isDefault flag, and a third item had a different base. This cycle caused Swift's sort function to enter a pathological state, leading to excessive reallocations and a main thread app hang of 2000ms or more.

The fix involves reordering the comparator closure to prioritize the isDefault check. The condition if lhs.isDefault != rhs.isDefault { return lhs.isDefault && !rhs.isDefault } is now the very first check, making isDefault the primary sort key. This ensures a valid total order, preventing the sorting cycle and resolving the app hang.

Fixes REPOPROMPT-J0

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Audit disposition — fix direction is correct; rebase and prove the comparator contract (2026-08-14)

Making isDefault the primary key removes the described cycle created by applying it only across different base IDs. The narrow code change is directionally correct.

Before merge, add property-style tests for irreflexivity, antisymmetry, and transitivity across generated option triples, plus a deterministic final tie-breaker when localized display names compare equal. Resolve the current integration/CI failure and rerun on the exact final head. I would retain this fix rather than close it.

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Deep-review assessment — 2026-08-14

Disposition: logically correct fix; add comparator-law coverage and rerun current CI. Making isDefault the primary key removes the described cycle between same-base and different-base options. The one-line ordering change is preferable to trying to patch individual model combinations.

Please add property/table tests for irreflexivity, antisymmetry, and transitivity across default/non-default, same/different base, effort ranks, and equal localized display names. Add a stable final tie-break such as normalized ID when localized comparison is .orderedSame, so distinct options do not remain unordered. The reviewed exact-head run also had an unrelated compile/test integration failure; rebase and require green current-head CI before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant