[i,l,-r] auto-narrow: rank auto-picked reviewers availability-first (SECOP-1098)#16
Merged
Merged
Conversation
phosphore
added a commit
that referenced
this pull request
Jul 10, 2026
… per-candidate degradation (SECOP-1104) Addresses the four defects the adversarial review found in the SECOP-1098 ranking (all in this unmerged #16): - Latency: enrich the shortlist CONCURRENTLY (async lookup→presence+tz chains, joined once) instead of ~3×15 strictly-sequential blocking calls on the already-slow propose POST. - Per-candidate degradation: each chain .exceptionally → 'unknown' (tier 2); one bad candidate no longer aborts ranking for all (the outer fail-open now only catches systemic errors). - Timezone wrap: compare offsets on the 24h circle (min(d, 86400-d)) so antimeridian teammates with identical local clocks rank as close, not ~24h apart. - Scope-error visibility: SlackClient.isActive/timezoneOffsetSeconds WARN on !isOk (e.g. missing users:read), and the ranking INFO log carries the tier distribution (active/tzNear/other) so a silently inert ranking is detectable. Tests: antimeridian promotion, per-candidate degradation vs wholesale fallback. Suite 1050/1050. NOTE: still to do before #16 merges — rebase onto the SECOP-1100..1103 fixes (PR #17) and reconcile the version bump; and the cross-phase lookupUserByEmail cache (ranking re-looks-up winners the fan-out then looks up again) is deferred as a pure optimization.
On an empty picker selection we auto-pick the requester's closest teammates; ranking them purely by team affinity meant an asleep or off-shift teammate ranked the same as one online now. Since the point of picking is to minimise how long the requester waits, reorder the affinity shortlist availability-first before capping at 5: tier 0: currently active on Slack tier 1: in a working-hours timezone within 2h of the requester tier 2: everyone else (stable sort, so affinity breaks ties within a tier) Wiring: ProposalHandler gains a rankReviewersByAvailability default (no-op passthrough — mail/debug keep pure affinity); SlackProposalHandler overrides it using users.getPresence + users.info (tz), reusing the SlackClient it already holds, so GroupsResource needs no new dependency. Guarantees: - availability is a tiebreaker, never a filter — an offline teammate is deprioritised, not dropped, so an off-hours request still lands in the closest teammates' DMs; - fail-open: any Slack enrichment error falls back to affinity order; - bounded: only a 15-teammate shortlist is enriched, behind the picker's existing per-user rate limiter. NOTE: presence/tz behaviour under the bot token wants live validation on a real workspace before this deploys (it's fail-open, so no risk to the base flow if it misbehaves). Version -> 2.3.0-wavemm.12.
… per-candidate degradation (SECOP-1104) Addresses the four defects the adversarial review found in the SECOP-1098 ranking (all in this unmerged #16): - Latency: enrich the shortlist CONCURRENTLY (async lookup→presence+tz chains, joined once) instead of ~3×15 strictly-sequential blocking calls on the already-slow propose POST. - Per-candidate degradation: each chain .exceptionally → 'unknown' (tier 2); one bad candidate no longer aborts ranking for all (the outer fail-open now only catches systemic errors). - Timezone wrap: compare offsets on the 24h circle (min(d, 86400-d)) so antimeridian teammates with identical local clocks rank as close, not ~24h apart. - Scope-error visibility: SlackClient.isActive/timezoneOffsetSeconds WARN on !isOk (e.g. missing users:read), and the ranking INFO log carries the tier distribution (active/tzNear/other) so a silently inert ranking is detectable. Tests: antimeridian promotion, per-candidate degradation vs wholesale fallback. Suite 1050/1050. NOTE: still to do before #16 merges — rebase onto the SECOP-1100..1103 fixes (PR #17) and reconcile the version bump; and the cross-phase lookupUserByEmail cache (ranking re-looks-up winners the fan-out then looks up again) is deferred as a pure optimization.
phosphore
force-pushed
the
lorenzo/secop-1098-availability-ranking
branch
from
06beed2 to
8413739
Compare
Member
Author
|
Rebased onto the merged review fixes (PR #17, master a3fe96e) and reconciled the version → 2.3.0-wavemm.13. The availability ranking now sits on top of the atomic-claim / dedupe-redesign / IAM-retry fixes; full suite 1054/1054. Plus the SECOP-1104 fixes (parallel enrichment, per-candidate degradation, antimeridian tz wrap, scope-error visibility). Still wants live Slack validation of users.getPresence/users.info under the bot token before it deploys (fail-open regardless). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On an empty picker selection, the auto-pick chooses the requester's closest teammates. Ranking them purely by affinity meant an asleep teammate ranked the same as one online now — but the point of picking is to shorten the requester's wait. This reorders the affinity shortlist availability-first before capping at 5:
Stable sort, so affinity still breaks ties within a tier.
Wiring (no new CDI bean):
ProposalHandlergains arankReviewersByAvailabilitydefault (no-op passthrough → mail/debug keep pure affinity);SlackProposalHandleroverrides it viausers.getPresence+users.info, reusing theSlackClientit already holds.GroupsResourcejust callsthis.proposalHandler.rankReviewersByAvailability(...)— the handler it's already injected with.Guarantees (per the ticket + Lorenzo's steer):
Tests: presence-promotion, tz-tiebreak, fail-open, singleton short-circuit, default-passthrough, and a GroupsResource wire-up test. Suite 1048/1048. Version →
2.3.0-wavemm.12.users.getPresence/users.infobehave under the bot token and the added call volume (~2–3/candidate on a ≤15 shortlist) sits comfortably under Slack rate limits. Fail-open means no risk to the base flow regardless — worst case it silently behaves like today's affinity order.🤖 Generated with Claude Code