Apply fagerstrom form suggestions#18
Conversation
WalkthroughA single form definition for the Fagerstrom Nicotine Dependence assessment updates its scoring scheme by switching option score keys from positive to negative values for yes/no and time-based questions, adjusting question labels, and modifying score calculation and validation bounds to reflect the new scheme. ChangesFagerstrom Nicotine Dependence Form Scoring Revision
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
What's holding this up? |
|
@gdevenyi we use a hacky way of reordering the question options by making the scale negative instead (1 -> -1, 2 -> -2, ...). If this is ok then the pr is good to go. |
|
Why is this needed? Can you explain the underlying issue? |
|
Anne requests that the ordering of the questions options to be changed from lowest to highest score value to highest to lowest score value as seen in issue https://github.com/DouglasNeuroInformatics/CPP/issues/90. The design of ODC form does not allow for this ordering to be possible as it always sorts the options by numerical value, so the current makeshift solution is to make the values negative. A possible solution for this in the future is maybe a |
|
another workaround that could be done is inversing the scale values like done in this form here however this could be confusing comparing it the official form and its scoring method. |
|
I think its a bit silly to auto-sort these things when the display is based on this. @joshunrau can we look into adding the sort control feature and defaulting it to off? |
|
This is not a design in ODC sorting. In the ECMAScript spec, non-negative integer object keys are automatically sorted when iterating over them. An alternative here could be to use something like "1.0".
1. Integer indices, ascending numeric order 2. String keys, in insertion order 3. Symbol keys are NOT included Exampleconst obj = {};
obj["b"] = 1;
obj["2"] = 2;
obj["a"] = 3;
obj["0"] = 4;
obj["1"] = 5;
Object.keys(obj); // ["0", "1", "2", "b", "a"]Integer indices sort numerically first ( Important nuances
The practical implication: if you're using an object as a map with mixed numeric-string keys, your iteration order may surprise you if you added string keys before numeric ones. |
There was a problem hiding this comment.
Pull request overview
Updates the Fagerström Nicotine Dependence (FTND) instrument definition to address option ordering issues by changing option value encodings and adjusting the computed total score logic accordingly.
Changes:
- Updated several radio option value keys (including introducing negative values) to correct ordering behavior.
- Adjusted total score computation to account for negative-coded answers.
- Updated validation schema bounds to match the new value ranges and tweaked one English question label.
Comments suppressed due to low confidence (5)
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts:101
- The English label has an extra "a" ("How many cigarettes a do you smoke…"), which reads as a typo in the user-facing question text.
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts:115 - The French option text "31 ou plus 3" appears to have an extraneous trailing "3" in the user-facing label.
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts:150 - The total score calculation currently special-cases
cigaretteAmountand multiplies the remaining answers by -1. Since the schema now constrains the other answers to be <= 0, computing the score as the sum of absolute values is simpler and avoids depending on a specific field name.
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts:150 - This computed measure is named
auditCScore, which appears to be a copy/paste artifact from the AUDIT-C instrument and is misleading in the Fagerström instrument. Consider renaming it to something FTND-specific (or adding a new correctly-named measure while keepingauditCScoreas a backwards-compatible alias if consumers rely on it).
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts:62 - This French option label has a trailing space, which can show up in UI rendering and makes the string inconsistent with the other options.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts (1)
142-149: ⚡ Quick winMeasure name
auditCScoreis misleading.This is the Fagerström Nicotine Dependence test, not AUDIT-C. Consider renaming to
ftndScoreortotalScorefor clarity.Proposed fix
measures: { - auditCScore: { + ftndScore: { kind: 'computed', label: { en: 'Total Score:', fr: 'Score total:' }, value: (data) => sum(Object.values(data).map((v) => Math.abs(v ?? 0))) } },🤖 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 `@lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts` around lines 142 - 149, Rename the misleading computed field auditCScore to a clear name like ftndScore (or totalScore) in the form definition: change the property key auditCScore to ftndScore (and keep its kind, label, and value implementation sum(Object.values(data).map((v) => Math.abs(v ?? 0)))); then update all references/usages, imports, exports and any tests or type interfaces that expect auditCScore to use ftndScore instead so callers read the new property name consistently (ensure no runtime references remain to auditCScore).
🤖 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 `@lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts`:
- Line 115: The French label for option key '3' in the
FAGERSTRÖM_NICOTINE_DEPENDENCE options map contains a stray "3" ("31 ou plus
3"); locate the options object in
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts and change the value for key
'3' from "31 ou plus 3" to "31 ou plus" so the label reads correctly.
- Line 101: Fix the typo in the FAGERSTRÖM_NICOTINE_DEPENDENCE question text:
replace the string "4. How many cigarettes a do you smoke per day?" (the en
value for that question) with "4. How many cigarettes do you smoke per day?" so
the English prompt reads correctly.
---
Nitpick comments:
In `@lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts`:
- Around line 142-149: Rename the misleading computed field auditCScore to a
clear name like ftndScore (or totalScore) in the form definition: change the
property key auditCScore to ftndScore (and keep its kind, label, and value
implementation sum(Object.values(data).map((v) => Math.abs(v ?? 0)))); then
update all references/usages, imports, exports and any tests or type interfaces
that expect auditCScore to use ftndScore instead so callers read the new
property name consistently (ensure no runtime references remain to auditCScore).
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3f15aea-a056-4264-8e77-264f165eecec
📒 Files selected for processing (1)
lib/forms/FAGERSTRÖM_NICOTINE_DEPENDENCE/index.ts
Change the scale and total score calculation to fix order of options within multiple questions.
Questions appear as negative values, get converted to numbers and multiplied by -1
https://github.com/DouglasNeuroInformatics/CPP/issues/90
form can be viewed here
Summary by CodeRabbit
Release Notes