Skip to content

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

Merged
kodjima33 merged 1 commit into
BasedHardware:mainfrom
igor-popov-dev:fix/wmn-shortlist-honors-suggestion-ttl
Aug 24, 2026
Merged

kodjima33 merged 1 commit into
BasedHardware:mainfrom
igor-popov-dev:fix/wmn-shortlist-honors-suggestion-ttl

Conversation

@igor-popov-dev

@igor-popov-dev igor-popov-dev commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor

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

Review in cubic

BasedHardware#11974 gave a pending Candidate a real deadline: two days, stored as
`expires_at`, after which the Suggested surface stops showing it. Exactly one
reader enforces it. `candidate_has_lapsed` says in its own docstring that
"every read path must ask this rather than trusting `status`", and only
`routers/candidates.py` does.

What Matters Now does not. `_build_subjects` reads canonical state as raw
documents and hardcodes `unexpired=True` for every candidate subject, so the
deadline is repealed on that surface: a proposal the user was never shown
because it aged out is judged, ranked, and rendered as a live recommendation,
and accepting it writes the task the Suggested surface had already retired.
The backlog makes it worse rather than rarer -- pre-deadline Candidates carry
no `expires_at` at all (1,014 of them, measured in the INV-TASK-2 record, with
no backfill), so months-old proposals stay permanently eligible here.

- `unexpired` for a candidate subject now answers the real question. The field
  already exists for this; snapshots at line 584 compute it the same way.
- The deadline moves into one predicate with two entry points:
  `candidate_has_lapsed` for a parsed record, `stored_candidate_has_lapsed`
  for a reader holding the stored document. A rule stated in a docstring is a
  rule the next reader ships without.

Verified by executing the reader, not by reading it: a Candidate one day past
its `expires_at`, and one from the pre-deadline backlog, both reach
`filter_shortlist` on main and neither does after the fix.

Failure-Class: new

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Backend bug fix: What Matters Now recommender hardcoded unexpired=True for candidate subjects, re-surfacing lapsed suggestions the Suggested surface already dropped. Regression tests demonstrated failing before/passing after. 5/5.

@kodjima33
kodjima33 merged commit c31c74e into BasedHardware:main Aug 24, 2026
34 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.

2 participants