Skip to content

fix(math): clamp subgroup-k-smoother smoothed-k to available clusterings (#2575)#2609

Open
jucor wants to merge 1 commit into
edgefrom
jc/subgroup-k-smoother-clamp
Open

fix(math): clamp subgroup-k-smoother smoothed-k to available clusterings (#2575)#2609
jucor wants to merge 1 commit into
edgefrom
jc/subgroup-k-smoother-clamp

Conversation

@jucor

@jucor jucor commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #2575. PR #2536 clamped smoothed-k in group-k-smoother, but the identical bug remained one level down in :subgroup-k-smoother — which #2536 did not touch. This applies the same clamp to the subgroup smoother.

Root cause (same chain as #2536, one level down): a group's subgroup clustering runs k-means for k ∈ (range 2 (inc M)), where M is count-based on the group's base-cluster count (M = min(max-k, 2 + ⌊#base-clusters/12⌋)). The :subgroup-k-smoother carries smoothed-k forward across ticks (anti-flap) without clamping to the current keys. When a group's membership drops below a /12 boundary, M falls; the carried smoothed-k can exceed M; (get group-subgroup-clusterings smoothed-k) returns nil(map f nil)() — an empty subgroup clustering → conv-repness crashes. On post-#2536 master, #2536's guard turns this into a clear IllegalArgumentException ("group-clusters is nil or empty"), but conv-update still fails — the guard makes it legible, it doesn't prevent the empty clustering.

Reported by @lgelauff, reproduced via warm-path replay across 50+ public openData conversations.

Change

math/src/polismath/math/conversation.clj, :subgroup-k-smoother — mirror #2536's group-level clamp into the per-group smoothed-k computation:

;; clamp the carried smoothed-k to a key that exists in THIS group's current clusterings
clamped-smoothed-k (if (contains? group-subgroup-clusterings smoothed-k) smoothed-k this-k)

this-k is always a valid key (chosen from the current keys); when M drops, the group genuinely supports fewer subclusters, so smoothed-k should follow it down.

Test plan

  • New regression test stale-subgroup-smoothed-k-is-clamped-to-available-subgroup-clusters in conv_edge_cases_test.clj, mirroring the existing group-level stale-smoothed-k-is-clamped-to-available-group-clusters. Verified RED before the fix (smoothed-k=5 not in (2 3), downstream lookup → nil), GREEN after.
  • Full math test suite: 53 tests, 150 assertions, 0 failures, 0 errors (baseline was 52/148).

🤖 Generated with Claude Code

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jucor

jucor commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

Was auto-closed by github when I merged the update to the docs in #2610 , because the descriptions talks about "fix in #2609". Github mistook that for "2610 is the fix of 2609". It isn't.

…ngs (#2575)

PR #2536 clamped smoothed-k in group-k-smoother but left the identical bug in
:subgroup-k-smoother. A group's subgroup clustering runs k-means for
k in (range 2 (inc M)), where M is count-based on the group's base-cluster
count. When group membership drops below a /12 boundary, M falls, the carried
smoothed-k can exceed M, and (get group-subgroup-clusterings smoothed-k)
returns nil — an empty subgroup clustering that crashes conv-repness (cryptic
ISeq error pre-#2536, or #2536's clear IAE on master). conv-update still fails.

Mirror #2536's group-level clamp into the per-group subgroup smoother: if the
carried smoothed-k is no longer a key in this group's current subgroup
clusterings, fall back to this-k (the best available k by silhouette).

Test: stale-subgroup-smoothed-k-is-clamped-to-available-subgroup-clusters in
conv_edge_cases_test.clj, mirroring the group-level regression test. Full math
suite: 53 tests, 150 assertions, 0 failures.

Reported by lgelauff (Lodewijk), reproduced via warm-path replay across 50+
public openData conversations.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jucor
jucor force-pushed the jc/subgroup-k-smoother-clamp branch from ed02b20 to 207fa9f Compare July 17, 2026 01:27
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.

conv-update crash: subgroup-k-smoother is missing the smoothed-k clamp that #2536 added to group-k-smoother (empty subgroup clustering)

2 participants