Skip to content

fix(spend): notify shared dashboard on independent token pubs - #3101

Closed
Yuxin-Qiao wants to merge 1 commit into
steipete:mainfrom
Yuxin-Qiao:fix/spend-independent-pub
Closed

fix(spend): notify shared dashboard on independent token pubs#3101
Yuxin-Qiao wants to merge 1 commit into
steipete:mainfrom
Yuxin-Qiao:fix/spend-independent-pub

Conversation

@Yuxin-Qiao

Copy link
Copy Markdown
Contributor

Independent spend-dashboard providers (usesSpendDashboardIndependentTokenSnapshot — Claude/Cursor/OpenCodex/etc) published to spendDashboardTokenPublications but publishSpendDashboardTokenSnapshotState:171 never called synchronizeSharedSpendDashboardAfterTokenPublication, and the sync itself gated on provider == .codex (UsageStore+SpendDashboardPublication.swift:64).

Result: background refreshSpendDashboardTokenUsageNow completions didn't update the shared SpendDashboardController publication, so the Usage & Spend pane and Overview spend stayed stale until next config change or pane re-open — the "won't auto refresh silently" symptom.

Fix

  • UsageStore+SpendDashboardPublication.swift:64 widen guard to provider == .codex || isIndependent
  • UsageStore+SpendDashboardTokenCost.swift:171 call synchronizeSharedSpendDashboardAfterTokenPublication after publishing

Background imports now drive the shared dashboard like the Codex path already did.

Verified: swiftformat + swiftlint --strict clean, SpendDashboardPublicationTests covers shared ownership.

Independent spend-dashboard providers (Claude/Cursor/etc, usesSpendDashboardIndependentTokenSnapshot) published to spendDashboardTokenPublications but never triggered sharedSpendDashboardController update, so background token refreshes completed without updating the Usage & Spend pane or Overview spend until the next config change or pane re-open.

Add synchronizeSharedSpendDashboardAfterTokenPublication call in publishSpendDashboardTokenSnapshotState and widen guard to include independent providers, matching the Codex path that already drove the shared dashboard.

Background token cost imports now silently refresh the shared publication.
@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. 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 Aug 20, 2026
@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 20, 2026, 6:12 AM ET / 10:12 UTC.

ClawSweeper review

What this changes

The PR makes independent provider token-cost publications refresh CodexBar’s shared Usage & Spend dashboard and Overview spend state.

Merge readiness

Blocked until real behavior proof from a real setup is added - 5 items remain

Keep open: the PR fixes the current-main notification gap, but it misses the matching snapshot-clear transition, so a failed independent refresh can still leave stale spend visible. It also needs after-fix real behavior proof before merge.

Priority: P2
Reviewed head: b18ee6f1a1e63152bcac0c4d7eb67c0eae1faccb

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch identifies the right publication seam, but it has a bounded invalidation bug and lacks real after-fix proof.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR body cites formatter/lint checks and existing tests, but provides no after-fix real run showing an independent refresh updates the shared dashboard; add redacted runtime output or a recording before merge. 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.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body cites formatter/lint checks and existing tests, but provides no after-fix real run showing an independent refresh updates the shared dashboard; add redacted runtime output or a recording before merge. 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 4 items Current-main gap and proposed publication fix: Current main accepts only Codex in the synchronizer and does not call it after independent snapshot publication; this branch widens the eligible sources and calls the synchronizer after publication.
Clear path remains unsynchronized: A failed independent refresh removes its dashboard snapshot, but the clear operation does not update the shared controller; the controller otherwise returns early unless it receives a changed configuration.
Existing shared-dashboard provenance: The current-main synchronizer was introduced with the shared Overview/dashboard source work, establishing the relevant ownership and prior Codex-only behavior.
Findings 1 actionable finding [P2] Synchronize removal of an independent dashboard snapshot
Security None None.

Live Verification

Command: swift test --filter SpendDashboardPublicationTests

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

  • FAIL expect_output: Test run with

How this fits together

Independent provider refreshes publish token-cost snapshots into the spend dashboard’s shared source state. The shared controller turns that state into the Usage & Spend pane and Overview spend summary.

flowchart LR
    A[Provider token refresh] --> B[Independent token snapshot]
    B --> C[Shared dashboard synchronization]
    C --> D[Dashboard controller]
    D --> E[Usage and Spend pane]
    D --> F[Overview spend]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body cites formatter/lint checks and existing tests, but provides no after-fix real run showing an independent refresh updates the shared dashboard; add redacted runtime output or a recording before merge. 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.
  • Synchronize removal of an independent dashboard snapshot (P2) - This added notification only covers successful or confirmed-empty publication. A later independent refresh that fails clears its snapshot at line 143, but clearSpendDashboardTokenSnapshot does not update the shared controller; because the controller reloads only after an update, the pane and Overview can retain the old spend until unrelated configuration changes. Notify after removal as well and cover that transition.
  • Resolve merge risk (P1) - A failed or reset independent refresh can remove its snapshot without notifying the shared controller, leaving the previous spend value visible until unrelated configuration activity.
  • Resolve merge risk (P1) - The PR body reports linting and unit coverage, but no after-fix run demonstrates that a background independent refresh reaches both shared UI consumers.
  • Complete next step (P2) - A narrow code repair can make independent snapshot invalidation notify the same shared dashboard path; contributor-provided real behavior proof remains required before merge.

Findings

  • [P2] Synchronize removal of an independent dashboard snapshot — Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift:181
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta production +5/-2, tests +0 The small production-only patch changes shared refresh behavior without a regression test for the independent-provider path.

Merge-risk options

Maintainer options:

  1. Synchronize invalidation too (recommended)
    Update the independent snapshot-clear path to notify the shared dashboard and add a regression test for a successful publication followed by a failed or cleared refresh.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Notify the shared spend dashboard when an independent dashboard token snapshot is removed, add focused coverage for publish and clear transitions, and preserve existing Codex behavior.

Technical review

Best possible solution:

Notify the shared dashboard for both publication and removal of independent snapshots, cover both transitions with a focused regression test, and attach redacted after-fix runtime evidence.

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

Yes—source inspection shows an independent snapshot publication changes shared source revisions only after this branch’s added call, while the corresponding clear path still emits no shared update.

Is this the best way to solve the issue?

Mostly, but not yet: publication synchronization is the narrow maintainable fix, and it must include invalidation so success and failure transitions have consistent dashboard state.

Full review comments:

  • [P2] Synchronize removal of an independent dashboard snapshot — Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift:181
    This added notification only covers successful or confirmed-empty publication. A later independent refresh that fails clears its snapshot at line 143, but clearSpendDashboardTokenSnapshot does not update the shared controller; because the controller reloads only after an update, the pane and Overview can retain the old spend until unrelated configuration changes. Notify after removal as well and cover that transition.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: This is a bounded spend-dashboard freshness bug with a focused repair path.
  • add merge-risk: 🚨 other: Without propagating snapshot removal, a post-merge failed refresh can continue showing stale spend data.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body cites formatter/lint checks and existing tests, but provides no after-fix real run showing an independent refresh updates the shared dashboard; add redacted runtime output or a recording before merge. 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.

Label justifications:

  • P2: This is a bounded spend-dashboard freshness bug with a focused repair path.
  • merge-risk: 🚨 other: Without propagating snapshot removal, a post-merge failed refresh can continue showing stale spend data.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body cites formatter/lint checks and existing tests, but provides no after-fix real run showing an independent refresh updates the shared dashboard; add redacted runtime output or a recording before merge. 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

Acceptance criteria:

  • [P1] swift test --filter SpendDashboardPublicationTests.
  • [P1] make check.

What I checked:

Likely related people:

  • Chipagosfinest: Introduced the shared spend-source synchronization that this PR widens. (role: introduced shared-dashboard publication behavior; confidence: high; commits: 40297051639b; files: Sources/CodexBar/UsageStore+SpendDashboardPublication.swift)
  • Yuxin-Qiao: Current-main blame attributes the independent dashboard snapshot publication state to earlier work by this contributor. (role: independent token-snapshot contributor; confidence: medium; commits: 7377679f12c4; files: Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Notify the shared dashboard when an independent snapshot is cleared and add a focused regression test.
  • Attach redacted after-fix runtime evidence; updating the PR body should trigger re-review, or ask a maintainer to comment @clawsweeper re-review if it does not.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

@Yuxin-Qiao

Copy link
Copy Markdown
Contributor Author

Superseded by larger silent-refresh PR

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

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant