Skip to content

perf: match cost history fingerprint to visible content - #2067

Merged
steipete merged 2 commits into
mainfrom
codex/cost-history-visible-fingerprint
Jul 11, 2026
Merged

perf: match cost history fingerprint to visible content#2067
steipete merged 2 commits into
mainfrom
codex/cost-history-visible-fingerprint

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

  • compare only cost-history fields the hosted submenu actually renders
  • preserve refreshes for raw daily-history availability transitions
  • omit Codex project rows from providers whose cost view does not render them
  • add focused fingerprint regressions and the missing changelog credit for @Yuxin-Qiao

Follow-up to #2063. The original performance fix landed while its deeper review and packaged QA were still running.

Why

The first typed fingerprint still retained complete daily entries, so hidden accounting fields could invalidate the hosted submenu as history grew. This projects the fingerprint onto visible chart/model/project/source data, bounds it to the five rendered projects and two rendered sources, and keeps the raw empty/nonempty state used by submenu availability.

Proof

  • focused: 60 tests across CostHistoryChartMenuViewTests, StatusMenuCodexCostHistoryRefreshTests, and StatusMenuOpenRefreshTests
  • make check
  • full suite: 610 selections, 51/51 groups, zero retries/timeouts
  • autoreview: clean after fixing the raw daily availability transition
  • release build: passed on the reviewed candidate
  • signed packaged debug QA: Developer ID + Gatekeeper accepted; 20 projects × 30 days × 2 sources; menu rendered only projects 00–04 and both visible sources; 30 open/hover/close cycles stayed byte-identical; no external TCP connection or hang/crash log signal; production app PID unchanged
  • optimized benchmark: typed visible fingerprint was 396× faster at 5 projects and 1,442× faster at 20 projects than the legacy reflective signature

Risk

Low and localized to cost-history hosted-submenu invalidation. No dependencies, persistence, credentials, or provider network behavior changed.

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jul 11, 2026
@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 2:12 PM ET / 18:12 UTC.

Summary
The branch makes cost-history submenu invalidation provider-aware and limited to rendered daily, model, project, and source data, preserves raw daily availability transitions, adds focused regression coverage, and updates the unreleased changelog.

Reproducibility: yes. at source level. Current main compares complete daily entries, so hidden accounting-field changes can invalidate the submenu; the PR body also supplies packaged stress and benchmark evidence for the real menu path.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 2 source files, 1 test file, 1 changelog entry. The production change is localized, with most of the patch devoted to focused regression coverage.
  • Rendered bounds: 5 projects and 2 sources per project. The fingerprint now has the same fixed upper bound as the project rows visible in the submenu.
  • Reported benchmark: 396× faster at 5 projects; 1,442× at 20 projects. The PR body reports substantial optimized gains over the legacy reflective signature on the reviewed candidate.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Risk before merge

  • [P1] The exact reviewed head still has pending macOS test and Linux build jobs, so merge should wait for their completion.

Maintainer options:

  1. Decide the mitigation before merge
    Merge the render-aligned fingerprint and regression tests after exact-head checks pass, with the repository owner retaining or relocating the unreleased changelog credit through the normal release process.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No repair lane is needed; this owner-authored, correct patch only needs normal exact-head check gating and merge handling.

Security
Cleared: The diff introduces no dependency, external code execution, credential, permission, persistence, network, packaging-script, or supply-chain change.

Review details

Best possible solution:

Merge the render-aligned fingerprint and regression tests after exact-head checks pass, with the repository owner retaining or relocating the unreleased changelog credit through the normal release process.

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

Yes, at source level. Current main compares complete daily entries, so hidden accounting-field changes can invalidate the submenu; the PR body also supplies packaged stress and benchmark evidence for the real menu path.

Is this the best way to solve the issue?

Yes. The patch uses the same validity helper, ordering, provider filter, field conditions, and row limits as rendering while separately preserving raw empty/nonempty availability.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 300ddee98ec7.

Label changes

Label changes:

  • add P2: This is a localized menu-open performance and refresh-correctness fix with limited blast radius.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The owner-authored PR is exempt from the external-contributor proof gate; it nevertheless reports signed packaged QA, repeated live menu cycles, release-build validation, and optimized benchmark evidence.

Label justifications:

  • P2: This is a localized menu-open performance and refresh-correctness fix with limited blast radius.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The owner-authored PR is exempt from the external-contributor proof gate; it nevertheless reports signed packaged QA, repeated live menu cycles, release-build validation, and optimized benchmark evidence.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Authored both follow-up commits and previously introduced Codex project cost rollups in the same rendering area, making him the strongest routing candidate for landing and release-note scope. (role: repository owner and recent feature contributor; confidence: high; commits: 1e49619e7330, dc85f4790bcc, d835b437c079; files: CHANGELOG.md, Sources/CodexBar/CostHistoryChartMenuView.swift, Sources/CodexBar/StatusItemController+HostedSubmenus.swift)
  • Yuxin-Qiao: Git blame and merged history show that the typed cost-history fingerprint refined by this PR originated in their predecessor change. (role: original typed-fingerprint contributor; confidence: high; commits: 060ffe13012d; files: Sources/CodexBar/CostHistoryChartMenuView.swift, Sources/CodexBar/StatusItemController+HostedSubmenus.swift, Tests/CodexBarTests/CostHistoryChartMenuViewTests.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.

@steipete

Copy link
Copy Markdown
Owner Author

Exact-head proof for d835b437c0791b4a5cf665d25558d179e74245ea:

  • Scope: 4 files, +287/−45; no dependency, persistence, credential, or network-contract changes.
  • Focused: 60 tests across cost-history fingerprint, hosted-refresh, and open-menu suites.
  • Full local: 610 selections in 51/51 groups; zero failures, retries, or timeouts.
  • Quality: make check clean; structured autoreview clean after adding the raw daily-availability transition regression.
  • Build/signing: exact-head debug app embeds d835b437; strict deep codesign passes; Gatekeeper accepts the Developer ID bundle.
  • Exact-head CLI fixture: 30 days, 20 ordered projects, 2 canonical sources per project, all local; no provider credentials required.
  • Exact-head live menu: 10 open/hover/close cycles; exactly projects 00–04 and both sources for each, no hidden projects/overflow row; cycle 1 and 10 AX output byte-identical; no established TCP connection; production app process unchanged.
  • Performance: optimized visible fingerprint measured 396× faster with 5 projects and 1,442× faster with 20 projects than the legacy reflective signature.
  • Hosted: lint, changes, security, Linux arm64/x64, both macOS shards, and aggregate lint-build-test all green at this exact head.

Local screenshots remain private. An installed-tool auto-detection Keychain prompt appeared during isolated app startup; it was denied, no credential was read, and all QA preferences/app-group/widget state were restored exactly.

@steipete
steipete merged commit 0bb8749 into main Jul 11, 2026
8 checks passed
@steipete
steipete deleted the codex/cost-history-visible-fingerprint branch July 11, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant