Skip to content

Add CI + dev deps; fix test nondeterminism; guard artifact/date-kind drift - #5

Merged
XVVH merged 1 commit into
mainfrom
devin/1784300646-review-followup-fixes
Jul 17, 2026
Merged

XVVH merged 1 commit into
mainfrom
devin/1784300646-review-followup-fixes

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to the code review of PRs #1–#4. Addresses the gaps that weren't covered there: test reproducibility, missing CI, and unguarded drift risks. No production/UI code changes — only tests, tooling, and CI.

1. Test nondeterminism (from PR #4 review). test_generation.py seeded per-entry RNGs with the built-in hash(title), which is salted per process (PYTHONHASHSEED), so run_tests(seed=42) was not actually reproducible run-to-run (reproduced: 3 runs of the same "seeded" call gave IMG 1828 / 3307 / 9255). Replaced with a stable digest:

def stable_hash(s: str) -> int:
    return int.from_bytes(hashlib.blake2b(s.encode(), digest_size=8).digest(), "big")
# R2/R3 now: random.Random(... stable_hash(e["title"]) ...)

test_common.py verifies it via subprocess under two different PYTHONHASHSEED values → identical output.

2. DATE_KIND_TO_YMD coverage guard (from PR #2 review). Nothing enforced that DATE_KIND_TO_YMD covers every date kind except year; a future kind added to DATE_KIND_LABELS but not DATE_KIND_TO_YMD would make generate_from silently fall through to emitting the raw title. Added test_common.py:

assert set(DATE_KIND_TO_YMD) == set(DATE_KINDS) - {"year"}
assert set(DATE_KINDS) == set(DATE_KIND_LABELS)

3. Artifact-sync guard (from PR #1 review). data.json, data.min.json, and index.html are generated together but committed as static files, so they can silently drift. Added test_artifacts.py:

  • data.json == data.min.json
  • base64 embedded in index.html decodes back to data.min.json
  • regenerating index.html from the committed data.min.json reproduces it byte-for-byte

4. Dev deps + CI (from PR #1 review). pytest/coverage weren't declared anywhere and there was no committed CI workflow, so the tests weren't wired to run automatically. Added requirements-dev.txt and .github/workflows/ci.yml (Python 3.10–3.12) running python3 test_generation.py and pytest.

Verification

  • python3 -m pytest -q → 84 passed (77 existing + 7 new).
  • python3 test_generation.py → PASS rubric (452 entries, seed=42).
  • Reproducibility: same generate_from output under PYTHONHASHSEED=1 and 999.

Link to Devin session: https://app.devin.ai/sessions/c699ea7702c843db9106e378612ebdcb
Requested by: @XVVH


Open in Devin Review

…drift

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@XVVH
XVVH merged commit 736e3e5 into main Jul 17, 2026
4 checks passed
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.

1 participant