feat(delphi): D10 — Clojure-parity rep comment selection (PR 8)

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

@@ -1216,3 +1216,293 @@ ever appears in real Polis data. It does.
   out of this committed doc; the unredacted findings are in Claude's
   per-project memory store (`~/.claude/projects/...`). Open a follow-up
   discussion with the team before any user-facing action.
+
+## Session: PR 14a — Scalar deletion (2026-06-11)
+
+Foundation pass before D10/D11/D12. The scalar implementations in
+`repness.py` were test-only (production calls only `compute_group_comment_stats_df`
++ `select_rep_comments_df` + `select_consensus_comments_df` via
+`conv_repness`). Deleting them removes the "where do I put this helper?"
+ambiguity for D10/D11/D12 (which all add new helpers to `repness.py`) and
+shrinks the test surface by ~35 obsolete unit tests.
+
+### What landed
+
+**Production code (`delphi/polismath/pca_kmeans_rep/repness.py`)** — 445 lines deleted:
+- DELETE primitives: `prop_test`, `two_prop_test` (both also dead in production —
+  only test consumers).
+- DELETE orchestration: `comment_stats`, `add_comparative_stats`, `repness_metric`,
+  `finalize_cmt_stats`, `passes_by_test`, `best_agree`, `best_disagree`,
+  `select_rep_comments`, `select_consensus_comments`.
+- DELETE unused: `calculate_kl_divergence` (no callers anywhere).
+- KEEP: `z_score_sig_90`, `z_score_sig_95` (trivial threshold checks used scalar-side
+  in selection logic; vectorizing them would not save lines).
+- ENRICH docstrings: `prop_test_vectorized` and `two_prop_test_vectorized` now
+  embed the scalar-equivalent closed-form algebra (so the formula stays readable
+  even though the scalar functions are gone). Pattern from the user during the
+  PR 14a discussion: "where we cannot achieve readability on the vectorized,
+  put in comments showing the non-vectorized equivalent."
+
+**Tests** — net -35 passed tests (296 → 295... actually delta is 330 → 295):
+- DELETE entirely: `tests/test_old_format_repness.py` (557 lines, mirror of
+  scalar-only tests in `test_repness_unit.py`; the "old format" was the scalar
+  dict-in/dict-out API).
+- DELETE classes: `TestCommentStats`, `TestSelectionFunctions`,
+  `TestConsensusAndGroupRepness` in `test_repness_unit.py`. Also
+  `TestStatisticalFunctions::test_prop_test` and `test_two_prop_test`.
+- MIGRATE D4/D5/D6 BlobInjection (`test_discrepancy_fixes.py:1602+`) from
+  per-(gid, tid) scalar loop calls to a single vectorized call on a DataFrame
+  built from the blob's `repness` entries. This pattern (1) tests the actual
+  production code path, (2) produces a `.to_string()` diagnostic that beats
+  hand-formatted f-strings, (3) drops loop overhead.
+- MIGRATE `TestD5ProportionTest::test_prop_test_matches_clojure_formula`,
+  `TestD6TwoPropTest::test_two_prop_test_matches_clojure_formula` + edge cases
+  to single vectorized calls on N-row DataFrames.
+- CONSOLIDATE `TestD8FinalizeStats`'s 7 scalar boundary tests into one
+  parametrized DataFrame test (`test_repful_classification_boundary`) that
+  exercises the production `np.where(rat > rdt, 'agree', 'disagree')` logic.
+  Boundary cases preserved: `rat < rdt`, `rat > rdt`, `rat == rdt` (non-zero,
+  zero), negative z-scores.
+- MIGRATE `TestD7RepnessMetric::test_metric_formula_is_product` to hand-computed
+  reference values (`1.3*1.8*0.8*2.5 = 4.68` for agree, `0.7*-0.9*0.2*-1.5 = 0.189`
+  for disagree — signed product).
+- DELETE redundant scalar-formula tests in `TestSyntheticEdgeCases`
+  (test_prop_test_matches_clojure_formula_synthetic — duplicated by migrated
+  TestD5ProportionTest; test_clojure_repness_metric_product — duplicated by
+  TestD7RepnessMetric; test_clojure_repful_uses_rat_vs_rdt — purely tautological).
+- Rename misleading `test_compute_group_comment_stats_matches_scalar` →
+  `test_compute_group_comment_stats_consistency_with_conv_repness`.
+- Cross-checks in `TestVectorizedFunctions` (test_repness_unit.py) replaced
+  inline scalar calls with `_prop_test_reference` / `_two_prop_test_reference`
+  closed-form staticmethods.
+
+### Suite delta (pre/post PR 14a)
+
+- Pre-baseline (edge @ 2dce7385f): **330 passed, 12 skipped, 58 xfailed**.
+- Post (@ this PR): **295 passed, 12 skipped, 58 xfailed**.
+- Delta: -35 passed, 0 failed, 0 new xfailed. The -35 matches the deleted
+  scalar-only test count (test_old_format_repness ~20 + scalar classes in
+  test_repness_unit ~9 + scalar test methods in test_discrepancy_fixes ~6
+  consolidated/removed).
+
+### For PR 14c (readability refactor)
+
+PR 14c will refactor `compute_group_comment_stats_df` for readability and
+needs to mirror the scalar recipe. **The deleted scalar code is the
+reference.** Retrieve via:
+
+```bash
+git show <PR-14a-commit>~1:delphi/polismath/pca_kmeans_rep/repness.py \
+  | sed -n '161,302p'
+```
+
+Specifically (using the pre-deletion line numbers — file was 1008 lines at
+edge HEAD 2dce7385f):
+
+- `comment_stats` lines 161-201 — the per-(group, comment) recipe.
+- `add_comparative_stats` lines 203-235 — in-vs-out comparison.
+- `repness_metric` lines 237-271 — the `r*rt*p*pt` product.
+- `finalize_cmt_stats` lines 273-301 — agree-vs-disagree branch.
+
+The pre-PR-14a commit hash will be the parent of PR 14a's commit. Clojure
+originals: `math/src/polismath/math/repness.clj:78-100,173-188,191-200`.
+
+### Pyright noise (unrelated to PR 14a, raised same session)
+
+Discovered during PR 14a that pyright produces ~10 errors on `repness.py` from
+pandas-stubs false positives (`pd.DataFrame(columns=...)`, `df['col'] = value`,
+Series-vs-DataFrame narrowing). PR #2560 added a pyright config that points
+at `delphi/.venv` but did NOT set any rule overrides, so default-mode
+`reportArgumentType` / `reportIndexIssue` errors surface for valid pandas code.
+Verified pre-existing on edge HEAD (not introduced by PR 14a).
+
+Handoff written: `~/polis/HANDOFF_PYRIGHT_PANDAS_STUBS.md`. Do NOT just turn
+off rules globally — investigate pandas-stubs version, community patterns,
+targeted ignores. Tracked as Claude task #8.
+
+### What's Next
+
+PR 14a unblocks (in stack order):
+1. **D10** — Rep comment selection. Research agent already produced a fix
+   proposal (this session). Helpers `passes_by_test`, `beats_best_by_test`,
+   `beats_best_agr` go top-level in `repness.py`; reduce structure uses
+   `df.to_dict('records')` iteration with mutable `{sufficient, best, best_agree}`
+   state. Boundary cases identified for synthetic test fixtures.
+2. **D11** — Consensus selection. Research agent produced a fix proposal:
+   needs new `consensus_stats_df(vote_matrix_df) -> pd.DataFrame` (whole-data,
+   not per-group), plus rewrite of `select_consensus_comments_df` with the
+   `{'agree': [...], 'disagree': [...]}` output shape (top 5 each).
+   `conv_repness` must grow `mod_out` kwarg.
+3. **D12** — Comment priorities. Research agent produced a fix proposal:
+   Clojure source at `conversation.clj:311-330,341-352,648-679`;
+   `pca.clj:167-178`. Needs new `pca_project_cmnts`, `comment_extremity`,
+   `importance_metric`, `priority_metric` in Python. `meta_tids` shape mismatch
+   (Python set vs Clojure map) flagged.
+4. **PR 14b** — Backfill missing blob injection tests (D7 metric, D8 finalize,
+   full stats-stage injection).
+5. **Goldens** — Re-record `vw` and `biodiversity` (sklearn KMeans seeding
+   decision pending — see `delphi/scratch/COPILOT_MATH_QUESTIONS.md`).
+6. **PR 14c** — Readability refactor of `compute_group_comment_stats_df`,
+   using the deleted scalar code (retrievable via `git show`) as the
+   readability reference. Research agent produced a clean split proposal:
+   `_build_group_comment_index` (plumbing) + `_compute_per_group_stats`
+   (math) + 5-line orchestrator.
+
+## Session: ns-PASS fix (2026-06-11)
+
+**Context.** While preparing the D10/D11/D12 rework on top of PR 14a, a
+Clojure re-read surfaced a latent bug in `compute_group_comment_stats_df`:
+`ns` (and `total_votes`) were computed as `na + nd`, silently dropping PASS
+votes. Clojure's `:ns` is `(count-votes votes)` (math/repness.clj:56-61,
+:70), which calls `(filter identity votes)`. In Clojure 0 is truthy, so
+PASS (0) counts; only `nil` is filtered out. Therefore Clojure
+`ns = na + nd + np`, and every downstream metric (`pa`, `pd`, `pat`,
+`pdt`, `ra`, `rd`, `rat`, `rdt`, `agree_metric`, `disagree_metric`, plus
+D11's `consensus_stats_df` which will mirror the same recipe at the
+whole-conversation level) was off whenever PASS votes existed.
+
+**Why D5 BlobInjection didn't catch it.** D5's blob-injection tests pull
+`(n-success, n-trials)` straight from the Clojure blob's `repness`
+entries and feed them to `prop_test_vectorized`. They bypass
+`compute_group_comment_stats_df` entirely, so the bug downstream of
+`ns = na + nd` was invisible. The same gap will recur for D11 / D12 until
+we ship pure-formula tests that build a tiny vote matrix and assert on the
+counts. Lesson: blob-injection is necessary but not sufficient — every
+formula whose inputs are themselves computed by Python needs at least one
+pure-formula unit test that exercises the input-building code.
+
+**TDD cycle.**
+- **BASELINE** — full suite at PR 14a parent: 295 passed, 12 skipped,
+  58 xfailed.
+- **RED** — added `TestNsIncludesPassVotes` in `tests/test_repness_unit.py`
+  with four pure-formula tests: single-comment mixed AGREE/DISAGREE/PASS,
+  all-PASS column, NaN-vs-PASS distinction, two-group `other_votes`
+  including out-group PASS. All four failed on the buggy code (4 fails).
+- **GREEN** — in `polismath/pca_kmeans_rep/repness.py`:
+  - `total_counts` now computes `total_votes=('vote', 'size')` directly in
+    the groupby agg, instead of `total_agree + total_disagree`.
+  - `group_counts` now computes `ns=('vote', 'size')` directly, instead of
+    `na + nd`.
+  - `'size'` on the already-`dropna(subset=['vote'])`-filtered frame counts
+    exactly the non-NaN entries — including PASS (0). This is the
+    Clojure `(count (filter identity votes))` recipe verbatim.
+  - Both sites carry a comment citing repness.clj:56-61, :70 and the
+    truthy-0 reasoning.
+  - Docstring updated: `ns` now documented as `agree + disagree + PASS`.
+- **FULL SUITE** — 299 passed, 12 skipped, 58 xfailed. Delta = +4
+  (exactly the new ns-PASS tests). No existing pre-PR-14a or PR-14a test
+  broke. The pre-existing `TestVectorizedFunctions` fixtures use only
+  AGREE/DISAGREE/NaN (no PASS), so they were never sensitive to the bug.
+
+**Cascade to D11.** D11's plan introduces a `consensus_stats_df(vote_matrix_df)`
+function computing whole-conversation stats (the `:mod-out` Clojure path).
+That function will inevitably mirror the same `na + nd + np` recipe — so
+the ns-PASS fix lands BEFORE D10 in the stack to keep D11's implementation
+clean. D11 should follow the same pure-formula test pattern: build a vote
+matrix with mixed PASS and assert `ns == count of non-NaN cells`.
+
+**Goldens.** Stays DEFERRED. Re-recording is gated on
+sklearn-KMeans-seeding consensus (see scratch/COPILOT_MATH_QUESTIONS.md);
+no values shift at the goldens commit until D10/D11/D12 land.
+
+**Stack position.** New commit inserted between PR 14a (#2564) and D10
+(#2566). D10, D11, D12, goldens rebased cleanly on top; 2-sided docs
+conflicts in PLAN/JOURNAL at each downstream commit resolved manually
+to merge both edits (downstream docs additions kept; ns-PASS row and
+session entry preserved).
+
+## Session: PR 8 — D10 rep comment selection (2026-06-11)
+
+Landed in `/goal` mode (autonomous run targeting D10 + D11 + D12 + goldens
+as a stacked PR series). Decisions made without inline user check are
+documented in `~/polis/D10_D11_D12_GOLDENS_DECISIONS.md` for batch review.
+
+### What landed
+
+**Production code (`delphi/polismath/pca_kmeans_rep/repness.py`)** — added
+3 top-level helpers + `_finalize_row_for_output` + rewrote `select_rep_comments_df`:
+
+- `passes_by_test(s) -> bool` — Clojure `passes-by-test?` (repness.clj:165-170).
+  OR'd on `(rat, pat)` and `(rdt, pdt)` z-sig-90. **NO `pa >= 0.5` gate** —
+  the pre-D10 Python gate was a botched-port over-restriction with no
+  Clojure analog.
+- `beats_best_by_test(s, current_best_z) -> bool` — Clojure `beats-best-by-test?`
+  (repness.clj:133-139). Strict `>` on `max(rat, rdt)` vs current best z.
+- `beats_best_agr(s, current_best) -> bool` — Clojure `beats-best-agr?`
+  (repness.clj:142-162). Four-branch agree-priority logic:
+    1. `na == 0 and nd == 0` → reject.
+    2. Current best AND `current_best['ra'] > 1.0` → compare 4-way signed
+       product `ra * rat * pa * pat`.
+    3. Current best (else, `ra <= 1.0`) → compare `pa * pat`.
+    4. No current best → accept if `z90(pat)` OR `(ra > 1.0 AND pa > 0.5)`.
+- `_finalize_row_for_output(row, *, is_best_agree=False)` — Clojure
+  `finalize-cmt-stats` (repness.clj:173-188) + best-agree flagging
+  (repness.clj:262-264). Adds `best_agree=True` and `n_agree=na` keys for
+  the best-agree slot.
+- `select_rep_comments_df(stats_df, mod_out=None) -> List[Dict[str, Any]]` —
+  single-pass reduce over `stats_df.to_dict('records')` mirroring Clojure
+  `select-rep-comments` (repness.clj:212-281). Per-row state
+  `{sufficient, best, best_agree}` updated by the three helpers; final
+  assembly is dedup-best-agree-from-sufficient → sort by metric →
+  prepend best-agree → take 5 → agrees-before-disagrees.
+
+**Caller (`conv_repness`)** — dropped the `_stats_row_to_dict` wrapping
+step since `select_rep_comments_df` now returns finalized dicts directly.
+
+**Two pre-D10 bugs fixed alongside the rewrite** (research-agent flagged):
+- `pa >= 0.5 / pd >= 0.5` over-gate in the passing filter — removed. No
+  Clojure analog; was dropping legitimate candidates.
+- "Fill-from-other-category" + "first-row" fallback blocks — deleted. The
+  `:best` / `:best_agree` mechanism IS the Clojure fallback.
+
+**Tests** — 18 new tests (in `tests/test_discrepancy_fixes.py`):
+- `TestD10PassesByTest` (4 tests): agree-side significant, disagree-side
+  significant, neither significant, no `pa >= 0.5` gate.
+- `TestD10BeatsBestByTest` (3 tests): None-best, max-rat-rdt, strict `>`.
+- `TestD10BeatsBestAgr` (6 tests): Branch 1 (na=nd=0), Branch 2 (ra>1),
+  Branch 3 (ra<=1), Branch 4 z90(pat), Branch 4 (ra>1 AND pa>0.5), Branch 4
+  rejection.
+- `TestD10SelectRepCommentsBoundary` (5 tests): empty input, single unvoted
+  row → best fallback, sufficient-empty-best-agree-only, take-5 cap with
+  agrees-before-disagrees ordering, **the eviction edge case** (best_agree
+  outside sufficient evicting 5th-highest-metric).
+
+**Re-xfailed with updated reasons** (D14 / D1 upstream divergence):
+- `TestD9ZScoreThresholds::test_z_values_match_clojure`
+- `TestD5ProportionTest::test_pat_values_match_clojure_blob`
+- `TestD6TwoPropTest::test_rat_values_match_clojure_blob`
+- `TestD7RepnessMetric::test_repness_metric_matches_clojure_blob`
+- `TestD8FinalizeStats::test_repful_matches_clojure_blob`
+- `TestD10RepCommentSelection::test_rep_comments_match_clojure`
+
+Why xfailed despite D10 landing: D10 enables shared comments in the
+selection (overlap rises from 0% to ~20% on vw cold_start), but
+per-(gid, tid) stats still mismatch because Python and Clojure put
+different participants in the "same" group ID. That's upstream
+PCA/KMeans group-membership divergence (D14 / D1), not D10. D10 is
+verified via the 18 synthetic helper + boundary tests above.
+
+### Suite delta (pre/post D10)
+
+- Pre (post-14a): 295 passed, 12 skipped, 58 xfailed.
+- Post (this PR): 313 passed, 12 skipped, 58 xfailed.
+- Delta: +18 passed, 0 failed, 0 new xfailed. The +18 matches the 18 new
+  D10 synthetic tests exactly.
+
+### Decisions made autonomously (under `/goal` mode)
+
+See `~/polis/D10_D11_D12_GOLDENS_DECISIONS.md`. Highlights:
+- **S1**: Python convention key names (`repful`, `best_agree`, `n_agree`)
+  instead of Clojure hyphens. Math blob alignment is a future PR.
+- **S2**: `select_rep_comments_df` returns `List[Dict[str, Any]]` instead
+  of `pd.DataFrame` — variable extra keys (best_agree flag) make list-of-
+  dicts cleaner than DF-with-NaN-columns.
+- **D10.1**: Two pre-D10 bugs (pa>=0.5 gate, fill-from-other fallback)
+  folded into D10 rather than separate PRs — the rewrite replaces the
+  function so a surgical fix would be more noise than value.
+- **D10.7**: Real-data blob-comparison tests re-xfailed with reasons
+  pointing at D14/D1, not softened to overlap-thresholds — more honest.
+
+### What's Next
+
+PR 9 (D11) on top of D10 in the same spr stack.

delphi/docs/PLAN_DISCREPANCY_FIXES.md

@@ -28,7 +28,9 @@ This plan's "PR N" labels map to actual GitHub PRs as follows:
 | PR 7 (D8) | #2522 | Stack 15/17 | Fix D8: finalize comment stats |
 | PR 12 (D15) | #2523 | Stack 16/17 | Fix D15: moderation handling |
 | (K-inv) | #2524 | Stack 17/17 | Fix K-means k divergence: preserve row order |
-| PR 8 (D10) | — (WIP) | — | Fix D10: rep comment selection — **NEEDS REWORK** |
+| PR 14a (scalar deletion) | #2564 | — | Delete dead scalar paths in `repness.py`; migrate blob injection tests to vectorized |
+| PR ns-PASS fix | — (in flight) | — | Fix `ns` to include PASS votes in `compute_group_comment_stats_df` (Clojure `count-votes` parity) |
+| PR 8 (D10) | — (in flight) | — | Fix D10: rep comment selection — single-pass reduce matching Clojure |
 | PR 9 (D11) | — (WIP) | — | Fix D11: consensus selection — **NEEDS REWORK** |
 | PR 10 (D3) | — (WIP) | — | Fix D3: k-smoother buffer — **NEEDS REWORK** |
 | PR 11 (D12) | — (WIP) | — | Fix D12: comment priorities — **NEEDS REWORK** |
@@ -914,3 +916,14 @@ Tagging this as a follow-up. No code changes until we discuss.
 - **`to_dynamo_dict` parallel inline implementations** were refactored to
   route through the same helpers as `to_dict` in PR #2523 follow-up. No
   further action needed.
+- **D10 take-5 eviction edge case** (2026-06-11). The Clojure-parity
+  `select_rep_comments_df` introduced in PR 8 mirrors Clojure exactly:
+  `take(5)` runs AFTER prepending the `best_agree` slot. When `best_agree`
+  was kept by `beats_best_agr?` as a non-significant agree-priority fallback
+  (i.e. it failed `passes_by_test?` but qualified via Branch 4) AND
+  `:sufficient` already has 5 entries, the prepend pushes the total to 6
+  and `take(5)` silently evicts the 5th-highest-metric `sufficient` entry
+  — possibly a strong dissenting view. Mirrored for blob parity; flag for
+  future product review. See `# TODO(parity-eviction)` in
+  `delphi/polismath/pca_kmeans_rep/repness.py::select_rep_comments_df` and
+  the synthetic test `TestD10SelectRepCommentsBoundary::test_take_5_eviction_when_best_agree_outside_sufficient`.