fix(backend): stop recommending a suggestion that already expired

Merged
IG
2mo ago
+161-8

What changed and why

#11974 gave a pending Candidate a real deadline — two days, stored as expires_at — and candidate_has_lapsed states the rule in its own docstring: "Every read path must ask this rather than trusting status." One read path asks. routers/candidates.py drops a lapsed Candidate from the Suggested surface; What Matters Now does not.

_build_subjects reads canonical state as raw documents and passes unexpired=True for every candidate subject. unexpired is one of the six recommendation gates, so hardcoding it repeals the deadline on that surface: a proposal the user was never shown, because it aged out unseen, is still shortlisted, judged, ranked and rendered as a live recommendation — and accepting it writes the task the Suggested surface had already retired. The two surfaces disagree about the same row on the same day.

A lapsed candidate's eligibility is also part of the projection's material version, and before this change the lapse itself was never what moved it. Its material_token is status:created_at, and unexpired was the constant True, so expiry could not register as a change. Other fields in the same hash do move on their own — facts.days_to_due and the due-window gates advance daily for a candidate that has a due date — so the projection is not necessarily frozen; it is simply never re-judged because the suggestion expired. For a candidate with no due date and no other account activity, that does mean the cached projection matches on refresh and is re-published unchanged, carrying the expired suggestion's card past its own deadline. Computing unexpired honestly makes the lapse a material change in its own right, so the projection is re-judged without it in every case rather than incidentally.

The backlog makes this the common case rather than a rare one. Candidates written before the deadline existed carry no expires_at at all — 1,014 of them in the measurement recorded on INV-TASK-2, with no backfill — and candidate_has_lapsed is what derives their deadline from created_at. A reader that never calls it treats that entire graveyard as permanently eligible, and load_canonical_product_state reads candidates with a flat limit(200), so old rows also compete for the budget the live ones need.

  • unexpired for a candidate subject now answers the question the field exists to ask. The snapshot subject three blocks down already computes it this way (unexpired=snapshot.expires_at > now).
  • The deadline moves behind one predicate with two entry points: candidate_has_lapsed for a parsed record, stored_candidate_has_lapsed for a reader that holds the stored document and never parses it. A rule that lives in a docstring is a rule the next reader ships without.

Out of scope, stated rather than silently left: the limit(200) read is still unordered and unfiltered, so a large enough lapsed backlog can still crowd live suggestions out of the page before any gate runs. That needs a query change plus an index, not a gate fix.

Product invariants affected

INV-TASK-2 — "a Candidate nobody acts on expires rather than accumulating", and the MUST NOT clause "Admit a proposal the Suggested surface will not show." This is that clause applied to a second surface: the suggestion had already been retired, and the recommender put it back in front of the user.

How it was verified

Verified by executing the reader against upstream/main at 02c48d7da6, not by reading it. Three pending Candidates — one a day past its stored expires_at, one from the pre-deadline backlog (no expires_at, created 90 days ago), one created an hour ago — driven through _build_subjects and filter_shortlist:

# before
cand-lapsed            open=True unexpired=True gates=True
cand-legacy-lapsed     open=True unexpired=True gates=True
cand-live              open=True unexpired=True gates=True
SHORTLIST: ['cand-lapsed', 'cand-legacy-lapsed', 'cand-live']

# after
cand-lapsed            open=True unexpired=False gates=False
cand-legacy-lapsed     open=True unexpired=False gates=False
cand-live              open=True unexpired=True  gates=True
SHORTLIST: ['cand-live']

The committed regression test fails on main with:

>           assert not by_id[lapsed].eligibility.unexpired, lapsed
E           AssertionError: candidate-lapsed
E           assert not True
E            +  where True = ShortlistEligibility(open=True, unexpired=True, passes_recommendation_gates=True, ...).unexpired
  • python -m pytest tests/unit/test_task_recommendations.py — 43 passed (42 before, +1 new).
  • python -m pytest tests/unit/test_candidate_lifecycle.py — 51 passed (49 before, +2 parametrized cases).
  • Candidate, staged-task, rollout and What-Matters-Now suites together — test_backend_candidate_capture.py, test_candidate_lifecycle.py, test_candidates_router.py, test_candidates_skip_malformed.py, test_conversation_suggestion_visibility.py, test_get_candidate_skip_malformed.py, test_run_dev_candidate_acceptance.py, test_smoke_what_matters_now.py, test_staged_candidate_migration.py, test_staged_tasks_batch_scores.py, test_staged_tasks_dedup.py, test_staged_tasks_review_controls.py, test_task_intelligence_attribution.py, test_task_intelligence_contract_freeze.py, test_task_intelligence_known_gaps.py, test_task_intelligence_rollout.py, test_task_recommendations.py, test_what_matters_now_smoke_fixture.py, test_working_memory_candidate_schema.py — 329 passed.
  • .github/scripts/check_task_capture_authority.py — INV-TASK-2 holds (automatic capture proposes only).
  • black --check on the four changed Python files — clean.

Not verified against a live Firestore project or a live LLM judge: the reader is exercised through the state dict load_canonical_product_state returns, and the parity test through the repo's strict Firestore transaction fake.

Tests

  • backend/tests/unit/test_task_recommendations.py::test_a_lapsed_suggestion_is_not_recommended_after_the_suggested_surface_drops_it — the regression test, red before the change with the assertion quoted above. It covers both the stored-expires_at case and the pre-deadline backlog case, and pins that a live suggestion is still shortlisted, so the fix cannot turn into "stop recommending candidates".
  • backend/tests/unit/test_candidate_lifecycle.py::test_the_stored_document_answers_the_deadline_exactly_like_the_record — the guard surface for the class below: the record-shaped and document-shaped entry points must agree at the second before the deadline, at the deadline, and long after it, with and without a stored expires_at, and neither may call a resolved Candidate lapsed. It caught a real defect while being written — the stored document can carry an enum where Firestore returns a string, and the twin answered False where the record answered True.

Failure class (fixes)

Failure-Class: new