[Stack 6/17] Fix D2: in-conv participant threshold + D2c vote count source#2513
Open
jucor wants to merge 1 commit intospr/edge/bdc830dbfrom
Open
[Stack 6/17] Fix D2: in-conv participant threshold + D2c vote count source#2513jucor wants to merge 1 commit intospr/edge/bdc830dbfrom
jucor wants to merge 1 commit intospr/edge/bdc830dbfrom
Conversation
This was referenced Mar 30, 2026
Open
Open
jucor
changed the title
jucor
force-pushed
the
spr/edge/c0a682ec
branch
2 times, most recently
from
43304ef to
02284b0
Compare
## Summary
Fixes the in-conv participant threshold (D2), vote count source (D2c), and base-cluster sort order (D2b) to match Clojure. Adds monotonicity guard tests (D2d).
### D2: In-conv threshold
- **Before**: `threshold = 7 + sqrt(n_cmts) * 0.1` — increasingly restrictive for larger conversations (e.g., 8.8 for biodiversity's 314 comments)
- **After**: `threshold = min(7, n_cmts)` — matches Clojure exactly
### D2b: Base-cluster sort order (from Copilot review)
- **Before**: Base clusters sorted by size (descending) with IDs reassigned — changes encounter order of centers fed into group-level k-means
- **After**: Keep k-means ID order, matching Clojure's `(sort-by :id ...)`
### D2c: Vote count source (raw vs filtered matrix)
- **Before**: `_compute_user_vote_counts` and `n_cmts` used `self.rating_mat` (filtered — moderated-out comment columns removed). A participant who voted on 8 comments could drop to 5 visible votes after 3 comments were moderated-out, falling below threshold.
- **After**: Both use `self.raw_rating_mat` (includes all votes, even on moderated-out comments), matching Clojure's `user-vote-counts` (conversation.clj:217-225) which reads from `raw-rating-mat`.
### D2d: In-conv monotonicity (design decision)
Python does full recompute from `raw_rating_mat` every time, so monotonicity ("once in, always in") is guaranteed without persistence — votes are immutable in PostgreSQL, so a participant's count never decreases. This is **strictly better** than Clojure's approach (which persists in-conv to `math_main` because it uses delta vote processing).
5 guard tests (T1-T5) document this invariant and warn that switching to delta processing would require persisting in-conv to DynamoDB (ref: #2358).
### Impact
- biodiversity: 428 → 441 in-conv participants (now matches Clojure)
- Verified on 4 datasets with complete Clojure cold-start blobs
### Incremental vs cold-start blob testing
D2 tests run against both **cold-start** and **incremental** Clojure blobs (infrastructure from #2420):
- **Cold-start blobs** are computed in one pass on the full dataset. The in-conv threshold `min(7, n_cmts)` is evaluated once with the final `n_cmts`. Python matches these exactly.
- **Incremental blobs** were built progressively as votes trickled in over the conversation's lifetime. The threshold was evaluated at each iteration with a smaller `n_cmts`, admitting a few extra participants during earlier iterations. The difference is tiny (1–2 participants).
D2 tests on incremental blobs are currently **xfailed** with an explanatory comment. Matching incremental behaviour exactly would require simulating the progressive threshold — tracked as future work under Replay Infrastructure.
### Test results
```
253 passed, 5 skipped, 36 xfailed (0 failures)
```
## Test plan
- [x] D2 tests pass on all datasets with complete Clojure cold-start blobs
- [x] D2c: 3 synthetic tests verify vote counts include moderated-out votes, n_cmts includes moderated-out comments, participants stay in-conv after moderation
- [x] D2d: 5 monotonicity tests (basic across updates, survives moderation, worker restart + moderation, restart without new votes, mixed participants)
- [x] D2 tests xfail on incremental blobs (with explanatory comments)
- [x] Full test suite: 253 passed, 0 failures
- [x] Golden snapshots re-recorded for affected datasets
🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Squashed commits
- Fix D2: in-conv threshold min(7, n_cmts) to match Clojure
- Skip D2 tests on datasets with incomplete Clojure blobs
- Address Copilot review: fix base-cluster sort order (D2b) and stale comment
- Add PR 1 test results to journal
- Plan: add D2c (vote count source) and D2d (in-conv monotonicity) to fix plan
- Journal: add session 3 findings (D2c vote count source, D2d monotonicity)
- Re-record golden snapshots and remove passing xfail markers
- xfail D2 in-conv tests on incremental blobs
- Journal: add session 4, update plan with D2 incremental in Replay PR B
- Fix D2c: use raw_rating_mat for vote counts and n_cmts threshold
commit-id:c0a682ec
jucor
force-pushed
the
spr/edge/c0a682ec
branch
from
02284b0 to
7f20a34
Compare
Delphi Coverage Report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the in-conv participant threshold (D2), vote count source (D2c), and base-cluster sort order (D2b) to match Clojure. Adds monotonicity guard tests (D2d).
D2: In-conv threshold
threshold = 7 + sqrt(n_cmts) * 0.1— increasingly restrictive for larger conversations (e.g., 8.8 for biodiversity's 314 comments)threshold = min(7, n_cmts)— matches Clojure exactlyD2b: Base-cluster sort order (from Copilot review)
(sort-by :id ...)D2c: Vote count source (raw vs filtered matrix)
_compute_user_vote_countsandn_cmtsusedself.rating_mat(filtered — moderated-out comment columns removed). A participant who voted on 8 comments could drop to 5 visible votes after 3 comments were moderated-out, falling below threshold.self.raw_rating_mat(includes all votes, even on moderated-out comments), matching Clojure'suser-vote-counts(conversation.clj:217-225) which reads fromraw-rating-mat.D2d: In-conv monotonicity (design decision)
Python does full recompute from
raw_rating_matevery time, so monotonicity ("once in, always in") is guaranteed without persistence — votes are immutable in PostgreSQL, so a participant's count never decreases. This is strictly better than Clojure's approach (which persists in-conv tomath_mainbecause it uses delta vote processing).5 guard tests (T1-T5) document this invariant and warn that switching to delta processing would require persisting in-conv to DynamoDB (ref: #2358).
Impact
Incremental vs cold-start blob testing
D2 tests run against both cold-start and incremental Clojure blobs (infrastructure from #2420):
min(7, n_cmts)is evaluated once with the finaln_cmts. Python matches these exactly.n_cmts, admitting a few extra participants during earlier iterations. The difference is tiny (1–2 participants).D2 tests on incremental blobs are currently xfailed with an explanatory comment. Matching incremental behaviour exactly would require simulating the progressive threshold — tracked as future work under Replay Infrastructure.
Test results
Test plan
🤖 Generated with Claude Code
Squashed commits
commit-id:c0a682ec
Stack: