Skip to content

Stabilize dashboard fixture clock - #2390

Merged
steipete merged 2 commits into
steipete:mainfrom
ProspectOre:fix/dashboard-test-clock
Jul 28, 2026
Merged

Stabilize dashboard fixture clock#2390
steipete merged 2 commits into
steipete:mainfrom
ProspectOre:fix/dashboard-test-clock

Conversation

@ProspectOre

Copy link
Copy Markdown
Contributor

Summary

Stabilize Spend Dashboard controller tests by injecting the same fixed clock used by their fixture snapshots.

Root cause

Several tests used a July 15 fixture while SpendDashboardSource.makeRequest used the real current date. Once the calendar moved beyond the fixture window, valid fixture spend was filtered out and the assertions saw $0.

Change

  • add one fixture-clock controller helper
  • route the eight affected controller constructions through it
  • preserve the production request clock and all dashboard behavior

Verification

  • xcodebuildmcp swift-package test --package-path /Users/alec/Dev/codexbar-dashboard-ci --filter SpendDashboardControllerTests --parallel false --output text — 24 tests across 3 suites passed
  • swiftformat --lint Tests/CodexBarTests/SpendDashboardControllerTests.swift — clean
  • swiftlint --strict — 0 violations
  • make check completes relevant validation and stops only at the repository's pre-existing Node 25 CommonJS/ESM mismatch in docs/site.js

@ProspectOre
ProspectOre marked this pull request as draft July 22, 2026 07:43
@ProspectOre
ProspectOre marked this pull request as ready for review July 22, 2026 07:52
# Conflicts:
#	Tests/CodexBarTests/SpendDashboardControllerTests.swift
@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 28, 2026
@clawsweeper

clawsweeper Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 27, 2026, 11:34 PM ET / July 28, 2026, 03:34 UTC.

ClawSweeper review

What this changes

This PR routes Spend Dashboard controller test setup through a helper that builds requests with the same fixed clock used by the test fixtures.

Merge readiness

Blocked until real behavior proof is added - 4 items remain

Keep this PR open for normal review. The proposed change is a focused test-only stabilization with no evident production or security impact, but the PR body only reports validation results rather than including after-fix terminal output or an artifact from the current head.

Priority: P3
Reviewed head: b7caf5647a13a2886717c1d93a3209ec90242305

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The proposed test-only approach is plausibly well-scoped, but reviewer confidence is capped because the reported verification is not accompanied by inspectable after-fix evidence.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR body states that focused tests passed, but it provides no copied terminal output, linked artifact, recording, or redacted runtime log that demonstrates the changed tests succeeding on the current head; add that evidence before merge and redact any private paths or account data. 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) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body states that focused tests passed, but it provides no copied terminal output, linked artifact, recording, or redacted runtime log that demonstrates the changed tests succeeding on the current head; add that evidence before merge and redact any private paths or account data. 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 Narrow test-only diff: The PR modifies one test file, replacing direct controller constructions with a shared fixture-clock controller helper; the supplied metadata reports 25 additions and 38 deletions.
Claimed focused validation: The PR body reports that the 24-test SpendDashboardControllerTests selection passed, along with formatter and linter checks, but it does not include copied terminal output, a linked log, or another inspectable artifact for the current head.
Recent test-area integration signal: The latest supplied branch commit is an integration commit by steipete resolving this test file after syncing from main, which is the strongest available history signal for routing this review.
Findings None None.
Security None None.

How this fits together

Spend Dashboard controller tests load dated usage fixtures and build dashboard requests that filter spend by time window. The request builder feeds the controller state and assertions, so a fixture-aligned clock keeps those tests deterministic without changing production request timing.

flowchart LR
    A[Dated dashboard fixtures] --> B[Test setup]
    B --> C[Fixture-aligned clock]
    C --> D[Dashboard request builder]
    D --> E[Controller state]
    E --> F[Test assertions]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body states that focused tests passed, but it provides no copied terminal output, linked artifact, recording, or redacted runtime log that demonstrates the changed tests succeeding on the current head; add that evidence before merge and redact any private paths or account data. 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.
  • Resolve merge risk (P1) - The helper is intended to stabilize a calendar-dependent test path, but the review context contains validation claims rather than inspectable after-fix terminal output from the current head.
  • Resolve merge risk (P1) - The repository policy requires focused tests where possible and a broader make test/make check handoff; the reported make check interruption should be accompanied by enough output to separate the claimed pre-existing documentation-tool failure from this change.
  • Complete next step (P2) - The remaining merge gate is contributor-provided after-fix evidence, which a repair lane cannot produce on the contributor's setup.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed test surface 1 file affected; 25 added, 38 removed The patch is tightly scoped to controller-test setup and does not touch production sources, dependencies, or release tooling.
Fixture-clock adoption 8 controller constructions routed through one helper Centralizing the fixture-aligned request builder reduces the chance that individual dated-fixture tests keep using wall-clock time.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Keep production clocks unchanged and land a narrowly scoped fixture-clock helper only after the contributor adds redacted current-head test output showing the affected controller tests pass and the normal validation status is clear.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep production clocks unchanged and land a narrowly scoped fixture-clock helper only after the contributor adds redacted current-head test output showing the affected controller tests pass and the normal validation status is clear.

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

No. The PR describes a clear calendar-dependent fixture failure, but the supplied review evidence does not include a failing or passing terminal transcript that can independently establish the behavior on the current head.

Is this the best way to solve the issue?

Unclear. A fixture-clock helper is a narrow, maintainable approach if it is confined to fixture-backed tests, but the available evidence is insufficient to independently confirm the final helper implementation and its exact test coverage.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P3: This is a contained automated-test determinism improvement with no reported user-facing runtime impact.
  • 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 states that focused tests passed, but it provides no copied terminal output, linked artifact, recording, or redacted runtime log that demonstrates the changed tests succeeding on the current head; add that evidence before merge and redact any private paths or account data. 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

What I checked:

  • Narrow test-only diff: The PR modifies one test file, replacing direct controller constructions with a shared fixture-clock controller helper; the supplied metadata reports 25 additions and 38 deletions. (Tests/CodexBarTests/SpendDashboardControllerTests.swift, b7caf5647a13)
  • Claimed focused validation: The PR body reports that the 24-test SpendDashboardControllerTests selection passed, along with formatter and linter checks, but it does not include copied terminal output, a linked log, or another inspectable artifact for the current head. (b7caf5647a13)
  • Recent test-area integration signal: The latest supplied branch commit is an integration commit by steipete resolving this test file after syncing from main, which is the strongest available history signal for routing this review. (Tests/CodexBarTests/SpendDashboardControllerTests.swift, b7caf5647a13)
  • Related but separate stabilization: The closed unmerged token-provenance PR describes the same calendar-versus-fixture failure mode in a different dashboard test surface and explicitly says it complements this PR, rather than replacing it.

Likely related people:

  • steipete: Authored the supplied integration commit that resolved a conflict in the sole affected dashboard-controller test file. (role: recent area contributor; confidence: low; commits: b7caf5647a13; files: Tests/CodexBarTests/SpendDashboardControllerTests.swift)

Rank-up moves

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

  • Add a redacted terminal transcript from the current head showing the focused SpendDashboardControllerTests run and its passing result.
  • Include the relevant make check/make test outcome or clearly show the unrelated pre-existing documentation-tool failure so the handoff matches repository policy.

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.

@steipete
steipete merged commit e3f9cdc into steipete:main Jul 28, 2026
9 checks passed
@steipete

Copy link
Copy Markdown
Owner

Final maintainer verification completed on head b7caf5647a13a2886717c1d93a3209ec90242305.

  • Merged current main into the contributor branch and resolved the single dashboard-test overlap by keeping the fixed-clock helper together with the newly isolated test UserDefaults.
  • Re-read the complete exact-head diff and the affected dashboard request/test path; the final PR remains test-only and limited to fixture-clock stabilization.
  • Independent autoreview completed cleanly with no actionable findings.
  • Fresh exact-head hosted CI was fully green, including both macOS test shards and the aggregate gate: https://github.com/steipete/CodexBar/actions/runs/30326119467

Merged as e3f9cdc26a3aeb7d7327ee680ab05d9f1d5dadda. Thank you, @ProspectOre, for pinning this fixture to a deterministic clock.

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

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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.

2 participants