Skip to content

fix: preserve packet_feedback and created_at on packet re-save - #88

Open
clates wants to merge 4 commits into
mainfrom
fix/packet-feedback-preservation
Open

fix: preserve packet_feedback and created_at on packet re-save#88
clates wants to merge 4 commits into
mainfrom
fix/packet-feedback-preservation

Conversation

@clates

@clates clates commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • Bug A (data loss): INSERT OR REPLACE in _insert_weekly_packet triggers a SQLite-internal DELETE + INSERT on primary key conflict. With FOREIGN KEY ... ON DELETE CASCADE on packet_feedback, this silently deletes all user-submitted feedback on every packet re-save. Since generate_trio_for_student is called as a background task after every feedback submission, feedback was being erased immediately after submission.
  • Bug B (audit timestamp): Both created_at and updated_at were set to now on every call, overwriting the original creation timestamp on re-saves.

Changes

  • src/packet_store.py — Replace INSERT OR REPLACE INTO weekly_packets with INSERT INTO ... ON CONFLICT(packet_id) DO UPDATE SET .... The upsert updates the row in-place without deleting it, so the ON DELETE CASCADE to packet_feedback never fires. created_at is intentionally excluded from the DO UPDATE SET list so it is only written on the first insert.
  • src/packet_store.py — Add conn.execute("DELETE FROM daily_lessons WHERE packet_id = ?", ...) in save_weekly_packet before _persist_daily_lessons. This explicit delete replaces the implicit cascade cleanup that the old DELETE approach provided, since _persist_daily_lessons uses plain INSERT (not INSERT OR IGNORE) and would fail on UNIQUE constraint for (packet_id, day_label) without it.
  • tests/test_packet_store.py — Add test_resave_preserves_packet_feedback: saves a packet, saves feedback, re-saves the packet, asserts feedback is still present. Add test_resave_does_not_overwrite_created_at: saves a packet, re-saves it, asserts created_at is unchanged.

Test plan

  • test_resave_preserves_packet_feedback — confirms feedback rows survive a packet re-save
  • test_resave_does_not_overwrite_created_at — confirms created_at is immutable after first insert
  • Existing test_save_weekly_packet_persists_rows — confirms initial save still writes all rows correctly
  • Existing test_save_weekly_packet_rolls_back_on_error — confirms transaction atomicity is preserved
  • CI runs on Python 3.12 where datetime.UTC is available (local env is 3.10; import error is pre-existing)

🤖 Generated with Claude Code

clates and others added 4 commits June 16, 2026 19:22
… fp precision, cascade deletes

- Add null guards for progress_blob/plan_rules_blob in logic.py, main.py, agent.py
  (issues #77, #78, #79) — new students with NULL blobs no longer crash
- Fix generate_weekly_plan to use datetime.now(UTC) instead of local time
  (issue #80) — prevents off-by-one day in week_of on non-UTC servers
- Fix _get_grade_level to fall back to student metadata instead of literal 0
  (issue #81) — first-time plan generation no longer fails with no-standards error
- Round activity_bias in process/reverse_quantity_feedback to 6 decimal places
  (issue #82) — prevents floating-point drift across feedback cycles
- Replace delete-then-insert with INSERT OR REPLACE in save_weekly_packet
  (issue #83) — eliminates duplicate-packet race condition under concurrent saves
- Cascade-delete weekly_packets before deleting student profile in delete_student
  (issue #84) — packet_feedback no longer left as orphans after student deletion

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Adds two rules to prevent direct commits to main:
- All changes must go through a PR, no exceptions
- Automated analysis/fix workflows must create a branch before making changes,
  then open a PR rather than committing directly

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…l test

- delete_student() now catches OperationalError when weekly_packets table
  doesn't exist (minimal test DBs only have student_profiles)
- Update test_get_grade_level_defaults_to_zero to expect 1 (the new safe
  default) and add a second test covering the metadata_fallback path

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Replace INSERT OR REPLACE with INSERT ... ON CONFLICT(packet_id) DO
  UPDATE SET in _insert_weekly_packet. SQLite's INSERT OR REPLACE
  internally does DELETE + INSERT, which cascades to packet_feedback and
  silently deletes all user feedback on every background re-generation.
  The new upsert updates the row in-place so no cascade fires.
- Exclude created_at from the DO UPDATE SET list so the original
  creation timestamp is only written on the first insert and never
  overwritten on subsequent saves.
- Add explicit DELETE FROM daily_lessons WHERE packet_id = ? in
  save_weekly_packet before calling _persist_daily_lessons, replacing the
  implicit cascade cleanup that the old DELETE approach provided.
- Add regression tests: test_resave_preserves_packet_feedback and
  test_resave_does_not_overwrite_created_at.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Base automatically changed from fix/claude-identified-bugs to main July 9, 2026 04:52
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