Skip to content

Add predictive pace warnings#1960

Merged
steipete merged 12 commits into
steipete:mainfrom
vincent-peng:predictive-pace-warnings
Jul 9, 2026
Merged

Add predictive pace warnings#1960
steipete merged 12 commits into
steipete:mainfrom
vincent-peng:predictive-pace-warnings

Conversation

@vincent-peng

@vincent-peng vincent-peng commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a default-off Predictive pace warnings setting for Codex and Claude session/weekly limits
  • notify when the existing pace projection predicts exhaustion before reset, once per provider/account/window risk episode
  • re-arm after an authoritative healthy observation or a genuinely new reset cycle, while tolerating provider reset-time corrections and relative-TTL drift
  • keep selected Claude accounts isolated by stable account ID and keep ambient OAuth/CLI identity stable without coupling web/API observations to an unrelated CLI account
  • reuse the existing sound and on-screen delivery controls even when threshold warnings are disabled
  • keep account identity out of notification copy when personal-info hiding is enabled
  • localize the preference and notification copy across all app catalogs

Closes #1299.

Maintainer improvements

  • synchronized the branch with current main, including the compact quota-warning settings editor from Polish quota warning settings #1974
  • added stable view-state coverage for predictive-only delivery controls
  • hardened background-work revision updates against no-op preference assignments
  • repaired account identity boundaries across selected accounts, CLI/OAuth, web, and API strategies
  • used the winning Claude OAuth credential's one-way owner discriminator when OAuth usage has no email and active-account metadata is unavailable
  • replaced exact-second reset identities after live testing exposed duplicate alerts from small reset-time movement
  • added the Unreleased changelog entry, preserving and thanking @vincent-peng

Validation

  • swift test --filter PredictivePaceWarningTests — 19 passed
  • focused quota/settings integration coverage — 91 passed
  • make check — passed
  • make test — all 49 shards passed
  • branch-wide structured autoreview against the landed Polish quota warning settings #1974 base — clean, no actionable findings
  • dependencies unchanged

Exact packaged/live proof

Developer-ID signed debug app on macOS 27 build 26A5368g, arm64, Xcode 26.6 / SDK 26.5:

  • exact CodexGitCommit=094233d4; deep/strict signature valid; Gatekeeper accepted
  • binary SHA-256 e51cb36ae854ed0af5823eb22b314f8cec0dae210ad19855fe80b932ac27fbca
  • isolated config enabled only a synthetic Codex CLI fixture; browser cookies and Keychain access were disabled; no real credentials were used
  • source-blind alert-cycle proof on candidate e8fbede0 produced zero warnings by default, exactly two enabled warnings (Codex Session and Weekly), and zero repeats after both reset timestamps moved
  • the subsequent exact-head change is isolated to the no-email Claude OAuth account discriminator; the exact-head focused/full suites reran after that change
  • the exact-head live menu showed the synthetic 20% Session / 10% Weekly data with a clearly fictitious account
  • exact-head Settings showed Predictive pace warnings enabled, threshold warnings disabled, and both sound/on-screen delivery controls still visible
  • sound and on-screen delivery were disabled during the alert run

The accepted product contract is documented in docs/superpowers/specs/2026-07-01-predictive-pace-warning-notifications-design.md.

@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 9:55 AM ET / 13:55 UTC.

Summary
The branch adds default-off predictive pace warnings for Codex and Claude quota windows with account scoping, notification delivery, settings UI, localization, and regression tests.

Reproducibility: not applicable. as a bug reproduction: this is an implementation PR for an accepted feature request. Source inspection gives high confidence about the implemented paths, while proof sufficiency remains a PR validation gate.

Review metrics: 3 noteworthy metrics.

  • Changed Surface: 36 files changed, 1,295 added, 46 removed. The feature crosses settings, notification runtime, provider refresh identity, localization, and tests.
  • Localization Catalogs: 23 Localizable.strings files modified. New user-visible settings and notification copy must keep placeholders consistent across app catalogs.
  • Focused Tests: 1 added test file, 721 lines. The state-machine and account-identity tests are the main evidence for non-rendered provider paths.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Add redacted current-head logs, terminal output, or a reproducible validation transcript for the Claude OAuth no-email fallback path.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes exact-head app/settings proof and earlier alert-cycle proof, but it does not show the current-head Claude OAuth no-email fallback running in a real setup after the final discriminator change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P2] The PR body's exact-head live proof does not exercise the final Claude OAuth no-email owner fallback; it relies on tests plus the claim that the last change is isolated to the discriminator.
  • [P1] Incorrect Claude account discrimination could suppress or duplicate predictive alerts for OAuth users whose usage snapshots have no rendered email.

Maintainer options:

  1. Prove The No-Email Claude OAuth Path (recommended)
    Require redacted current-head runtime output showing the winning OAuth credential owner discriminator still allows exactly one scoped predictive warning when active-account metadata is absent.
  2. Accept Test-Only Discriminator Validation
    Maintain the branch as-is if maintainers decide the final auth-provider discriminator change is sufficiently covered by focused tests and does not need live OAuth proof.

Next step before merge

  • [P1] No code repair is queued; the remaining blocker is maintainer proof-sufficiency judgment or contributor-provided current-head real behavior proof for the Claude OAuth no-email path.

Maintainer decision needed

  • Question: Is current-head runtime proof for the Claude OAuth no-email discriminator required before this predictive warning PR can merge?
  • Rationale: The code finding appears fixed, but the posted proof does not show the final current-head auth-provider fallback running in a real setup rather than only through focused tests.
  • Likely owner: steipete — steipete authored the accepted design and the latest Claude identity fallback commit, so they are best positioned to decide whether proof is sufficient for that non-rendered path.
  • Options:
    • Require Current-Head Claude Proof (recommended): Ask for redacted logs, terminal output, or a reproducible live run that exercises the no-email Claude OAuth fallback on 094233d before merge.
    • Accept Isolated-Change Proof: Treat the earlier alert-cycle proof plus current-head focused/full tests as enough because the final commit only changes the discriminator fallback.

Security
Cleared: No concrete security or supply-chain issue is visible; the diff adds Swift app logic, tests, localizations, and uses existing one-way OAuth owner evidence without dependency, workflow, secret, or package-resolution changes.

Review details

Best possible solution:

Land the accepted implementation after maintainers accept the proof boundary or after the PR adds redacted current-head runtime proof for the Claude OAuth no-email fallback path.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction: this is an implementation PR for an accepted feature request. Source inspection gives high confidence about the implemented paths, while proof sufficiency remains a PR validation gate.

Is this the best way to solve the issue?

Yes for the code direction: the implementation follows the accepted default-off design and fixes the prior Claude OAuth fallback issue. Merge readiness is still gated by maintainer proof sufficiency, not by a remaining source defect.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 41331e5fb397.

Label changes

Label justifications:

  • P2: This is a normal-priority opt-in feature with bounded provider/account identity risk, not an emergency or active regression.
  • merge-risk: 🚨 auth-provider: The diff adds Claude OAuth, CLI, and token-account identity scoping for notifications, so incorrect account discrimination could affect provider-specific alert behavior after merge.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes exact-head app/settings proof and earlier alert-cycle proof, but it does not show the current-head Claude OAuth no-email fallback running in a real setup after the final discriminator change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Authored the merged predictive-warning decision, current-main quota settings merge, and the latest PR-head Claude no-email OAuth fallback fix. (role: accepted design owner and recent identity contributor; confidence: high; commits: bcff88880788, 41331e5fb397, 094233d4d165; files: docs/superpowers/specs/2026-07-01-predictive-pace-warning-notifications-design.md, Sources/CodexBar/PredictivePaceWarnings.swift, Sources/CodexBar/UsageStore+Refresh.swift)
  • Zihao-Qi: Recent merged quota-warning settings work changed the shared settings UI that this branch extends for predictive-only delivery controls. (role: recent adjacent settings contributor; confidence: medium; commits: 9718948dcdc1, daf7677335c4, 5c6e6f99847d; files: Sources/CodexBar/QuotaWarningSettingsViews.swift, Sources/CodexBar/PreferencesGeneralPane.swift, Sources/CodexBar/SettingsStore+Defaults.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (13 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-07T05:37:13.249Z sha f34ca8a :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-07T05:52:24.324Z sha f34ca8a :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-07T06:08:52.909Z sha f34ca8a :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-07T08:51:33.918Z sha f34ca8a :: needs maintainer review before merge. :: none
  • reviewed 2026-07-07T08:58:03.304Z sha f34ca8a :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T13:09:56.977Z sha e8fbede :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T13:18:17.496Z sha e8fbede :: needs changes before merge. :: [P2] Use the OAuth owner when active UUID is missing
  • reviewed 2026-07-09T13:44:17.627Z sha 094233d :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jul 7, 2026
@vincent-peng vincent-peng marked this pull request as ready for review July 7, 2026 03:20
@vincent-peng

Copy link
Copy Markdown
Contributor Author

Added redacted runtime proof to the PR body and marked the PR ready for review.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8c8e98edf

ℹ️ 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".

Comment thread Sources/CodexBar/PredictivePaceWarnings.swift
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 7, 2026
@vincent-peng

Copy link
Copy Markdown
Contributor Author

Added two follow-up commits after reviewing #1299, #1831, and the latest feedback:

  • d4048c2 localizes predictive pace warning settings and notification copy across app catalogs while keeping provider-first notification titles.
  • 5023a79 threads Claude OAuth owner evidence into predictive warning state as a non-rendered discriminator for no-email OAuth snapshots, with focused regression coverage.

Validation:

  • swift test --filter PredictivePaceWarningTests
  • node Scripts/check-app-locales.mjs --test && node Scripts/check-app-locales.mjs (passes; existing unrelated missing-key warnings remain)
  • make check

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 7, 2026
@vincent-peng

Copy link
Copy Markdown
Contributor Author

Updated the PR after the latest review:

  • f34ca8a passes the Claude OAuth owner discriminator through the selected token-account refresh path as well.
  • Added focused regression coverage for selected Claude OAuth token accounts with no rendered email.
  • Updated the PR body summary/validation to include this follow-up.

Validation:

  • swift test --filter PredictivePaceWarningTests (11/11)
  • make check

Current CI on f34ca8a is green for lint, Linux builds, GitGuardian, and one macOS shard; remaining macOS shards are still pending.

@vincent-peng

Copy link
Copy Markdown
Contributor Author

All checks are green on f34ca8a, and the selected Claude OAuth token-account follow-up is pushed and documented.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@vincent-peng

Copy link
Copy Markdown
Contributor Author

Added fresh current-head proof to the PR body.

Summary:

  • f34ca8a redacted app runtime with disposable HOME/config and a synthetic Codex app-server shows both predictive-pace-warning-codex-session and predictive-pace-warning-codex-weekly being enqueued.
  • swift test --filter PredictivePaceWarningTests on f34ca8a passes 11/11, including the selected Claude OAuth no-email owner discriminator path added after the older runtime proof.

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 7, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 7, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8fbede03e

ℹ️ 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".

Comment thread Sources/CodexBar/UsageStore+Refresh.swift Outdated
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jul 9, 2026
Co-authored-by: Vincent Peng <mail@vincentpeng.me>
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Exact-head maintainer proof for 094233d4d16518cc5fbee78aca65e52d76ca816a:

  • Public Model Identifier Gate: PASS (no model-bearing public mutation).
  • Focused predictive warning suite: 19 passed.
  • Focused quota/settings integration coverage: 91 passed.
  • make check: passed.
  • make test: all 49 shards passed.
  • Structured branch review: clean; no accepted/actionable findings.
  • Fresh exact-head automated review gap fixed: no-email Claude OAuth usage now falls back to its existing one-way credential owner discriminator; both review threads resolved with regression coverage.
  • Developer-ID signed debug package: CodexGitCommit=094233d4; deep/strict signature valid; Gatekeeper accepted; app binary SHA-256 e51cb36ae854ed0af5823eb22b314f8cec0dae210ad19855fe80b932ac27fbca.
  • Exact-head live fixture: isolated synthetic Codex CLI, Keychain/browser access disabled; menu showed 20% Session / 10% Weekly risk and the clearly fictitious fixture account.
  • Exact-head Settings proof: predictive warnings enabled while threshold warnings were disabled; sound and on-screen delivery controls remained visible.
  • Source-blind alert-cycle proof on the prior candidate produced zero default-off alerts, exactly one Session + one Weekly alert when enabled, and zero repeats after both reset timestamps moved. The subsequent exact-head change is isolated to Claude OAuth account discrimination and reran focused/full proof.
  • Dependencies: unchanged; no freshness audit required.

Screenshots stayed local. CI/security were checked against this exact head before merge.

@steipete steipete merged commit ba64aa0 into steipete:main Jul 9, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: predictive pace warning notifications

2 participants