feat(groups): voting with structural ballot independence (I14) - #638
Merged
Conversation
VOTE phases collect explicit ballots; the deliverable is the auditable artifact - weighted tally, raw ballots, losing-side dissents - because LLM ballots are correlated. Independence is enforced at save time (PARALLEL + NONE scope, hard-rejected otherwise), ballots stay peer-hidden via F4's commit-reveal, and parsing is three-tier with out-of-contract votes as non-ballots that count against quorum (as do abstentions). Ties go to the tiePolicy: one moderator tiebreak turn under a separate conversation key, or an honest NONE. HUMAN_DECIDES is save-time rejected until I6 ships. Also folds in the paragraph-4 gap: decision_reached finally has producers (votes and debate verdicts), with a bounded Slack tally block.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
|
Warning Review limit reached
Next review available in: 12 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR adds independent ChangesVOTE phase configuration and validation
Ballot parsing and tallying
Vote execution and output
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GroupContextBuilder
participant GroupMembers
participant VoteTallyEngine
participant PhaseExecutionEngine
participant SlackGroupDiscussionListener
GroupContextBuilder->>GroupMembers: provide independent vote prompt and ballot contract
GroupMembers->>VoteTallyEngine: produce ballot transcript entries
VoteTallyEngine->>PhaseExecutionEngine: return tally outcome
PhaseExecutionEngine->>SlackGroupDiscussionListener: emit decision_reached with decision
SlackGroupDiscussionListener-->>GroupMembers: post winner and bounded tally
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/main/java/ai/labs/eddi/engine/internal/groups/GroupContextBuilder.java (1)
189-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one default
VoteConfigbetween the prompt and the tally.Line 193 creates
new VoteConfig()when the phase carries none.PhaseExecutionEngine.recordVoteDecisionrepeats the same fallback at its own line 432. The ballot prompt and the tally must agree onmethod,options, andoptionsSource. If either default changes independently, the prompt asks for one contract and the tally parses another.Extract the fallback into one accessor, for example a static
VoteTallyEngine.configOf(DiscussionPhase), and call it from both sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ai/labs/eddi/engine/internal/groups/GroupContextBuilder.java` around lines 189 - 196, Centralize the null fallback for vote configuration in a shared accessor such as VoteTallyEngine.configOf(DiscussionPhase). Update GroupContextBuilder’s VOTE branch and PhaseExecutionEngine.recordVoteDecision to use this accessor, ensuring both prompt generation and tallying share the same method, options, and optionsSource defaults.src/test/java/ai/labs/eddi/integrations/slack/SlackGroupDiscussionListenerTest.java (1)
499-513: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the tally truncation branch.
The two new tests cover a well-formed tally and a malformed one. They do not cover the
MAX_TALLY_LINESoverflow branch atSlackGroupDiscussionListenerlines 314-316. Add a case with seven or more options and assert the message contains"… and 1 more option(s)". Use aLinkedHashMapso the retained six are deterministic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/integrations/slack/SlackGroupDiscussionListenerTest.java` around lines 499 - 513, The test suite needs coverage for the MAX_TALLY_LINES overflow path in SlackGroupDiscussionListener. Add a test alongside onDecisionReached_voteWithTally_postsTheTallyBlock that builds the totals with a LinkedHashMap containing at least seven options, invokes onDecisionReached with the tally decision, and verifies the posted message contains "… and 1 more option(s)"; preserve deterministic ordering so exactly six options are retained.src/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceExtendedTest.java (1)
193-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an assertion that the tiebreak runs under its own conversation key.
TIEBREAK_CONVERSATION_KEYexists so the tiebreak prompt never becomes the moderator's recent history. This test proves the moderator broke the tie, but it passes equally if the tiebreak reuses the moderator's own conversation. Assert the key thatmemberConversationIdsgained, or assert the conversation id thatstartConversationwas called with for"mod".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceExtendedTest.java` around lines 193 - 207, The votePhase_tie_moderatorDecides_viaOneTiebreakTurn test only verifies the result, not that the moderator tiebreak uses TIEBREAK_CONVERSATION_KEY. Add an assertion against the recorded memberConversationIds entry or startConversation invocation for "mod", confirming the tiebreak uses its dedicated conversation key rather than the moderator’s normal conversation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/changelog.md`:
- Line 8: Update the changelog entry for “feat(groups): I14” to use the actual
publication date of August 7, 2026, or explicitly mark it as planned if the
future date is intentional.
In `@src/main/java/ai/labs/eddi/configs/groups/mongo/AgentGroupStore.java`:
- Around line 149-152: Update the weight validation in the vote configuration
loop of AgentGroupStore to reject any non-finite value, including NaN and
positive infinity, before storage or use by VoteTallyEngine. Preserve the
existing null and negative-value rejection and the current error path
identifying the specific weight key.
In `@src/main/java/ai/labs/eddi/engine/internal/groups/PhaseExecutionEngine.java`:
- Around line 429-469: Update recordVoteDecision to accept AtomicInteger
turnCounter and int maxTurns, and before invoking moderatorTiebreak enforce the
same turn and cost-ceiling checks used by checkConvergence and runDissentRound;
increment turnCounter when the tiebreak call is allowed. Pass both budget
parameters from GroupConversationService.executeDiscussion at its
recordVoteDecision call site, preserving the existing decision flow when the
tiebreak is blocked.
In `@src/main/java/ai/labs/eddi/engine/internal/groups/VoteTallyEngine.java`:
- Around line 247-259: The unresolved branch in VoteTallyEngine’s tally flow
currently discards losing-side dissent data needed by
PhaseExecutionEngine.moderatorTiebreak. Preserve the parsed ballots or an
equivalent dissent-calculation callback in TallyOutcome, then have the
moderator-resolved path compute dissents against the selected option using
losingDissents rather than reusing the tie record’s empty list; keep normal
winner handling unchanged.
- Around line 242-245: Update the leader filtering in the tally calculation
around max and leaders to compare each total against max using a small
floating-point tolerance rather than exact equality. Ensure mathematically tied
weighted totals are included in leaders so the existing tie policy can handle
them, while preserving the max > 0.0 requirement.
In
`@src/main/java/ai/labs/eddi/integrations/slack/SlackGroupDiscussionListener.java`:
- Around line 300-323: Update appendVoteTally to bound each option key before
appending it to the tally output, reusing the existing buildPreview helper for
truncation. Keep the MAX_TALLY_LINES limit and value rendering unchanged, while
ensuring every formatted tally line has bounded option text so the complete
decision message remains deliverable.
---
Nitpick comments:
In `@src/main/java/ai/labs/eddi/engine/internal/groups/GroupContextBuilder.java`:
- Around line 189-196: Centralize the null fallback for vote configuration in a
shared accessor such as VoteTallyEngine.configOf(DiscussionPhase). Update
GroupContextBuilder’s VOTE branch and PhaseExecutionEngine.recordVoteDecision to
use this accessor, ensuring both prompt generation and tallying share the same
method, options, and optionsSource defaults.
In
`@src/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceExtendedTest.java`:
- Around line 193-207: The votePhase_tie_moderatorDecides_viaOneTiebreakTurn
test only verifies the result, not that the moderator tiebreak uses
TIEBREAK_CONVERSATION_KEY. Add an assertion against the recorded
memberConversationIds entry or startConversation invocation for "mod",
confirming the tiebreak uses its dedicated conversation key rather than the
moderator’s normal conversation.
In
`@src/test/java/ai/labs/eddi/integrations/slack/SlackGroupDiscussionListenerTest.java`:
- Around line 499-513: The test suite needs coverage for the MAX_TALLY_LINES
overflow path in SlackGroupDiscussionListener. Add a test alongside
onDecisionReached_voteWithTally_postsTheTallyBlock that builds the totals with a
LinkedHashMap containing at least seven options, invokes onDecisionReached with
the tally decision, and verifies the posted message contains "… and 1 more
option(s)"; preserve deterministic ordering so exactly six options are retained.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 36aacf4c-56e7-42ad-a064-55c35a88c89b
📒 Files selected for processing (14)
docs/changelog.mddocs/group-conversations.mdsrc/main/java/ai/labs/eddi/configs/groups/model/AgentGroupConfiguration.javasrc/main/java/ai/labs/eddi/configs/groups/model/DiscussionStylePresets.javasrc/main/java/ai/labs/eddi/configs/groups/mongo/AgentGroupStore.javasrc/main/java/ai/labs/eddi/engine/internal/GroupConversationService.javasrc/main/java/ai/labs/eddi/engine/internal/groups/GroupContextBuilder.javasrc/main/java/ai/labs/eddi/engine/internal/groups/PhaseExecutionEngine.javasrc/main/java/ai/labs/eddi/engine/internal/groups/VoteTallyEngine.javasrc/main/java/ai/labs/eddi/integrations/slack/SlackGroupDiscussionListener.javasrc/test/java/ai/labs/eddi/configs/groups/mongo/AgentGroupStoreTest.javasrc/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceExtendedTest.javasrc/test/java/ai/labs/eddi/engine/internal/groups/VoteTallyEngineTest.javasrc/test/java/ai/labs/eddi/integrations/slack/SlackGroupDiscussionListenerTest.java
AgentGroupConfigurationTest.phaseType_allValues pins the enum size; I14 added VOTE as the 12th value.
…sent carry - recordVoteDecision now takes (turnCounter, maxTurns): the moderator tiebreak is a real LLM turn, gated on both the turn budget and the cost ceiling like every other extra call, and counted when it runs. - TallyOutcome carries the parsed ballots so a tie-policy resolution computes losing-side dissents against ITS chosen option; reusing the unresolved record dropped the minority report for the closest votes. - Weighted totals tie by epsilon (1e-9), not ==; ballot weights must be finite at save time (NaN passes every < comparison). - Slack tally lines width-bounded via buildPreview so a synthesis- derived option paragraph cannot push the message past Slack limits. - CodeQL: 6 log-injection sinks sanitized; useless null check removed.
This was referenced Aug 8, 2026
aisabella-ai
self-requested a review
August 8, 2026 08:07
aisabella-ai
approved these changes
Aug 8, 2026
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
3 participants
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.
Second Wave 2 queue item from
planning/group-collaboration-NEXT.md§3 (design: plan §I14). AVOTEphase collects explicit ballots; the deliverable is the auditable process artifact — weighted tally, every raw ballot, losing-side dissents — because LLM ballots are correlated (shared priors, sycophancy) and the plan says so out loud.Design
AgentGroupStore.validateVotePhases) HARD-rejects a VOTE phase that is notPARALLEL+contextScope: NONE. Ballots are cast blind against the pre-fan-out snapshot, andVOTEtranscript entries stay peer-hidden until their phase completes — F4's commit-reveal visibility matrix covered the new phase type the momentmapPhaseToEntryTypemapped it (the newPhaseType.VOTEenum value flushed every exhaustive switch at compile time).VoteTallyEngine, mirroringDebateVerdictParser's discipline incl.FAIL_ON_TRAILING_TOKENS): strict JSON → JSON embedded in prose/fence → a reply naming exactly ONE option's text. Out-of-contract votes are non-ballots, never write-ins; non-ballots and abstentions count against quorum — a mostly-silent team has not reached quorum, and that is signal. "Option B" label votes resolve positionally.VoteConfig: MAJORITY|APPROVAL, EXPLICIT options (the reliable path) orLAST_SYNTHESISOption A: …extraction from the newest synthesis, quorum (default 0.5), per-agent weights,weightByConfidence(default off, correlated-self-report caveat in the Javadoc),tiePolicy.MODERATOR_DECIDESruns one moderator turn under a separate__vote_tiebreakconversation key (the I2 judge's rule: contract prompts must not become the moderator's recent history), resolved by the same exact-scan rule as a ballot;NO_DECISIONrecords an honesttype: NONEand the discussion continues.HUMAN_DECIDESis save-time rejected until I6 ships human members — the queue sequences I14 before I6, and a silently-degrading enum value would be a lie (deviation recorded in the changelog; I6 wires it).decision_reachedfinally fires — the §4 gap folded in as the plan prescribed: producers for vote decisions AND I3 debate verdicts (fired after the dissent round so the event carries merged dissents). Slack renders a bounded, instanceof-guarded tally block.Tests
121 across the touched classes; full
engine.internalsuite green; checkstyle clean.decision_reachedwith the winner; tie + MODERATOR_DECIDES resolves via one tiebreak turn (vote+moderator-tiebreak); tie + NO_DECISION does not fail the discussion; ballots land as VOTE entries.Summary by CodeRabbit
VOTEdiscussion phases with majority or approval voting.