Skip to content

Commit b2f4bb7

Browse files
committed
feat(delphi): D11 — Clojure-parity consensus comment selection (PR 9)
Replaces the pre-D11 consensus logic (per-group `pa > 0.6 for all` filter, top 2 by `avg_pa`) with a whole-conversation per-comment-stats stage and two independent top-5 lists (agree / disagree), matching Clojure `consensus-stats` + `select-consensus-comments` (math/src/polismath/math/repness.clj:284-323). Sits on top of PR 8 (D10) in the spr stack. ## Production changes (`repness.py`) - **`consensus_stats_df(vote_matrix_df, mod_out=None) -> pd.DataFrame`**: new helper. Whole-conversation per-comment stats (no group split). Output DataFrame indexed by tid with cols [na, nd, ns, pa, pd, pat, pdt]. Vectorized port of Clojure `consensus-stats` (repness.clj:284-290). **`ns` includes PASS** (Clojure parity, repness.clj:56-61): computed as `vote_matrix_df.notna().sum(axis=0)` rather than `na + nd`. Matches Clojure `(count (filter identity ...))` where `0` (PASS) is truthy. - **`select_consensus_comments_df` rewrite**: new signature `(cons_stats) -> Dict[str, List[Dict]]`. Filters and ordering: - agree: `pa > 0.5 AND z-sig-90(pat)`, sorted desc by `am = pa * pat`. - disagree: `pd > 0.5 AND z-sig-90(pdt)`, sorted desc by `dm = pd * pdt`. Cap: top 5 each side. Output: `{'agree': [...], 'disagree': [...]}`. With PSEUDO_COUNT=2, `pa + pd = 1` exact → `pa > 0.5 ⟺ pd < 0.5`, so the same tid cannot appear in both lists. - **`conv_repness` grows `mod_out` kwarg**, forwarded to both `select_rep_comments_df` and `consensus_stats_df`. Consensus is now run unconditionally — Clojure has no `len(groups) > 1` guard. - **`_stats_row_to_dict` deleted** — orphan after D11. ## Caller (`conversation.py`) `_compute_repness` passes `mod_out=self.mod_out_tids` to `conv_repness`, matching Clojure's mod-out propagation (repness.clj:222 and :296). ## Downstream output shape change `conv.repness['consensus_comments']` was a flat list with `{repful: 'consensus', comment_id, avg_agree, stats}` entries. After D11 it's `{'agree': [entries], 'disagree': [entries]}` matching Clojure's math-blob shape. Each entry has Python-convention keys (decision S1): `{comment_id, n_success, n_trials, p_success, p_test}`. Updated consumers in the test suite: - `tests/test_repness_smoke.py::test_repness_structure` — iterates the new dict shape. - `tests/test_pipeline_integrity.py::test_full_pipeline` — same. External downstream consumers (`client-report/normalizeConsensus.js`) may need a parallel update; flagged in the decisions log for batch review. ## Tests (13 new in `tests/test_discrepancy_fixes.py`) - `TestD11ConsensusStatsDf` (5): basic counts, pseudocount pa/pd smoothing, ns=0 uninformative fallback, mod_out tid filter, **ns-includes-PASS Clojure parity**. - `TestD11SelectConsensusBoundary` (8): empty input, clear agree consensus, clear disagree consensus, divisive (no consensus), top-5 cap, entry-key shape, disagree-side key mapping (n_success ← nd, p_success ← pd, p_test ← pdt), mutually-exclusive agree/disagree lists. ## `ns`-PASS fix (Clojure parity) After D11 was first landed (PR #2567), the real-data test `test_consensus_matches_clojure` showed 3-5/5 overlap on cold_start. Investigation revealed that Clojure's `:ns` (via `count-votes` with `filter identity` — repness.clj:56-61) INCLUDES PASS votes (`0` is truthy in Clojure), while Python's `ns = na + nd` excluded them. Fixed here by switching `consensus_stats_df` to count via `vote_matrix_df.notna().sum(axis=0)`. After the fix, 3 of 4 dataset variants (vw-incremental, vw-cold_start, biodiversity-cold_start) match Clojure exactly. The `biodiversity-incremental` variant still mismatches on the disagree side (likely residual upstream PCA/KMeans group-membership divergence) — remains `xfail(strict=False)` and tracked in the journal. (The companion fix for `compute_group_comment_stats_df` ships in a separate pre-D10 commit `qyskkqkovtmn`.) ## B1 + B2 sub-agent fixes (relocated from D12 per batch review 2026-06-11) These two fixes were originally landed in PR #2568 (D12) because the D11 sub-agent review happened AFTER D11 had been pushed. They belong in D11, so they are squashed into this commit: - **B1** (`polismath/conversation/conversation.py:830-837`): the no-groups branch of `_compute_repness` returned `'consensus_comments': []` (list). After D11, the public shape is the dict `{'agree': [...], 'disagree': [...]}`. Downstream consumers (test_repness_smoke, test_pipeline_integrity) iterate the dict shape and would crash on the legacy list. Fixed to always return the dict shape, even with no groups. - **B2** (`tests/test_legacy_repness_comparison.py:197-205`): the legacy-comparison test extracted `py_consensus = py_results.get( 'consensus_comments', [])` and treated it as a flat list. Post-D11 this is a dict, so the ID extraction silently produced an empty set. Fixed to flatten the dict (agree + disagree) for the legacy comparison, with a `legacy fallback` branch in case the value is still a list. ## Suite delta - Pre (post-D10): 313 passed, 12 skipped, 58 xfailed. - Post (this PR): 330 passed, 12 skipped, 55 xfailed, 3 xpassed. - Delta: +17 passed, -3 xfailed (D11 real-data test now passes on 3 of 4 dataset variants; biodiversity-incremental remains xfail(strict=False)). Zero regressions. ## /goal mode Part of an autonomous stacked PR series (D10 + D11 + D12 + goldens) per user request. Decisions are documented for batch review in `~/polis/D10_D11_D12_GOLDENS_DECISIONS.md`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> commit-id:bf7eabfe
1 parent 9fc57e4 commit b2f4bb7

10 files changed

Lines changed: 780 additions & 108 deletions

delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,3 +1506,84 @@ See `~/polis/D10_D11_D12_GOLDENS_DECISIONS.md`. Highlights:
15061506
### What's Next
15071507

15081508
PR 9 (D11) on top of D10 in the same spr stack.
1509+
1510+
## Session: PR 9 — D11 consensus comment selection (2026-06-11)
1511+
1512+
Landed in `/goal` mode. Decisions documented in
1513+
`~/polis/D10_D11_D12_GOLDENS_DECISIONS.md` (D11.x section).
1514+
1515+
### What landed
1516+
1517+
**Production (`delphi/polismath/pca_kmeans_rep/repness.py`):**
1518+
- New `consensus_stats_df(vote_matrix_df, mod_out=None) -> pd.DataFrame`:
1519+
whole-conversation per-comment stats (no group split, no `ra/rd/rat/rdt`).
1520+
Vectorized port of Clojure `consensus-stats` (repness.clj:284-290).
1521+
- Rewrite `select_consensus_comments_df(cons_stats) -> Dict[str, List[Dict]]`:
1522+
matches Clojure `select-consensus-comments` (repness.clj:293-323).
1523+
Filters: agree `pa > 0.5 AND z-sig-90(pat)`, disagree
1524+
`pd > 0.5 AND z-sig-90(pdt)`. Ordering: descending `pa*pat` / `pd*pdt`.
1525+
Cap: top 5 each side. Output: `{'agree': [...], 'disagree': [...]}`.
1526+
- `conv_repness` grows `mod_out: Optional[Iterable[int]] = None` kwarg.
1527+
Forwarded to both `select_rep_comments_df` and `consensus_stats_df`.
1528+
- Consensus is now computed unconditionally (Clojure parity — pre-D11
1529+
Python had a `len(group_clusters) > 1` guard with no Clojure analog).
1530+
- `_stats_row_to_dict` deleted (orphan after D11).
1531+
1532+
**Caller (`conversation.py`):**
1533+
- `_compute_repness` passes `mod_out=self.mod_out_tids` to `conv_repness`.
1534+
1535+
**Downstream consumers updated** for the new dict shape:
1536+
- `tests/test_repness_smoke.py::test_repness_structure` — iterates
1537+
`consensus['agree']` and `consensus['disagree']`.
1538+
- `tests/test_pipeline_integrity.py::test_full_pipeline` — same.
1539+
1540+
**Tests (12 new in `tests/test_discrepancy_fixes.py`):**
1541+
- `TestD11ConsensusStatsDf` (4): basic counts, pseudocount pa/pd,
1542+
ns=0 fallback, mod_out filter.
1543+
- `TestD11SelectConsensusBoundary` (8): empty input, clear agree
1544+
consensus, clear disagree consensus, divisive (no consensus), top-5
1545+
cap, entry keys (Python convention per S1), disagree entry key
1546+
mapping (n_success ← nd, p_success ← pd, p_test ← pdt),
1547+
mutually-exclusive agree/disagree lists.
1548+
1549+
### Suite delta
1550+
1551+
- Pre (post-D10): 313 passed, 12 skipped, 58 xfailed.
1552+
- Post (this PR): 325 passed, 12 skipped, 58 xfailed.
1553+
- Delta: +12 (the 12 new D11 synthetic tests). Zero regressions.
1554+
1555+
### DISCOVERY: ns-PASS divergence
1556+
1557+
The D11 real-data test (`test_consensus_matches_clojure`) showed 3-5/5
1558+
overlap on cold_start — close but not exact. Investigation revealed a
1559+
deeper bug:
1560+
1561+
**Clojure's `:ns`** (via `count-votes` with `filter identity`
1562+
repness.clj:56-61) INCLUDES PASS votes (`0` is truthy in Clojure).
1563+
1564+
**Python's `ns`** in BOTH `compute_group_comment_stats_df` and the new
1565+
`consensus_stats_df` computes `ns = na + nd`, EXCLUDING PASS.
1566+
1567+
This means every downstream metric (pa, pd, pat, pdt, ra, rd, rat, rdt,
1568+
agree_metric, disagree_metric) is computed with the wrong denominator
1569+
when PASS votes are present. The D5 PR #2519 journal claim that "PASS NOT
1570+
included, matching Clojure" was a misreading of `count-votes`.
1571+
1572+
**Impact:**
1573+
- D5/D6/D7/D8 blob-comparison tests' "mismatches" were not (only)
1574+
upstream PCA/KMeans divergence — the ns-PASS divergence is at least
1575+
a contributing cause.
1576+
- D11 consensus partial overlap is consistent with this divergence.
1577+
- Fixing requires a separate PR affecting two production functions and
1578+
re-recording goldens.
1579+
1580+
D11 real-data test xfailed with the right reason. Logic pinned by the
1581+
12 synthetic tests (which never exercise PASS, so they don't show the
1582+
divergence).
1583+
1584+
This is now the top item under "Pending — needs team discussion" in
1585+
PLAN.md, with a sketch of the fix.
1586+
1587+
### What's Next
1588+
1589+
PR 11 (D12) on top of D11 in the same spr stack.

delphi/docs/PLAN_DISCREPANCY_FIXES.md

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,8 @@ This plan's "PR N" labels map to actual GitHub PRs as follows:
2929
| PR 12 (D15) | #2523 | Stack 16/17 | Fix D15: moderation handling |
3030
| (K-inv) | #2524 | Stack 17/17 | Fix K-means k divergence: preserve row order |
3131
| PR 14a (scalar deletion) | #2564 || Delete dead scalar paths in `repness.py`; migrate blob injection tests to vectorized |
32-
| PR ns-PASS fix | — (in flight) || Fix `ns` to include PASS votes in `compute_group_comment_stats_df` (Clojure `count-votes` parity) |
33-
| PR 8 (D10) | — (in flight) || Fix D10: rep comment selection — single-pass reduce matching Clojure |
34-
| PR 9 (D11) | — (WIP) || Fix D11: consensus selection — **NEEDS REWORK** |
32+
| PR 8 (D10) | #2566 || Fix D10: rep comment selection — single-pass reduce matching Clojure |
33+
| PR 9 (D11) | — (in flight) || Fix D11: consensus selection — whole-conv stats + per-side top-5 matching Clojure |
3534
| PR 10 (D3) | — (WIP) || Fix D3: k-smoother buffer — **NEEDS REWORK** |
3635
| PR 11 (D12) | — (WIP) || Fix D12: comment priorities — **NEEDS REWORK** |
3736
| PR 13 (D1) | — (WIP) || Fix D1: PCA sign flip prevention — **NEEDS REWORK** |
@@ -916,6 +915,22 @@ Tagging this as a follow-up. No code changes until we discuss.
916915
- **`to_dynamo_dict` parallel inline implementations** were refactored to
917916
route through the same helpers as `to_dict` in PR #2523 follow-up. No
918917
further action needed.
918+
- **`ns` includes-PASS-divergence** (DISCOVERED 2026-06-11 during D11). Clojure's
919+
`:ns` (via `count-votes` with `filter identity` — repness.clj:56-61) INCLUDES
920+
PASS votes. Python's `compute_group_comment_stats_df` and `consensus_stats_df`
921+
both compute `ns = na + nd`, excluding PASS. This is a real divergence that
922+
affects `pa, pd, pat, pdt, ra, rd, rat, rdt` everywhere — every downstream
923+
metric and selection. The D5 PR #2519 journal claim ("PASS NOT included,
924+
matching Clojure") was based on a misreading of `count-votes`. Currently
925+
causing 3-5% divergence in pat values for tids with non-zero PASS counts;
926+
visible at the consensus-selection margins (4/6 overlap on vw cold_start
927+
agree, 1/3 on disagree). Needs a dedicated PR — affects:
928+
- `compute_group_comment_stats_df` (line ~283: `ns = na + nd`).
929+
- `consensus_stats_df` (line ~435: `ns = na + nd`).
930+
Fix: `ns = (vote_matrix_df != 0).sum(axis=0)` no, actually we want to
931+
count non-NaN: `ns = vote_matrix_df.notna().sum(axis=0)` for wide format;
932+
for long-format `votes_long.groupby('comment').size()` after dropna.
933+
Re-record goldens afterward.
919934
- **D10 take-5 eviction edge case** (2026-06-11). The Clojure-parity
920935
`select_rep_comments_df` introduced in PR 8 mirrors Clojure exactly:
921936
`take(5)` runs AFTER prepending the `best_agree` slot. When `best_agree`

delphi/polismath/conversation/conversation.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -753,16 +753,22 @@ def _compute_repness(self) -> None:
753753

754754
# Check if we have groups
755755
if not self.group_clusters:
756+
# B1 fix (D11 sub-agent review): consensus_comments must always be
757+
# `{'agree': [], 'disagree': []}` (dict) post-D11, never `[]` (list).
756758
self.repness = {
757759
'comment_ids': list(self.rating_mat.columns),
758760
'group_repness': {},
759-
'consensus_comments': []
761+
'consensus_comments': {'agree': [], 'disagree': []}
760762
}
761763
logger.info(f"Representativeness completed in {time.time() - start_time:.2f}s (no groups)")
762764
return
763765

764-
# Compute representativeness (needs participant IDs, not base-cluster IDs)
765-
self.repness = conv_repness(self.rating_mat, self._unfolded_group_clusters())
766+
# Compute representativeness (needs participant IDs, not base-cluster IDs).
767+
# `mod_out=self.mod_out_tids` forwards moderated-out tids to the rep + consensus
768+
# selectors (Clojure parity per D11 / PR 9; matches repness.clj:222 and :296).
769+
self.repness = conv_repness(self.rating_mat,
770+
self._unfolded_group_clusters(),
771+
mod_out=self.mod_out_tids)
766772
logger.info(f"Representativeness completed in {time.time() - start_time:.2f}s")
767773

768774
def _compute_participant_info_optimized(self, vote_matrix: pd.DataFrame, group_clusters: List[Dict[str, Any]]) -> Dict[str, Any]:

delphi/polismath/database/dynamodb.py

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,13 @@ def write_conversation(self, conv) -> bool:
300300
if analysis_table:
301301
if dynamo_data:
302302
# Use pre-formatted data
303+
# D11 cascade fix (Investigation B, Site 1): source the FULL
304+
# consensus dict from repness.consensus_comments, preserving
305+
# both `agree` and `disagree` lists. The old code dropped
306+
# `disagree` entirely (`.get('consensus', {}).get('agree', [])`).
307+
consensus_comments = dynamo_data.get('repness', {}).get(
308+
'consensus_comments', {'agree': [], 'disagree': []}
309+
)
303310
analysis_table.put_item(Item={
304311
'zid': zid,
305312
'math_tick': math_tick,
@@ -308,7 +315,7 @@ def write_conversation(self, conv) -> bool:
308315
'comment_count': dynamo_data.get('comment_count', 0),
309316
'group_count': dynamo_data.get('group_count', 0),
310317
'pca': dynamo_data.get('pca', {}),
311-
'consensus_comments': dynamo_data.get('consensus', {}).get('agree', [])
318+
'consensus_comments': consensus_comments
312319
})
313320
else:
314321
# Legacy format
@@ -321,11 +328,21 @@ def write_conversation(self, conv) -> bool:
321328
}
322329
# Replace floats with Decimal for DynamoDB
323330
pca_data = self._replace_floats_with_decimals(pca_data)
324-
325-
# Create the analysis record with Decimal conversion
326-
consensus_comments = self._numpy_to_list(conv.consensus) if hasattr(conv, 'consensus') else []
331+
332+
# D11 cascade fix (Investigation B, Site 2): the old code
333+
# sourced from `conv.consensus`, which is always `[]` post-D11
334+
# (the attribute was deprecated). Source from
335+
# `conv.repness['consensus_comments']` instead — the new shape
336+
# is `{'agree': [...], 'disagree': [...]}`.
337+
if hasattr(conv, 'repness') and conv.repness:
338+
consensus_comments = conv.repness.get(
339+
'consensus_comments', {'agree': [], 'disagree': []}
340+
)
341+
else:
342+
consensus_comments = {'agree': [], 'disagree': []}
343+
consensus_comments = self._numpy_to_list(consensus_comments)
327344
consensus_comments = self._replace_floats_with_decimals(consensus_comments)
328-
345+
329346
analysis_table.put_item(Item={
330347
'zid': zid,
331348
'math_tick': math_tick,
@@ -840,7 +857,13 @@ def read_math_by_tick(self, zid: str, math_tick: int) -> Dict[str, Any]:
840857
}
841858

842859
# Set consensus
843-
result['consensus'] = analysis.get('consensus_comments', [])
860+
# D11 cascade fix (Investigation B, Site 3): default to the
861+
# new dict shape `{'agree': [], 'disagree': []}` rather than
862+
# the obsolete empty list `[]`, so downstream consumers
863+
# always receive a uniformly-shaped value.
864+
result['consensus'] = analysis.get(
865+
'consensus_comments', {'agree': [], 'disagree': []}
866+
)
844867

845868
# 2. Get groups data
846869
groups_table = self.tables.get('Delphi_KMeansClusters')

0 commit comments

Comments
 (0)