Skip to content

Suggest logic review: fix accidental correctness, add perceived randomness - #572

Merged
marcdel merged 9 commits into
mainfrom
suggest-logic-cleanup
Jun 11, 2026
Merged

Suggest logic review: fix accidental correctness, add perceived randomness#572
marcdel merged 9 commits into
mainfrom
suggest-logic-cleanup

Conversation

@marcdel

@marcdel marcdel commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Review of the suggest logic turned up a few things that were only working by accident, plus two perceived-randomness problems.

Bug fixes:

  • do_assign_match was throwing away the first pear's assignment in the both-available branch. It self-healed because secondary_matches generated both orderings of every pair, so the reversed duplicate re-placed the dropped pear later. Fixed and pinned with a test, then deduped the pairs.
  • Capacity for untitled tracks was counted before the reset, so a 3-pear track left someone benched. Now counted after.
  • Team.anchors/1 returned tracks without anchors, and the restore worked by toggling nil. Replaced with unanchored_track_names and an honest clear_anchors.

Behavior changes (perceived randomness):

  • Anchor choice is now weighted by 1/(tenure + 1) instead of uniform random, so the same pear stops anchoring a track for days running.
  • Equal-scored matches are shuffled instead of inheriting alphabetical map order, so identical boards no longer produce the identical suggestion and early-alphabet pears stop winning every tie.

Structural:

  • Suggest's untitled tracks are now created in the domain and persisted in one pass, with a single broadcast at the end instead of N+1 (boards were flashing the empty tracks mid-suggest). A failed insert now aborts instead of silently stranding pears. This absorbs the smallest-unused-number naming fix from the whimsy branch, so Make whimsy mode actually whimsical: confetti, sparkles, and a suggest drumroll #571's version of that change will be redundant once this merges.
  • Added MatchValidator unit tests and a StreamData property over generated boards: everyone seated, unlocked tracks max 2, locked tracks untouched, user anchors preserved.
  • Wrote down the scoring policy (recency-only, exact pairs) and the mob-day rule from 8fcf57a, and collapsed the two-phase match ordering into one composite-key sort.

Heads up: #571 will pick up small conflicts in lib/pears.ex and test/pears_test.exs after this merges. Resolution is to take main's side for the suggest bits (the fix lives in the Recommendator now).

🤖 Generated with Claude Code

marcdel and others added 9 commits June 11, 2026 05:49
@marcdel @claude
Fix both-available match assignments discarding the first pear
5732b8e 
The both-pears-available branch of do_assign_match threw away the team
returned by the first add_pear_to_track, so only the second pear was
assigned. The greedy loop self-healed by accident because
secondary_matches generates both orderings of every pair, letting the
reversed duplicate re-place the dropped pear later. Promote
assign_match/2 to the public API and pin the contract with a
regression test so the upcoming pair dedup can't reintroduce the bug.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Generate each available pair once in secondary_matches
232297e 
Scoring is symmetric, so both orderings of a pair always scored the
same and doubled the scoring and assignment work. The reversed
duplicates were also load-bearing for the discarded-add bug fixed in
the previous commit; with that contract pinned by a test, they can go.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Replace the toggle-nil anchor restore with explicit clear_anchors
809eb3d 
Team.anchors/1 actually returned tracks *without* anchors as
{nil, track_name} tuples, and remove_added_anchors cleared the
auto-chosen anchors by calling toggle_anchor(team, nil, track) —
correct only because nil never matched the current anchor. Snapshot
the unanchored tracks before choosing instead, and clear exactly
those afterwards via Team.clear_anchors/Track.clear_anchor.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Weight anchor choice by inverse track tenure
b8da3f4 
Uniform Enum.random let the same pear anchor a track for days running
(25% chance of a 3-day streak on a 2-pear track), which users read as
the tool playing favorites. Weight each pear by 1/(tenure + 1), where
tenure is the consecutive most-recent recorded days on that track, so
long-tenured pears become progressively less likely to stay behind
while the choice stays unpredictable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Shuffle equally-scored matches before greedy assignment
d7c8cf3 
Tie order used to come from map iteration, which is alphabetical for
team-sized maps: identical boards always produced the identical
suggestion and alphabetically-early pears systematically won ties.
Shuffling before the stable score sort randomizes order within each
score group without disturbing the recency ordering itself.

Three tests asserted one exact board among several equally-valid
outcomes; they now assert the intent (no yesterday repeats, locked
tracks untouched, everyone seated) instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Create suggest's untitled tracks in the domain and broadcast once
0db9cf4 
add_empty_tracks went through the public Pears.add_track per track, so
one suggest click persisted, session-updated, and broadcast N+1 times
— every connected board flashed the empty untitled tracks before the
final state — and a failed insert was silently swallowed, stranding
pears on the bench. The Recommendator now adds overflow tracks
in-memory and recommend_pears persists them in one pass, aborting
before the session or subscribers see anything if an insert fails.

Capacity is also now counted after the reset instead of before it,
which fixes under-provisioning on tracks with three or more pears: a
3-pear track frees two pears but exposes only one slot, so the
pre-reset count left someone benched.

Track ids for domain-created tracks are now max+1 rather than count+1
so they can't collide with database-sourced ids on the same board.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Add MatchValidator tests and suggest invariant properties
913462c 
MatchValidator was only exercised indirectly; pin its contract with
unit tests and normalize valid?/2 to return booleans instead of a
truthy Track struct. Add StreamData and a property over generated
boards (pears, tracks, placements, locks, anchors, history) asserting
the suggest invariants: every pear seated, unlocked tracks capped at a
pair, locked tracks untouched, user anchors preserved and auto-chosen
anchors cleared. The seating invariant verifiably catches the removal
of overflow-track creation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Document the scoring policy and collapse match ordering into one sort
cbcdecc 
The match ordering was built in two phases — a per-list sort inside
score_matches preserved through split_with buckets — which made the
policy hard to read. One composite-key sort on {never-paired?,
primary?, recency} now states it in one place, with the tie shuffle
intact. Write down the two implicit policy decisions: scoring is
recency-only (frequency deliberately ignored, exact pairs only), and
matched_on_day?'s count < 4 guard is the mob-day rule from 8fcf57a.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel @claude
Add stream_data to formatter import_deps and format
c34a9b9 
Keeps check all/gen all in their paren-free macro form and fixes a
long pipe the earlier format check missed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcdel
marcdel force-pushed the suggest-logic-cleanup branch from b4bfa2c to c34a9b9 Compare June 11, 2026 15:52
@marcdel
marcdel changed the base branch from whimsy-mode-celebrations to main June 11, 2026 15:52
@marcdel
marcdel merged commit 956fde9 into main Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@marcdel