Skip to content

fix(lifecycle): date-only --before crashes clear on every surface - #580

Open
kai392 wants to merge 2 commits into
vouchdev:testfrom
kai392:fix/critical-issue-clear-naive-datetime
Open

fix(lifecycle): date-only --before crashes clear on every surface#580
kai392 wants to merge 2 commits into
vouchdev:testfrom
kai392:fix/critical-issue-clear-naive-datetime

Conversation

@kai392

@kai392 kai392 commented Jul 29, 2026

Copy link
Copy Markdown

What changed

lifecycle.clear_claims normalises its before cutoff to utc before
comparing it against a claim's created_at, using the same _utc helper
stats.py, proposals.py, and digest.py already use. one chokepoint, so
the cli, the mcp tool, the jsonl handler, and the review console are all
fixed without touching a transport. the claim.bulk_clear audit event now
records the normalised cutoff instead of whatever shape the caller passed.

Why

the comparison was claim.created_at >= before. created_at is always
tz-aware; a date-only cutoff parses naive, and python refuses to order the
two — so the call raised TypeError: can't compare offset-naive and offset-aware datetimes before filtering anything.

2026-07-01 is not an unusual input, it is the exact shape the code itself
advertises: the cli's invalid date format message, the console's inline
use ISO 8601, e.g. 2026-07-01 error, and the kb.clear docs all name it. so
the crash sits on the documented happy path of every surface:

  • vouch claims-clear --before 2026-07-01 → traceback
  • kb.clear over mcp / jsonl → error response, nothing cleared
  • GET|POST /clear-claims?before=2026-07-01 → unhandled 500; the view only
    catches HTTPException, so the TypeError escapes both handlers

repro on test before the fix:

>>> life.clear_claims(store, auto_only=False,
...                   before=datetime.fromisoformat("2026-07-01"), actor="me")
TypeError: can't compare offset-naive and offset-aware datetimes

What might break

nothing on disk. no file moves, no field changes shape, no kb.* signature
changes. an aware before keeps its exact current behaviour — _utc is a
no-op on anything already tz-aware, so existing callers and the current
tests are unaffected. the only behaviour change is that a naive cutoff now
filters (read as utc) instead of raising, and the audit event's before
field is written normalised.

VEP

not a surface change — no vep.

Tests

  • make check passes locally (lint + mypy + pytest)
  • New / changed behaviour has a test
  • CHANGELOG.md updated under ## [Unreleased]

two regressions, both failing on the previous code with the TypeError
above:

  • tests/test_lifecycle.py::test_clear_claims_reads_a_naive_before_as_utc
    the unit case, plus the audit event's normalised cutoff. filed in a new
    tests/test_lifecycle.py rather than tests/test_clear_claims.py because
    that module's autouse fixture importorskips numpy, which the base ci job
    (pip install -e '.[dev,web]') does not install — the whole file is
    skipped there, so a regression parked in it would never run in ci.
  • tests/test_web.py::test_clear_date_only_before_does_not_500 — the console
    path, asserting 200 on the preview and 303 + archived on the apply.

validation run:

python -m ruff check src tests
python -m mypy src
python -m pytest tests/ -q --ignore=tests/embeddings

no ui files are touched (web/, src/vouch/web/, webapp/ are all
untouched), only tests/test_web.py, so there is no screenshot to add.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed date-only --before parsing for claim clearing by treating naive inputs as UTC.
    • Resolved timezone comparison errors that could surface as CLI/JSONL/MCP and review console failures.
    • Updated audit logging to record the normalized UTC cutoff.
    • Ensured the web /clear-claims flow no longer crashes for date-only before values.
  • Tests

    • Added lifecycle tests covering naive vs timezone-aware cutoff handling and dry-run behavior.
    • Added a web UI regression test for non-500 clearing with before=YYYY-MM-DD.

`clear_claims` compared a caller-supplied `before` straight against a
claim's `created_at`. a date-only cutoff parses naive, `created_at` is
always aware, and python refuses to order the two — so the documented
shape raised `TypeError` instead of filtering.

`2026-07-01` is exactly what the cli help, the console's inline error
text, and the kb.clear docs advertise, so the crash sits on the happy
path of all four surfaces: a traceback out of `vouch claims-clear
--before`, an error response over mcp and jsonl, and an unhandled 500 on
the review console's `/clear-claims` (the handler only catches
`HTTPException`).

normalised once at the `lifecycle.clear_claims` chokepoint with the same
`_utc` helper stats.py, proposals.py, and digest.py already use, so every
surface is fixed without touching a transport. an aware cutoff keeps its
existing behaviour; the audit event now records the normalised cutoff so
the logged value is unambiguous.

the regression lives in a new tests/test_lifecycle.py rather than
tests/test_clear_claims.py — that module skips without numpy, which the
base ci job does not install.
@kai392
kai392 requested a review from plind-junior as a code owner July 29, 2026 03:29
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fc6e9665-84ef-484d-a76d-4b3ddadf3565

📥 Commits

Reviewing files that changed from the base of the PR and between 2956937 and a90dff0.

📒 Files selected for processing (1)
  • CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

Walkthrough

clear_claims now normalizes naive and timezone-aware datetimes to UTC before filtering claims and recording audit data. Lifecycle and web tests cover cutoff handling, dry runs, audit serialization, and date-only review-console filters.

Changes

Claim clear UTC handling

Layer / File(s) Summary
Normalize clear claim timestamps
src/vouch/lifecycle.py, CHANGELOG.md
Adds UTC normalization for cutoff and claim timestamps, serializes the normalized audit cutoff, documents the behavior, and records the fix in the changelog.
Validate lifecycle and web clearing
tests/test_lifecycle.py, tests/test_web.py
Tests naive and aware cutoffs, claim selection, archival status, audit output, dry-run behavior, and date-only /clear-claims requests.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: plind-junior

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the lifecycle fix for date-only --before cutoffs causing clear failures across surfaces.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance review-ui browser review ui storage kb storage, migrations, schemas, and proposals tests tests and fixtures size: S 50-199 changed non-doc lines labels Jul 29, 2026
@kai392

kai392 commented Jul 29, 2026

Copy link
Copy Markdown
Author

the ci run here is sitting on action_required (first fork pr from this account), so for what it is worth i ran the same workflow on the fork against this exact commit before opening: lint, mypy src, the 3.11/3.12/3.13 pytest matrix, and the sdist + wheel build all green — https://github.com/kai392/vouch/actions/runs/30419500894 (1824 passed, 38 skipped). happy to rebase if test moves under me.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 297-304: Move the described clear-claims timezone normalization
bullet from the released 1.4.0 section into the [Unreleased] section of
CHANGELOG.md, preserving its wording and formatting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: adf93c2d-04d2-4e14-aebb-fb0784cd04ec

📥 Commits

Reviewing files that changed from the base of the PR and between 1fe8fb5 and 2956937.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/vouch/lifecycle.py
  • tests/test_lifecycle.py
  • tests/test_web.py

Comment thread CHANGELOG.md Outdated
@kai392

kai392 commented Jul 29, 2026

Copy link
Copy Markdown
Author

good catch, fixed in a90dff0. the bullet was sitting in the released 1.4.0 block — i checked the next ## header below the entry (1.3.0) and never the one above it (1.4.0 at line 238), so it read as [Unreleased] to me and was not. [Unreleased] had no ### Fixed section, so the move adds one after ### Changed. 1.4.0 is untouched — the diff is now a single additive hunk.

@kai392

kai392 commented Jul 29, 2026

Copy link
Copy Markdown
Author

coderabbit approved a90dff0. the coderabbit-approved status is still showing pending for a mechanical reason, not a review one: the gate run that would publish it fired on pull_request_review, and every pull_request/pull_request_review run on this pr is parked at action_required (first-time fork contributor), so it never got to the status step. ci is parked the same way. the pull_request_target gates — coderabbit-gate on synchronize, ui-screenshot-gate, trust-gate, labeler, koth — all ran and passed.

so this needs one "approve and run workflows" click to go green; nothing on my side is outstanding. the fork run of the same workflow on the identical tree was green end to end (lint, mypy src, 3.11/3.12/3.13, sdist + wheel): https://github.com/kai392/vouch/actions/runs/30419500894. the only commit since is the changelog move, which touches no code.

it landed in the released 1.4.0 block — i checked the next ## header
below the entry and not the one above it, so the bullet would have been
missing from the next release notes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs documentation, specs, examples, and repo guidance review-ui browser review ui size: S 50-199 changed non-doc lines storage kb storage, migrations, schemas, and proposals tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants