Skip to content

Bound BaseContent.time to a reasonable range#150

Merged
odesenfans merged 1 commit into
mainfrom
feat/bound-content-time
Apr 21, 2026
Merged

Bound BaseContent.time to a reasonable range#150
odesenfans merged 1 commit into
mainfrom
feat/bound-content-time

Conversation

@odesenfans
Copy link
Copy Markdown
Contributor

Require time to be non-negative and below year ~2262 (9223372036, comfortably within float precision). Matches the constraints already enforced downstream in pyaleph's pending-message schema.

Test plan

  • hatch -e testing run pytest (unchanged — same 4 pre-existing network-flaky tests fail on main)

Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a clean, minimal change that adds sensible bounds validation to the time field in BaseContent. The lower bound (0) prevents negative timestamps, and the upper bound (~year 2262) is well below the float precision cliff at 2^53. The implementation uses Pydantic's Field correctly, and the comment adequately explains the rationale. Existing tests use timestamps well within these bounds (e.g., time=1.0, time=1625652287.017), so no test failures are expected.

aleph_message/tests/test_models.py (line 100): Consider adding explicit tests for the new time bounds validation. For example: test_time_rejects_negative() to verify negative timestamps raise ValidationError, and test_time_rejects_futuristic() to verify timestamps above MAX_CONTENT_TIME are rejected. This would provide regression protection for the new validation logic.

Require time to be non-negative and below year ~2262 (9223372036, comfortably within float precision). Matches the constraints already enforced downstream in pyaleph's pending-message schema.
@odesenfans odesenfans force-pushed the feat/bound-content-time branch from b2672fa to c830a62 Compare April 21, 2026 21:45
Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a clean, minimal change that adds sensible validation constraints to the time field in BaseContent. The upper bound of 9,223,372,036 (year ~2262) is well below the float precision cliff at 2^53, and the lower bound of 0 prevents negative timestamps. The implementation uses Pydantic's Field constraints correctly, and the inline comment clearly explains the rationale. All existing tests use time values within the valid range (e.g., time=1.0, time=1625652287.017), so they should continue to pass. The PR description accurately notes that test results match main with only pre-existing network-flaky failures.

aleph_message/tests/test_models.py (line 105): Consider adding explicit tests for time boundary validation. For example, a test that verifies PostContent(time=-1, ...) raises a ValidationError, and another that verifies PostContent(time=MAX_CONTENT_TIME + 1, ...) also raises. This would provide explicit coverage for the new validation logic and prevent regressions.

@odesenfans odesenfans merged commit 593f75c into main Apr 21, 2026
10 checks passed
@odesenfans odesenfans deleted the feat/bound-content-time branch April 21, 2026 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants