Skip to content

Fix percentAgreement to prevent false consensus when subset of participants respond - #1012

Merged
JamesPHoughton merged 4 commits into
mainfrom
copilot/fix-754
Aug 27, 2025
Merged

Fix percentAgreement to prevent false consensus when subset of participants respond#1012
JamesPHoughton merged 4 commits into
mainfrom
copilot/fix-754

Conversation

Copilot AI commented Aug 25, 2025

Copy link
Copy Markdown
Contributor

The percentAgreement position modifier was incorrectly calculating consensus percentages, leading to false consensus when only a subset of participants had responded.

Problem

The original implementation had two issues:

  1. Included undefined values in agreement calculation: Participants who hadn't submitted responses (undefined values) were treated as "agreeing" with each other
  2. False consensus with partial responses: When only some participants responded, percentages were calculated using only responders as the denominator, creating misleading consensus signals

For example, with 4 participants where only 1 says "ready":

// Before fix
referenceValues = [undefined, undefined, undefined, "ready"]
definedValues = ["ready"]  // Filter out undefined
percentage = (1/1) * 100 = 100%  // False consensus!

This could trigger premature actions in scenarios like "ready to proceed" checks.

Solution

Modified the percentAgreement logic to:

  1. Filter out undefined values from agreement counting (numerator)
  2. Include all participants in the total count (denominator)
  3. Return false when no responses exist (no agreement possible)
// After fix  
referenceValues = [undefined, undefined, undefined, "ready"]
definedValues = ["ready"]  // Only count actual responses for agreement
percentage = (1/4) * 100 = 25%  // Correct percentage based on total participants

Behavior Changes

  • Partial responses: [undefined, undefined, "ready"] with ≥50% threshold → false (33% agreement, not 100%)
  • Mixed responses: [undefined, "ready", "ready"] with ≥50% threshold → true (67% agreement)
  • All undefined: [undefined, undefined, undefined]false (no responses yet)
  • Normal cases: ["ready", "ready", "ready"] → works exactly as before

This ensures percentAgreement provides accurate consensus measurements for real-time decision scenarios where participants may respond at different times.

Fixes #754.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…lation

Co-authored-by: JamesPHoughton <4304478+JamesPHoughton@users.noreply.github.com>
Copilot AI changed the title [WIP] percentAgreement should not return true when all values are undefined Fix percentAgreement to exclude undefined values from agreement calculation Aug 25, 2025
Copilot AI requested a review from JamesPHoughton August 25, 2025 18:53
@github-actions

Copy link
Copy Markdown
/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/App.jsx
  171:7  error  Unknown property 'test-player-id' found  react/no-unknown-property

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/ConditionalRender.jsx
  39:10  error  'tickTock' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/HairCheck.jsx
  116:6  warning  React Hook useEffect has missing dependencies: 'onFailure', 'onVideoSuccess', and 'player'. Either include them or remove the dependency array. If 'onVideoSuccess' changes too often, find the parent component that defines it and wrap that definition in useCallback  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/IdleProvider.jsx
   56:6   warning  React Hook useEffect has a missing dependency: 'resetTimer'. Either include it or remove the dependency array  react-hooks/exhaustive-deps
  120:57  error    ''' can be escaped with '&apos;', '&lsquo;', '&#39;', '&rsquo;'                                                react/no-unescaped-entities

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/Markdown.jsx
  17:6  error  Unnecessary escape character: !  no-useless-escape

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/SharedNotepad.jsx
  26:6  warning  React Hook useEffect has missing dependencies: 'defaultText', 'game', and 'record'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/TextChat.jsx
   71:6  warning  React Hook useEffect has a missing dependency: 'msgs.length'. Either include it or remove the dependency array  react-hooks/exhaustive-deps
  182:7  warning  Unexpected alert                                                                                                no-alert
  219:7  error    A control must be associated with a text label                                                                  jsx-a11y/control-has-associated-label

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/KitchenTimer.jsx
  9:10  error  'tickTock' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/Prompt.jsx
  19:3  error  iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations  no-restricted-syntax

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/Qualtrics.jsx
  54:10  error  'state' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/TrainingVideo.jsx
  61:6  warning  React Hook useEffect has missing dependencies: 'timer?.elapsed' and 'url'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/Consent.jsx
  129:6  warning  React Hook useEffect has a missing dependency: 'player'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/Countdown.jsx
  8:21  error  Use default import syntax to import 'ReactCountdown'  import/no-named-default

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/PreIdChecks.jsx
  16:6  warning  React Hook useEffect has a missing dependency: 'setChecksPassed'. Either include it or remove the dependency array. If 'setChecksPassed' changes too often, find the parent component that defines it and wrap that definition in useCallback  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/setup/LoopbackCheck.jsx
  132:34  error    Return values from promise executor functions cannot be read                                               no-promise-executor-return
  162:6   warning  React Hook useEffect has a missing dependency: 'player'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/cypress/e2e/01_Normal_Paths_Omnibus.js
  174:34  error  'actualOrder' is already declared in the upper scope on line 155 column 13  no-shadow
  201:31  error  'newOrder' is already declared in the upper scope on line 190 column 13     no-shadow

/home/runner/work/deliberation-empirica/deliberation-empirica/cypress/support/sharedSteps.js
  8:9  error  'log' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/getTreatments.js
    8:33  error  Missing file extension for "./preFlight/validateTreatmentFile"  import/extensions
  326:30  error  Unexpected 'await' inside a loop                                no-await-in-loop

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/index.js
  42:43  error  'p' is defined but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/postFlight/exportScienceData.js
  4:17  error  'warn' is defined but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/dispatch.js
  477:9  error  Unexpected use of continue statement  no-continue

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/preFlightChecks.js
  20:5  error  iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations  no-restricted-syntax

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/validateDlConfig.test.js
  2:32  error  Missing file extension for "./validateDlConfig"                                                                            import/extensions
  3:1   error  'vscode' import should occur before import of './validateDlConfig'                                                         import/order
  6:25  error  Unexpected block statement surrounding arrow body; parenthesize the returned value and move it immediately after the '=>'  arrow-body-style

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/validatePromptFile.test.js
  6:8  error  Missing file extension for "./validatePromptFile"  import/extensions

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/providers/dailyco.js
   37:23  error  Expected to return a value at the end of async function 'createRoom'     consistent-return
  157:23  error  Expected to return a value at the end of async function 'stopRecording'  consistent-return
  229:7   error  Unexpected if as the only statement in an else block                     no-lonely-if

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/utils/logging.js
  1:17  error  'warn' is defined but never used  no-unused-vars
  1:29  error  'log' is defined but never used   no-unused-vars

✖ 37 problems (28 errors, 9 warnings)
  3 errors and 0 warnings potentially fixable with the '--fix' option.

@JamesPHoughton

Copy link
Copy Markdown
Collaborator

There could be some unexpected behavior, actually, if one person has submitted, and the values are being computed and displayed before other participants (who will eventually submit) have a chance to do so. Can we think through the different edge cases here? It is possible that this will be used on the same page as the submission of the actual values. For example, if folks say "I'm ready to proceed" or "Not ready to proceed", if the initial values are all Undefined, and then one person submits "I'm ready to proceed" do we really want it to take that as consensus on proceeding? or should we just throw false when everything is undefined, but also include undefined responses in the denominator when looking for consensus?

@cypress

cypress Bot commented Aug 25, 2025

Copy link
Copy Markdown

Deliberation    Run #1691

Run Properties:  status check passed Passed #1691  •  git commit 5c3be53b13: Merge branch 'main' into copilot/fix-754
Project Deliberation
Branch Review copilot/fix-754
Run status status check passed Passed #1691
Run duration 06m 39s
Commit git commit 5c3be53b13: Merge branch 'main' into copilot/fix-754
Committer James Houghton
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 1
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 16
View all changes introduced in this branch ↗︎

@JamesPHoughton

Copy link
Copy Markdown
Collaborator

@copilot - can you make changes to account for the edge cases described above?

…or for consensus calculation

Co-authored-by: JamesPHoughton <4304478+JamesPHoughton@users.noreply.github.com>

Copilot AI commented Aug 26, 2025

Copy link
Copy Markdown
Contributor Author

@copilot - can you make changes to account for the edge cases described above?

I've addressed the edge case you identified. The fix changes the denominator calculation to use total participants instead of just responders, preventing false consensus when only a subset have responded.

Key change in commit 724e17c:

  • Changed from (maxCount / definedValues.length) * 100 to (maxCount / referenceValues.length) * 100

This ensures that if only 1 out of 4 people responds "ready", it shows 25% agreement rather than 100%, preventing premature consensus triggers. The logic still returns false when all values are undefined (no responses yet).

Copilot AI changed the title Fix percentAgreement to exclude undefined values from agreement calculation Fix percentAgreement to prevent false consensus when subset of participants respond Aug 26, 2025
@JamesPHoughton
JamesPHoughton marked this pull request as ready for review August 27, 2025 11:06
@github-actions

Copy link
Copy Markdown
/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/App.jsx
  171:7  error  Unknown property 'test-player-id' found  react/no-unknown-property

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/ConditionalRender.jsx
  39:10  error  'tickTock' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/HairCheck.jsx
  116:6  warning  React Hook useEffect has missing dependencies: 'onFailure', 'onVideoSuccess', and 'player'. Either include them or remove the dependency array. If 'onVideoSuccess' changes too often, find the parent component that defines it and wrap that definition in useCallback  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/IdleProvider.jsx
   56:6   warning  React Hook useEffect has a missing dependency: 'resetTimer'. Either include it or remove the dependency array  react-hooks/exhaustive-deps
  120:57  error    ''' can be escaped with '&apos;', '&lsquo;', '&#39;', '&rsquo;'                                                react/no-unescaped-entities

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/Markdown.jsx
  17:6  error  Unnecessary escape character: !  no-useless-escape

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/SharedNotepad.jsx
  26:6  warning  React Hook useEffect has missing dependencies: 'defaultText', 'game', and 'record'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/components/TextChat.jsx
   71:6  warning  React Hook useEffect has a missing dependency: 'msgs.length'. Either include it or remove the dependency array  react-hooks/exhaustive-deps
  182:7  warning  Unexpected alert                                                                                                no-alert
  219:7  error    A control must be associated with a text label                                                                  jsx-a11y/control-has-associated-label

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/KitchenTimer.jsx
  9:10  error  'tickTock' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/Prompt.jsx
  19:3  error  iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations  no-restricted-syntax

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/Qualtrics.jsx
  54:10  error  'state' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/elements/TrainingVideo.jsx
  61:6  warning  React Hook useEffect has missing dependencies: 'timer?.elapsed' and 'url'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/Consent.jsx
  129:6  warning  React Hook useEffect has a missing dependency: 'player'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/Countdown.jsx
  8:21  error  Use default import syntax to import 'ReactCountdown'  import/no-named-default

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/PreIdChecks.jsx
  16:6  warning  React Hook useEffect has a missing dependency: 'setChecksPassed'. Either include it or remove the dependency array. If 'setChecksPassed' changes too often, find the parent component that defines it and wrap that definition in useCallback  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/client/src/intro-exit/setup/LoopbackCheck.jsx
  132:34  error    Return values from promise executor functions cannot be read                                               no-promise-executor-return
  162:6   warning  React Hook useEffect has a missing dependency: 'player'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

/home/runner/work/deliberation-empirica/deliberation-empirica/cypress/e2e/01_Normal_Paths_Omnibus.js
  174:34  error  'actualOrder' is already declared in the upper scope on line 155 column 13  no-shadow
  201:31  error  'newOrder' is already declared in the upper scope on line 190 column 13     no-shadow

/home/runner/work/deliberation-empirica/deliberation-empirica/cypress/support/sharedSteps.js
  8:9  error  'log' is assigned a value but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/getTreatments.js
    8:33  error  Missing file extension for "./preFlight/validateTreatmentFile"  import/extensions
  326:30  error  Unexpected 'await' inside a loop                                no-await-in-loop

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/index.js
  42:43  error  'p' is defined but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/postFlight/exportScienceData.js
  4:17  error  'warn' is defined but never used  no-unused-vars

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/dispatch.js
  477:9  error  Unexpected use of continue statement  no-continue

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/preFlightChecks.js
  20:5  error  iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations  no-restricted-syntax

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/validateDlConfig.test.js
  2:32  error  Missing file extension for "./validateDlConfig"                                                                            import/extensions
  3:1   error  'vscode' import should occur before import of './validateDlConfig'                                                         import/order
  6:25  error  Unexpected block statement surrounding arrow body; parenthesize the returned value and move it immediately after the '=>'  arrow-body-style

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/preFlight/validatePromptFile.test.js
  6:8  error  Missing file extension for "./validatePromptFile"  import/extensions

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/providers/dailyco.js
   37:23  error  Expected to return a value at the end of async function 'createRoom'     consistent-return
  157:23  error  Expected to return a value at the end of async function 'stopRecording'  consistent-return
  229:7   error  Unexpected if as the only statement in an else block                     no-lonely-if

/home/runner/work/deliberation-empirica/deliberation-empirica/server/src/utils/logging.js
  1:17  error  'warn' is defined but never used  no-unused-vars
  1:29  error  'log' is defined but never used   no-unused-vars

✖ 37 problems (28 errors, 9 warnings)
  3 errors and 0 warnings potentially fixable with the '--fix' option.

@JamesPHoughton
JamesPHoughton merged commit 527af95 into main Aug 27, 2025
6 checks passed
@JamesPHoughton
JamesPHoughton deleted the copilot/fix-754 branch August 27, 2025 11:57
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.

percentAgreement should not return true when all values are undefined

2 participants