Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -1913,3 +1913,41 @@ Verified: local full suite **403 passed / 0 failed** (baseline was 1 failed +
_(Storage-v2 CI green-up — delphi_storage Dockerfile COPY, the
postgres://→postgresql:// backend hardening, and the PG-conformance CI wiring
— is tracked in `STORAGE_V2_IMPLEMENTATION_NOTES.md`, not here.)_

### Session: Clojure routing-bug (#1961) fix + D12 priority-parity discovery (2026-07-17)

**Clojure comment-routing bug fixed** (`math/src/polismath/math/conversation.clj`,
`:comment-priorities`). The node passed `meta-tid-value = (if meta-tids (get meta-tids tid 0) 0)`
into `priority-metric`'s `is-meta` slot. `(get … 0)` returns `0` for non-meta tids, and **0 is
truthy in Clojure**, so `(if is-meta …)` took the meta branch for EVERY comment → all
priorities = `meta-priority^2 = 49` → the TypeScript server's `selectProbabilistically`
degraded to uniform-random routing (the pre-2018 behavior). Introduced by **#1961**
(2025-03-15, "cutoff for large-convo processing if > 5000 comments"). Fix: pass a real
boolean — `(priority-metric (contains? meta-tids tid) A P S extremity)` — `contains?` is
false for non-meta tids and safe when `meta-tids` is nil. Verified end-to-end: regenerating
the vw cold-start blob from the rebuilt (fixed) math image yields **varied** priorities
(125 distinct, 5.16–61.95) instead of all-49.

**NEW — D12 comment-priorities are NOT actually at parity (discovered here).** The all-49
bug was *masking* a real priority non-parity. With the Clojure bug fixed and the Python
`priority_metric` bug-mirror hypothetically un-mirrored (honoring `is_meta`), fixed-Python
and fixed-Clojure vw priorities are **rank-uncorrelated** (Spearman −0.03; top-10 comment
overlap 0/10; Python range 0.18–16.8, Clojure 5.16–61.95). While both sides returned the
constant 49, D12's parity assertion passed **trivially** (49 == 49). Likely contributors:
participant filtering (Clojure `in-conv` = 67 vs Python ~68–69), a vote-replay delta in the
cold-start generator (copies 4555 of vw's 4683 votes), and — most importantly — the still-open
**extremity/PCA parity gaps** (priority = importance × novelty × extremity²; extremity is the
L2 norm of the PCA comment projection, exactly what D1/D1b are still closing).

**Decision (Julien, 2026-07-17): ship the Clojure fix ALONE.** Only Clojure's math blob feeds
production routing (via the TS server), so the Clojure fix restores correct routing on its own.
We do NOT un-mirror Python, do NOT regenerate the committed cold-start blobs (regenerating
flips them to varied and turns D12 parity legitimately RED — not achievable until extremity/PCA
parity lands), and keep the `priority_metric` bug-mirror in place. The Python un-mirror + blob
regen + true D12 value-parity is now **follow-up work under #2571, blocked on extremity/PCA
(D1/D1b) parity**.

**What's next for D12:** (1) close extremity/PCA parity; (2) reconcile participant-filtering and
the generator's vote-copy delta so cold-start inputs match; (3) THEN un-mirror `priority_metric`,
regenerate cold-start blobs, and change D12's test from the trivial constant-49 check to a real
varied-value / rank-parity assertion.
10 changes: 9 additions & 1 deletion delphi/docs/PLAN_DISCREPANCY_FIXES.md
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,14 @@ This replaces the reading from the math blob with a proper Python computation ma
3. Implement: comment projection/extremity in PCA, `importance_metric`, `priority_metric`, full computation
4. Fix buggy `_compute_votes_base()` method

> **⚠️ Parity blocker (2026-07-17):** step 2's Spearman comparison does **not** pass yet.
> The Clojure `#1961` truthy-0 bug made every Clojure priority `49`, so any Python output
> "matched" trivially; the Python side currently *mirrors* that (`priority_metric` returns
> `META_PRIORITY**2`). Now that the Clojure bug is fixed, fixed-Python vs fixed-Clojure vw
> priorities are rank-**uncorrelated** (Spearman −0.03) — priority parity depends on the
> extremity/PCA parity (D1/D1b) closing first. Do not un-mirror `priority_metric` or
> regenerate cold-start blobs until then. See `CLJ-PARITY-FIXES-JOURNAL.md` 2026-07-17.

---

### PR 12: Fix D15 — Moderation Handling
Expand Down Expand Up @@ -533,7 +541,7 @@ See `delphi/docs/INVESTIGATION_K_DIVERGENCE.md` for the full investigation.
| D9 | Z-score thresholds | **PR 3** | **#2518** | **DONE** ✓ |
| D10 | Rep comment selection | PR 8 | **#2566** | Code-complete + 18 synthetic tests (was mislabeled "VM draft" until 2026-07-04); Copilot-review fixes in **#2586**; open in stack, merge pending edge freeze |
| D11 | Consensus selection | PR 9 | **#2567** | Code-complete + 12 synthetic tests; consensus entries now Clojure blob shape (tid/n-success/… — #2586); open in stack, merge pending edge freeze |
| D12 | Comment priorities | PR 11 | **#2568** | Code-complete + 11 synthetic tests (bug-mirror per #2571); Decimal-preserving serialization (#2586); open in stack, merge pending edge freeze |
| D12 | Comment priorities | PR 11 | **#2568** | Code-complete + 11 synthetic tests (bug-mirror per #2571); Decimal-preserving serialization (#2586); open in stack, merge pending edge freeze. **⚠️ 2026-07-17: Python↔Clojure priority PARITY is NOT achieved.** The Clojure all-49 routing bug (#1961) was masking it — while both sides returned constant 49, the D12 test passed trivially. Clojure-side fix (`(contains? meta-tids tid)`) ships separately; with it, fixed-Python vs fixed-Clojure vw priorities are rank-**uncorrelated** (Spearman −0.03). Un-mirroring `priority_metric` + regenerating cold-start blobs is **BLOCKED on extremity/PCA parity (D1/D1b)**. See journal 2026-07-17. |
| D13 | Subgroup clustering | — | — | **Deferred** (unused) |
| D14 | Large conv optimization | — | — | **Deferred** (Python fast enough) |
| D15 | Moderation handling | PR 12 | **#2523** | **DONE** ✓ (zero-out-columns + downstream `to_math_blob` / `_compute_vote_stats` regressions fixed 2026-06-09 — `to_dict` now routes through `_compute_user_vote_counts()` / `_compute_votes_base()`; `_compute_vote_stats` uses `_get_clean_matrix(raw=True)`) |
Expand Down
12 changes: 6 additions & 6 deletions math/src/polismath/math/conversation.clj
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,12 @@
extremity (or (get extremities tid)
(do
(log/warn "No extremity for tid" tid "zid" (:zid conv))
0))
;; Use 0 as the default when meta-tids is null or doesn't contain the tid
meta-tid-value (if meta-tids
(get meta-tids tid 0)
0)]
(priority-metric meta-tid-value A P S extremity)))
0))]
;; Pass a real boolean. (get meta-tids tid 0) defaults non-meta
;; tids to 0, which is TRUTHY in Clojure, so priority-metric took
;; the meta branch for every comment (regression #1961). See #2571
;; and delphi/docs/MATH_ALGORITHM_HISTORY.md.
(priority-metric (contains? meta-tids tid) A P S extremity)))
Comment thread
jucor marked this conversation as resolved.
tids)))


Expand Down
41 changes: 41 additions & 0 deletions math/test/conv_edge_cases_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,44 @@
(is (= 1 (first result)))
;; bucket 1 has :p2 (voted) → count 1
(is (= 1 (second result))))))


;; ============================================================================
;; Bug 4: comment-priorities collapsed every comment to META_PRIORITY^2 (=49)
;;
;; #1961 (2025-03-15) changed the meta-tid lookup in the :comment-priorities fnk
;; from (meta-tids tid) to (get meta-tids tid 0). For a non-meta tid,
;; (get meta-tids tid 0) returns 0 — and 0 is TRUTHY in Clojure — so
;; priority-metric took the meta branch for EVERY comment, collapsing all
;; priorities to meta-priority^2 = 49 and degrading routing to uniform-random.
;; Fixed by passing a real boolean: (contains? meta-tids tid). See #2571.
;; ============================================================================

(deftest comment-priorities-only-meta-tids-get-meta-priority
(testing "only genuine meta tids get meta-priority^2; non-meta tids get varied importance-based priorities"
(let [priorities-fnk (:comment-priorities conversation/small-conv-update-graph)
tids [1 2 3]
meta-tids #{2} ; only tid 2 is a meta comment
group-votes {0 {:votes {1 {:A 5 :D 1 :S 8}
2 {:A 3 :D 0 :S 6}
3 {:A 1 :D 2 :S 7}}}
1 {:votes {1 {:A 2 :D 1 :S 5}
2 {:A 1 :D 1 :S 4}
3 {:A 4 :D 0 :S 9}}}}
conv {:zid 1 :group-votes group-votes}
pca {:comment-extremity [0.5 1.2 0.8]} ; one per tid, in tids order
meta-priority-sq (double (* conversation/meta-priority conversation/meta-priority))
priorities (priorities-fnk {:conv conv
:group-votes group-votes
:pca pca
:tids tids
:meta-tids meta-tids})]
(testing "the meta tid gets exactly meta-priority^2"
(is (== meta-priority-sq (double (get priorities 2)))))
;; Regression guard for #1961: with the truthy-0 bug, non-meta tids also
;; hit the meta branch and returned meta-priority^2.
(testing "non-meta tids do NOT get meta-priority^2"
(is (not (== meta-priority-sq (double (get priorities 1)))))
(is (not (== meta-priority-sq (double (get priorities 3))))))
(testing "non-meta priorities are varied, not a single constant"
(is (not (== (double (get priorities 1)) (double (get priorities 3)))))))))
Loading