Skip to content

python-math #10: feat(math): in-conv greedy floor + persistent carry (PR-E) - #2623

Draft
jucor wants to merge 1 commit into
spr/edge/ca2af3c7from
spr/edge/a861cd87
Draft

python-math #10: feat(math): in-conv greedy floor + persistent carry (PR-E)#2623
jucor wants to merge 1 commit into
spr/edge/ca2af3c7from
spr/edge/a861cd87

Conversation

@jucor

@jucor jucor commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

What

Ports the two Clojure :in-conv steps the Python pipeline was missing (conversation.clj:243-269) into the clojure-legacy engine mode (the setting that reproduces the old Clojure engine's behavior exactly) of Conversation._get_in_conv_participants. "In-conv" is the set of participants included in clustering. Improved mode (the default) is unchanged: threshold set only, no carry, no greedy floor.

In legacy mode, every tick now does:

  1. CARRY: unions the vote-threshold set (>= min(7, n_cmts) votes) into the PERSISTENT in-conv set carried on the conversation object ((or (:in-conv conv) #{}), conversation.clj:247), so a participant, once in, never leaves.
  2. GREEDY FLOOR: if fewer than 15 participants are in, greedily admits the top 15 - n remaining participants by vote count descending, INCLUDING below-threshold voters (conversation.clj:259-268), and persists them.

The persisted set feeds base clustering (it decides which matrix rows are clustered) and is serialized as :in-conv in the JSON result blob (to_dict), matching the clustered rows.

self.in_conv is threaded in-memory across update_votes (via the deepcopy in recompute), NOT persisted to DynamoDB — same lifetime as the PCA/smoother/cluster warm state.

Greedy tie rule (flagged for the integrator)

Clojure sorts a hash-map with (sort-by (comp - second)), whose order among EQUAL vote counts is hash-map iteration order — inherently non-deterministic. We break ties by matrix ROW ORDER (user-vote-counts insertion order) via a STABLE sort — a deterministic, reproducible surrogate for that underspecified Clojure tie case.

Testing

RED evidence (against the PR-C HEAD, before this commit): test_legacy_greedy_fills_to_fifteen — got 2 (threshold only) != 15; test_legacy_greedy_tie_break_is_row_order — low-vote participants never admitted; the carry / self.in_conv tests — AttributeError (no in_conv field / no greedy).

GREEN: 9 new tests pass; full suite 520 passed / 17 skipped / 47 xfailed (was 511 after PR-C), zero regressions. The vw cold-identity gate stays green (vw has 67 in-conv participants > 15, so no greedy fires and the serialized :in-conv is unchanged).

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

commit-id:a861cd87


Stack:


⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

@jucor
jucor marked this pull request as draft July 18, 2026 02:29
@jucor
jucor force-pushed the spr/edge/a861cd87 branch from d65c2ef to 55b02d5 Compare July 18, 2026 13:28
@jucor
jucor force-pushed the spr/edge/ca2af3c7 branch from 99c120b to 380f97a Compare July 18, 2026 13:28
@jucor
jucor force-pushed the spr/edge/a861cd87 branch from 55b02d5 to 7c607b1 Compare July 18, 2026 13:32
@jucor
jucor force-pushed the spr/edge/ca2af3c7 branch 2 times, most recently from 6a3085f to eb3dcb5 Compare July 18, 2026 13:35
@jucor
jucor force-pushed the spr/edge/a861cd87 branch from 7c607b1 to e5344f9 Compare July 18, 2026 13:35
@jucor
jucor requested a review from Copilot July 21, 2026 08:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports Clojure’s :in-conv semantics into the Python clojure-legacy engine path so legacy-mode clustering uses the same “persistent carry + greedy floor to 15” participant selection behavior as the Clojure math pipeline, while keeping the default improved mode unchanged.

Changes:

  • Add Conversation.in_conv warm-state to persist legacy-mode in-conv membership across ticks (carry semantics).
  • Implement legacy-mode greedy floor logic in _get_in_conv_participants (fill to 15 by vote-count, stable tie-break by row order, and prune banned participants).
  • Add a focused test module covering improved-vs-legacy behavior, carry persistence, serialization, monotonicity, and ban-prune floor refill.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
delphi/polismath/conversation/conversation.py Adds legacy-mode persistent in_conv state, greedy floor + prune logic in participant selection, and legacy serialization of in-conv to match clustered rows.
delphi/tests/test_in_conv_greedy_carry.py New unit tests validating legacy carry+greedy behavior and ensuring improved mode remains threshold-only.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread delphi/polismath/conversation/conversation.py
Comment thread delphi/polismath/conversation/conversation.py Outdated
@jucor
jucor force-pushed the spr/edge/ca2af3c7 branch from eb3dcb5 to 45d5fca Compare July 21, 2026 11:11
@jucor
jucor force-pushed the spr/edge/a861cd87 branch 2 times, most recently from 81f5d9d to c9ec6ad Compare July 21, 2026 19:49
This was referenced Jul 25, 2026
@jucor
jucor force-pushed the spr/edge/a861cd87 branch from a5cdbc5 to 727f71c Compare July 28, 2026 00:11
@jucor
jucor force-pushed the spr/edge/ca2af3c7 branch from 05487ba to 37b9be8 Compare July 28, 2026 00:12
@jucor jucor changed the title feat(math): in-conv greedy floor + persistent carry (PR-E) python-math #10: feat(math): in-conv greedy floor + persistent carry (PR-E) Jul 28, 2026
…(PR-E)

## What

Ports the two Clojure `:in-conv` steps the Python pipeline was missing (`conversation.clj:243-269`) into the `clojure-legacy` engine mode (the setting that reproduces the old Clojure engine's behavior exactly) of `Conversation._get_in_conv_participants`. "In-conv" is the set of participants included in clustering. Improved mode (the default) is unchanged: threshold set only, no carry, no greedy floor.

In legacy mode, every tick now does:

1. CARRY: unions the vote-threshold set (>= `min(7, n_cmts)` votes) into the PERSISTENT in-conv set carried on the conversation object (`(or (:in-conv conv) #{})`, `conversation.clj:247`), so a participant, once in, never leaves.
2. GREEDY FLOOR: if fewer than 15 participants are in, greedily admits the top `15 - n` remaining participants by vote count descending, INCLUDING below-threshold voters (`conversation.clj:259-268`), and persists them.

The persisted set feeds base clustering (it decides which matrix rows are clustered) and is serialized as `:in-conv` in the JSON result blob (`to_dict`), matching the clustered rows.

`self.in_conv` is threaded in-memory across `update_votes` (via the deepcopy in `recompute`), NOT persisted to DynamoDB — same lifetime as the PCA/smoother/cluster warm state.

## Greedy tie rule (flagged for the integrator)

Clojure sorts a hash-map with `(sort-by (comp - second))`, whose order among EQUAL vote counts is hash-map iteration order — inherently non-deterministic. We break ties by matrix ROW ORDER (`user-vote-counts` insertion order) via a STABLE sort — a deterministic, reproducible surrogate for that underspecified Clojure tie case.

## Testing

RED evidence (against the PR-C HEAD, before this commit): `test_legacy_greedy_fills_to_fifteen` — got 2 (threshold only) != 15; `test_legacy_greedy_tie_break_is_row_order` — low-vote participants never admitted; the carry / `self.in_conv` tests — AttributeError (no `in_conv` field / no greedy).

GREEN: 9 new tests pass; full suite 520 passed / 17 skipped / 47 xfailed (was 511 after PR-C), zero regressions. The `vw` cold-identity gate stays green (`vw` has 67 in-conv participants > 15, so no greedy fires and the serialized `:in-conv` is unchanged).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

commit-id:a861cd87
@jucor
jucor force-pushed the spr/edge/a861cd87 branch from 727f71c to afd1714 Compare July 28, 2026 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants