Skip to content

perf: avoid reflecting nested cost history projects on submenu open - #2063

Merged
steipete merged 1 commit into
steipete:mainfrom
Yuxin-Qiao:perf/cost-history-submenu-fingerprint
Jul 11, 2026
Merged

perf: avoid reflecting nested cost history projects on submenu open#2063
steipete merged 1 commit into
steipete:mainfrom
Yuxin-Qiao:perf/cost-history-submenu-fingerprint

Conversation

@Yuxin-Qiao

Copy link
Copy Markdown
Contributor

Summary

  • Cost History’s hosted-submenu signature recursively reflected each project and source, including their full daily and model-breakdown histories, on the main-thread menu-open path. Project rollups made this scale with data that the submenu never renders.
  • Replace the reflective signature with a typed visible-content fingerprint bounded to the global daily chart, five displayed projects, and two displayed sources per project.
  • Follow-up to Reduce hosted chart submenu stalls #1384; regression amplified by project rollups added in Add Codex project cost rollups #1884.

Test plan

  • swift test --filter CostHistoryChartMenuViewTests
  • swift test --filter StatusMenuHostedSubmenuRefreshTests
  • swift test --filter StatusMenuCodexCostHistoryRefreshTests
  • swift build -c release
  • make check
  • make test

Performance evidence

Release, 30 samples after warm-up, 30 days / 4 models / 1 source per project (swift run -c release --package-path /tmp/CostBench CostBench <projects> 1):

Projects Reflection median Reflection P95 Fingerprint median Fingerprint P95
5 2.19 ms 3.75 ms 0.001 ms 0.003 ms
20 4.49 ms 4.96 ms 0.001 ms 0.003 ms

Audit baseline on upstream main (same workload): 5 projects 18.29 ms / 20.26 ms P95; 20 projects 61.23 ms / 64.39 ms P95; visible projection ~0.06–0.07 ms median.

Risk

Main risk is an incomplete fingerprint masking a visible update. The implementation keeps the full bounded global daily array and tests every project/source field that the view renders. Project/source nested daily data is intentionally excluded because it is not displayed.

Made with Cursor

Replace Cost History hosted-submenu String(reflecting:) signatures with a typed visible-content fingerprint so menu-open work no longer traverses nested project/source daily data the UI never renders.

Co-authored-by: Cursor <cursoragent@cursor.com>
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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. 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, 10:06 AM ET / 14:06 UTC.

Summary
The PR replaces recursive reflection of cost-history submenu state with a typed fingerprint of rendered content and adds focused refresh-invalidation tests.

Reproducibility: yes. Current main synchronously reflects the complete nested project payload on submenu refresh, and the PR’s release benchmark measures that computation; this Linux review environment cannot rerun the macOS menu path.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 3 production files, 2 test files. The runtime change is narrowly contained to hosted render-signature computation and storage.
  • Regression coverage: 578 test additions. The test expansion covers fingerprint semantics and actual hosted-view identity behavior.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Next step before merge

  • No automated repair is indicated; this clean implementation PR should continue through ordinary maintainer review while required checks complete.

Security
Cleared: The patch changes only local Swift render-state comparison and tests, with no dependency, credential, network, workflow, or release surface.

Review details

Best possible solution:

Use the typed visible-content fingerprint so submenu refresh remains sensitive to rendered changes without traversing hidden nested histories.

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

Yes. Current main synchronously reflects the complete nested project payload on submenu refresh, and the PR’s release benchmark measures that computation; this Linux review environment cannot rerun the macOS menu path.

Is this the best way to solve the issue?

Yes. An explicit Equatable projection of rendered inputs is narrower and more maintainable than recursively reflecting the entire snapshot, and focused tests cover retained and invalidated states.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 99c3d94d4313.

Label changes

Label changes:

  • add P2: The PR fixes measurable user-visible submenu latency in a bounded rendering path without critical availability or data impact.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides copied release-benchmark output with the exact command, workload, sample count, and before/after median and P95 results on the changed path.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides copied release-benchmark output with the exact command, workload, sample count, and before/after median and P95 results on the changed path.

Label justifications:

  • P2: The PR fixes measurable user-visible submenu latency in a bounded rendering path without critical availability or data impact.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides copied release-benchmark output with the exact command, workload, sample count, and before/after median and P95 results on the changed path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides copied release-benchmark output with the exact command, workload, sample count, and before/after median and P95 results on the changed path.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Authored the merged hosted-submenu retention work and project cost-rollup expansion whose interaction this PR optimizes. (role: introduced behavior and recent area contributor; confidence: high; commits: 7c0ed036e2e6, 1e49619e7330; files: Sources/CodexBar/StatusItemController+HostedSubmenus.swift, Sources/CodexBar/CostHistoryChartMenuView.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
steipete merged commit 060ffe1 into steipete:main Jul 11, 2026
8 checks passed
steipete added a commit that referenced this pull request Jul 11, 2026
Project the hosted cost-history fingerprint onto visible chart, model, project, and source fields; preserve raw daily availability transitions.

Follow-up to #2063. Changelog retains contributor credit to @Yuxin-Qiao.
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. proof: sufficient Contributor real behavior proof is sufficient. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants