Skip to content

feat(s-remind2): a distinct second (final) reminder#298

Merged
CoJoA13 merged 6 commits into
mainfrom
feat/s-remind2
Jun 26, 2026
Merged

feat(s-remind2): a distinct second (final) reminder#298
CoJoA13 merged 6 commits into
mainfrom
feat/s-remind2

Conversation

@CoJoA13

@CoJoA13 CoJoA13 commented Jun 26, 2026

Copy link
Copy Markdown
Owner

S-remind2 — a distinct second (final) reminder

Re-introduces the second pre-due reminder that S-notify-4 deferred. The original was killed
because both reminders emitted under task.due_soon, and the in-app dedup
((recipient_user_id, task_id, event_key) WHERE task_id IS NOT NULL) collapsed REMIND_2 onto
REMIND_1's row → it delivered nothing. The fix: a distinct event key for REMIND_2.

What ships

  • task.due_final — a new event key for REMIND_2 (constants + _TASK_EVENT_VARS whitelist +
    ACTION_REQUIRED class — not CRITICAL, so it does not pierce quiet hours like task.overdue).
  • escalation.py — the REMIND branch now selects the key per step: REMIND_1 → task.due_soon
    (unchanged), REMIND_2 → task.due_final. OVERDUE/ESCALATE and all stamping/audit/recipient logic
    are byte-identical.
  • Migration 0068 (no DDL) — seeds the global task.due_final template (ON CONFLICT … DO NOTHING) and sets sla_policy.remind_2_before = INTERVAL '1 day' (a final reminder 1 business
    day before due
    , all task types). Downgrade nulls remind_2_before and deletes the template
    guarded against the notification.template_id RESTRICT FK.

_due_task_ids and due_steps need no change — S-claim-filter pre-activated the policy-gated
remind_2 claim disjunct, and the timer already emits REMIND_2; this slice just makes the offset
non-NULL and gives the emit a distinct key.

Scope

BE-only. Migration head 0067 → 0068. No new permission key, no WORM/authz/web/openapi/FE
change. The SPA renders the stored title/body + the notification class — it doesn't switch on
event_key, so a new key with the same class renders identically.

Tests

  • Unit: class_of("task.due_final") == ACTION_REQUIRED; the due_steps REMIND_2 threshold is
    already covered.
  • Integration (mutation-distinguishing): test_remind_2_distinct_final_reminder — one sweep
    produces two distinct notifications (task.due_soon + task.due_final) for the assignee
    (reminders isolated by pre-stamping overdue+escalate); under the old shared-key path the second
    deduped to one. Run-scoped, open-state, depends on 0068 setting remind_2_before on the
    testcontainer DB.
  • Enabling remind_2 made the two S-claim-filter exclusion tests' "fully-fired" tasks newly
    claimable (the now-live remind_2 disjunct) → both now stamp remind_2_sent_at; the DOC_ACK case
    stays mutation-distinguishing via the still-live escalate tautology. The two additive-safe
    escalation tests also pre-stamp remind_2 to keep isolating their escalate checks.

Gates

Local: ruff clean · mypy src (412 files) · pytest -m unit 1063 passed. The migration
round-trip (CI migrations) and the two-notification proof + seed-data effect (CI integration)
are authoritative (Docker-less dev box).

Review

Subagent-driven (per-task spec+quality review; migration via the specialized migration-reviewer) →
final whole-branch: opus senior reviewer (Ready=Yes) + diff-critic (CLEAN) converged, with a
full 24-test audit confirming no other timer test breaks or false-passes. The Minors they raised
(a now-stale "remind_2 is inert" prod docstring + test comment/message drift) are folded in.

🤖 Generated with Claude Code

CoJoA13 and others added 6 commits June 26, 2026 10:23
Re-introduce the deferred 2nd pre-due reminder (killed in S-notify-4 by the
shared task.due_soon dedup) under a distinct event key task.due_final, fired
1 business day before due. BE-only; migration 0068 (template seed + sla_policy
remind_2_before UPDATE); _due_task_ids already future-proofed by S-claim-filter.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add EVENT_TASK_DUE_FINAL + its variable whitelist + ACTION_REQUIRED class
mapping, and split the timer-sweep REMIND branch so REMIND_2 emits under
task.due_final (dodging the (recipient, task_id, event_key) dedup that
collapsed the second reminder). due_steps/_due_task_ids unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…le remind_2

Seed the global task.due_final notification template and set
sla_policy.remind_2_before = 1 day (all task types). No DDL (alembic check
clean); idempotent INSERT/UPDATE; downgrade nulls remind_2_before and deletes
the template guarded against the notification.template_id RESTRICT FK.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_2 tests

Add the mutation-distinguishing integration proof (one sweep → distinct
task.due_soon + task.due_final). Stamp remind_2_sent_at in the two S-claim-filter
exclusion tests (remind_2 is now a configured step; the DOC_ACK case stays
RED-on-old via the escalate tautology) and fix test_remind_fires_once's stale
'remind_2_before is NULL' docstring.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e test isolation

Final-review Minors: the _due_task_ids docstring + several test comments/messages
described remind_2 as inert/unconfigured, false after 0068 enabled it. Reword to
reflect the now-live remind_2 claim path, and add remind_2_sent_at=_STAMPED to the
two escalation tests so they truly isolate the escalate check (no spurious due_final).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CoJoA13 CoJoA13 enabled auto-merge (squash) June 26, 2026 16:45
@CoJoA13 CoJoA13 disabled auto-merge June 26, 2026 16:47
@CoJoA13 CoJoA13 merged commit e5b0d13 into main Jun 26, 2026
11 checks passed
@CoJoA13 CoJoA13 deleted the feat/s-remind2 branch June 26, 2026 17:10
CoJoA13 added a commit that referenced this pull request Jun 26, 2026
…less framing (#299)

Record the S-remind2 slice (distinct task.due_final 2nd reminder): slice-history
narrative + CLAUDE.md Recent-learnings bullet (dropping the oldest S-notify-5a to
hold the ~8 cap) + Current-status pointer + migration head 0067→0068. Also correct
the now-wrong 'Docker-less box' framing in docs/dev-workflow.md: Docker IS available
on this Linux box via 'sg docker -c' (the agent user is in the docker group), which
runs -m integration / /check-migrations / a real-stack migration live-smoke locally.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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