Skip to content
Draft
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
75 changes: 42 additions & 33 deletions delphi/polismath/conversation/conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,41 +147,25 @@ def priority_metric(is_meta: bool,
the decay factor `1 + 8 * 2^(-S/5)` lets new (low-S) comments bubble up
and fades as more votes accumulate.

History: this mirrored Clojure's #1961 truthy-0 bug (every tid took the
meta branch → all priorities 49; issue #2571) until 2026-07-22. Clojure
HEAD passes a real boolean since #2611 (conversation.clj:686), so the
real branching formula is both the correct AND the parity behavior, in
both engine modes.

Args:
is_meta: True for meta comments (treated as constant priority).
A, P, S, E: see `importance_metric`.

Returns:
Squared priority value.

.. warning::
**Current behavior (parity-bug mirror):** this function ALWAYS
returns ``META_PRIORITY ** 2`` and ignores ``is_meta`` and
``A, P, S, E``. It deliberately mirrors a Clojure bug — Clojure
treats meta-tid value 0 as truthy, so every tid takes the meta
branch — for byte-for-byte parity. The branching formula described
above is the *intended* semantics, restored once
https://github.com/compdemocracy/polis/issues/2571 is fixed. See the
``TODO(clojure-parity-bug)`` in the body below.
"""
# TODO(clojure-parity-bug): Clojure (conversation.clj:325) treats meta-tid
# value 0 as TRUTHY in (if is-meta ...), so every tid takes the meta branch.
# We mirror this bug for byte-for-byte Clojure parity. Switch back to
# honoring `is_meta` once the GitHub issue resolves:
# https://github.com/compdemocracy/polis/issues/2571
# Original semantic-correct code preserved below for reference and future
# restoration.
#
# Clojure-parity-bug-mirror: ALWAYS take the meta branch, ignoring is_meta.
return META_PRIORITY ** 2

# Original semantically-correct logic, restore when Clojure bug is fixed:
# if is_meta:
# inner = META_PRIORITY
# else:
# decay_factor = 1 + 8 * (2 ** (-S / 5))
# inner = importance_metric(A, P, S, E) * decay_factor
# return inner ** 2
if is_meta:
inner = META_PRIORITY
else:
decay_factor = 1 + 8 * (2 ** (-S / 5))
inner = importance_metric(A, P, S, E) * decay_factor
return inner ** 2


class Conversation:
Expand Down Expand Up @@ -1370,6 +1354,10 @@ def recompute(self) -> 'Conversation':
prev_base_clusters = getattr(result, 'base_clusters', [])
prev_group_clusterings = getattr(result, 'group_clusterings', {})
prev_group_k_smoother = getattr(result, 'group_k_smoother', {})
# Q2: Clojure's :comment-priorities shadows its current-tick input
# with (:group-votes conv) — the PREVIOUS tick's stored group-votes
# (conversation.clj:658). Captured here, consumed in legacy mode only.
prev_group_votes = getattr(result, 'group_votes', {})

# Compute PCA and projections
result._compute_pca(prev_pca=prev_pca)
Expand All @@ -1385,21 +1373,32 @@ def recompute(self) -> 'Conversation':
result._compute_repness()

# Compute comment priorities (D12 / PR 11). Needs PCA + group_votes.
result._compute_comment_priorities()
result._compute_comment_priorities(prev_group_votes=prev_group_votes)

# Compute participant info
result._compute_participant_info()

return result

def _compute_comment_priorities(self) -> Dict[Any, float]:
def _compute_comment_priorities(
self,
prev_group_votes: Optional[Dict[str, Any]] = None) -> Dict[Any, float]:
"""
Compute per-tid comment priorities matching Clojure
`:comment-priorities` (conversation.clj:648-679).
`:comment-priorities` (conversation.clj:656-687).

Per-tid: sum A/D/S across all groups → P = S - (A + D) → call
`priority_metric(is_meta, A, P, S, E)` where E is the comment
extremity computed from PCA.
extremity computed from the CURRENT tick's PCA.

Which tick's group-votes feed A/D/S is mode-dependent (Q2): Clojure
shadows its current-tick group-votes input with `(:group-votes conv)`
— the PREVIOUS tick's stored value (conversation.clj:658) — so
'clojure-legacy' mode uses `prev_group_votes` (empty on the first
tick, matching Clojure's nil). 'improved' mode uses the current
tick's (the sane behavior). Either way the CURRENT tick's group-votes
are stored on `self.group_votes` for the next tick's capture — the
in-memory analogue of Clojure persisting :group-votes in math_main.

Stores the result on `self.comment_priorities` and also returns it.
TS server `nextComment.ts::getNextPrioritizedComment` consumes this
Expand Down Expand Up @@ -1449,7 +1448,17 @@ def _compute_comment_priorities(self) -> Dict[Any, float]:
# the repness-stage aggregation — tracked in the follow-up issue
# "delphi: _compute_comment_priorities recomputes group votes on
# every tick".
group_votes = self._compute_group_votes()
current_group_votes = self._compute_group_votes()
# Stored for the NEXT tick's prev capture (Clojure keeps :group-votes
# on the conv / in math_main) — in both modes, like self.pca.
self.group_votes = current_group_votes
if resolve_engine_mode() == ENGINE_MODE_LEGACY:
Comment thread
jucor marked this conversation as resolved.
# Q2: previous tick's group-votes (conversation.clj:658);
# {} on the first tick == Clojure's nil (reduce over nothing
# → A/P/S all 0).
group_votes = prev_group_votes if prev_group_votes is not None else {}
else:
group_votes = current_group_votes

priorities: Dict[Any, float] = {}
for tid in self.rating_mat.columns:
Expand Down
68 changes: 16 additions & 52 deletions delphi/tests/test_discrepancy_fixes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1949,35 +1949,18 @@ class TestD12CommentPriorities:
"""

def test_comment_priorities_exist(self, request, conv, clojure_blob, dataset_name):
"""Python should produce comment-priorities matching Clojure.

Per D12.6: Clojure's `(if 0 ...)` truthiness quirk means every tid
takes the meta branch, so Clojure cold_start priorities are all
META_PRIORITY^2 = 49.0 for vw/biodiversity. Python now mirrors this
bug
(priority_metric returns META_PRIORITY**2 unconditionally), so both
sides should yield identical all-constant 49.0. Spearman is not
meaningful when both sides have zero variance — we instead verify
the constant-value parity directly.
"""Python produces REAL (varied) comment-priorities; blob coverage holds.

History: until 2026-07-22 this asserted the all-49 signature on both
sides (Python mirrored Clojure's #1961 truthy-0 bug, #2571). Clojure
HEAD is fixed (#2611) and Python is un-mirrored, so the pins here are
now: (a) priorities exist and cover the blob's tids; (b) Python's
values are NOT the all-constant bug signature. VALUE parity vs
Clojure is no longer checkable against these stale pre-#2611 blobs —
it is validated by the H-B replay battery against Clojure HEAD
(scripts/certify.py); see also the xfail in
test_legacy_clojure_regression.py::test_comment_priorities.
"""
# Per-variant xfail (g5, refined 2026-07-05): known-bad only where
# the Clojure incremental blob has VARIED priorities (no truthy-0
# bug there), so Python's all-49 mirror can't match. FLI and bg2050
# incremental blobs carry the all-49 signature and DO match — they
# gate. All cold_start variants gate. Once the Clojure bug (#2571)
# is fixed upstream, drop the Python mirror and this xfail.
_varied_priority_incrementals = (
'vw-incremental', 'biodiversity-incremental',
'bg2018-incremental', 'engage-incremental',
'pakistan-incremental')
if request.node.callspec.id in _varied_priority_incrementals:
request.applymarker(pytest.mark.xfail(
raises=AssertionError,
strict=False,
reason="D12.6: this Clojure incremental blob has varied "
"priorities (no truthy-0 bug there); Python's "
"all-49 mirror cannot match. See issue #2571."))

clj_priorities = clojure_blob.get('comment-priorities', {})
check.greater(len(clj_priorities), 0,
f"Clojure has {len(clj_priorities)} comment priorities")
Expand All @@ -1998,32 +1981,17 @@ def test_comment_priorities_exist(self, request, conv, clojure_blob, dataset_nam
check.greater(len(common_tids), 0, "Should have common priority tids")

tids_sorted = sorted(common_tids)
clj_vals = [clj_p[t] for t in tids_sorted]
py_vals = [py_p[t] for t in tids_sorted]
clj_unique = set(clj_vals)
py_unique = set(py_vals)
print(f"[{dataset_name}] clj_vals sample: {clj_vals[:5]}, "
f"min={min(clj_vals)}, max={max(clj_vals)}, "
f"unique={len(clj_unique)}")
print(f"[{dataset_name}] py_vals sample: {py_vals[:5]}, "
f"min={min(py_vals)}, max={max(py_vals)}, "
f"unique={len(py_unique)}")

# D12.6 Clojure-parity-bug mirror: both sides should return
# META_PRIORITY**2 = 49.0 for every tid.
META_PRIORITY_SQ = META_PRIORITY ** 2
check.equal(len(clj_unique), 1,
f"Clojure priorities should be all-constant (bug); got {len(clj_unique)} unique")
check.equal(len(py_unique), 1,
f"Python priorities should be all-constant (bug mirror); got {len(py_unique)} unique")
if len(clj_unique) == 1:
(clj_const,) = clj_unique
check.almost_equal(clj_const, META_PRIORITY_SQ, abs=1e-9,
msg=f"Clojure constant priority should be META_PRIORITY**2={META_PRIORITY_SQ}")
if len(py_unique) == 1:
(py_const,) = py_unique
check.almost_equal(py_const, META_PRIORITY_SQ, abs=1e-9,
msg=f"Python constant priority should be META_PRIORITY**2={META_PRIORITY_SQ}")
# Un-mirrored formula: real data always yields varied priorities.
# All-constant output would mean the #2571 mirror crept back in.
check.greater(len(py_unique), 1,
"Python priorities must be varied (real formula), not "
"the all-constant #2571 mirror signature")


class TestD12PriorityExtremityAlignment:
Expand Down Expand Up @@ -2229,10 +2197,6 @@ def test_priority_metric_meta_constant(self):
assert priority_metric(True, 5, 2, 10, 1.5) == META_PRIORITY ** 2
assert priority_metric(True, 0, 0, 0, 0) == META_PRIORITY ** 2

@pytest.mark.xfail(reason="Clojure parity bug mirror (D12.6): priority_metric always "
"returns META_PRIORITY**2 until upstream Clojure bug resolves. "
"Tests pin the semantically-correct formula and will pass again "
"when we revert the mirror.")
def test_priority_metric_non_meta_squared(self):
"""Non-meta: return = (importance * (1 + 8*2^(-S/5)))^2."""
# A=20, P=3, S=20, E=0 — ref from conversation.clj:337
Expand Down
11 changes: 10 additions & 1 deletion delphi/tests/test_engine_mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,16 @@ def _recompute_to_dict(self, monkeypatch, mode):
# Pin last_updated so the two runs share a deterministic value.
conv.last_updated = 0
result = conv.recompute()
return _strip_volatile(result.to_dict())
d = _strip_volatile(result.to_dict())
# ONE documented first-tick exception (Q2, 2026-07-22): Clojure's own
# :comment-priorities reads the PREVIOUS tick's group-votes
# (conversation.clj:658), which is nil on the first tick — so
# Clojure-faithful legacy tick-1 priorities come from zero counts and
# CANNOT equal improved's current-tick-based values. Every other key
# keeps the cold-start invariance guarantee. Legacy tick-1 zero
# semantics are pinned in test_priority_unmirror.py.
d.pop('comment_priorities', None)
return d

def test_vw_cold_run_identical_across_modes(self, monkeypatch):
improved = self._recompute_to_dict(monkeypatch, ENGINE_MODE_IMPROVED)
Expand Down
30 changes: 14 additions & 16 deletions delphi/tests/test_legacy_clojure_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,22 +312,20 @@ def test_comment_priorities(self, request, conversation_data):
clojure_output = conversation_data['clojure_output']
dataset_name = conversation_data['dataset_name']

# Per-variant xfail (g5, refined 2026-07-05): known-bad only where
# the Clojure incremental blob has VARIED priorities (no truthy-0
# bug there). FLI and bg2050 incremental blobs carry the all-49
# signature and match Python's mirror — they gate, as do all
# cold_start variants. Drop this once the Clojure bug (#2571) is
# fixed and the Python mirror is removed.
_varied_priority_incrementals = (
'vw-incremental', 'biodiversity-incremental',
'bg2018-incremental', 'engage-incremental',
'pakistan-incremental')
if request.node.callspec.id in _varied_priority_incrementals:
request.applymarker(pytest.mark.xfail(
raises=AssertionError, strict=False,
reason="D12.6: this Clojure incremental blob has varied "
"priorities (no truthy-0 bug there); Python's "
"all-49 mirror cannot match. See issue #2571."))
# Un-mirror (2026-07-22): Python computes the REAL priority formula
# (Clojure HEAD fixed #1961 via #2611; the #2571 mirror is removed),
# so exact-value parity against these STALE pre-#2611 reference blobs
# (all-49 signature on every cold_start + FLI/bg2050 incrementals;
# bug-free-Clojure varied values on the rest, but from a different
# warm-start trajectory) is not achievable for ANY variant. Value
# parity vs Clojure HEAD is validated by the H-B replay battery
# (scripts/certify.py). Re-enable this comparison after the blobs
# are regenerated with a fixed-Clojure generator (needs prodclone).
request.applymarker(pytest.mark.xfail(
raises=AssertionError, strict=False,
reason="reference blobs predate the Clojure #2611 priority fix; "
"Python un-mirrored 2026-07-22 — exact-value parity is "
"validated via the H-B replay battery until blob regen"))

print(f"\n[{dataset_name}] Testing comment priorities...")

Expand Down
Loading
Loading