Skip to content

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

Merged
plind-junior merged 3 commits into
vouchdev:testfrom
kai392:fix/critical-issue-clear-naive-datetime
Jul 29, 2026
Merged

fix(lifecycle): date-only --before crashes clear on every surface#580
plind-junior merged 3 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 handling for claim clearing by treating naive inputs as UTC.
    • Resolved timezone comparison issues that could trigger CLI/JSONL/MCP and review-console errors.
    • Updated audit logging to store the normalized UTC cutoff.
    • Prevented the web /clear-claims flow from crashing for date-only before values, correctly archiving eligible claims.
  • Tests

    • Added coverage for naive vs timezone-aware cutoff behavior, including dry-run mode.
    • Added a web regression test for before=YYYY-MM-DD avoiding server errors on load and POST.

`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

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bd697f5f-54c0-4469-a50f-86fc71c7efe5

📥 Commits

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

📒 Files selected for processing (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.

jak-glitch and others added 2 commits July 29, 2026 11:38
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.
@plind-junior
plind-junior merged commit a969fb7 into vouchdev:test Jul 29, 2026
7 of 8 checks passed
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.

3 participants