262 add rob 2#275
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds comprehensive ROB-2 (Risk of Bias 2) checklist support to the CoRATES platform, including schema definitions, deterministic domain-based auto-scoring, creation utilities, web UI components for interactive assessment, and Yjs-backed collaborative editing. Integrates ROB-2 into the existing checklist registry and hooks. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as ROB2 Checklist UI
participant ScoringEngine as Scoring Engine
participant ChecklistState as Checklist State
participant YjsDoc as Yjs Document
participant RemoteUser as Remote User
User->>UI: Select answer (Y/PY/PN/N/NI)
UI->>ChecklistState: Update question answer
ChecklistState->>ScoringEngine: scoreRob2Domain(domainKey, answers)
ScoringEngine->>ChecklistState: Return ScoringResult (judgement, ruleId)
ChecklistState->>YjsDoc: Sync domain state + auto-judgement
YjsDoc->>RemoteUser: Broadcast domain update
UI->>UI: Re-render with auto-score badge
RemoteUser->>UI: Receive and apply Yjs update
UI->>UI: Display updated judgement & domain status
sequenceDiagram
actor User
participant UI as ROB2 Checklist
participant Preliminary as PreliminarySection
participant DomainS as DomainSection
participant Overall as OverallSection
participant ScoringEngine as Scoring Engine
User->>Preliminary: Select aim (ASSIGNMENT or ADHERING)
Preliminary->>UI: Call onUpdate({preliminary: {..., aim}})
UI->>ScoringEngine: getActiveDomainKeys(isAdhering)
ScoringEngine->>UI: Return active domains
UI->>DomainS: Render active domains based on aim
User->>DomainS: Answer domain questions
DomainS->>ScoringEngine: scoreRob2Domain for each question
ScoringEngine->>DomainS: Auto-calculated judgement
DomainS->>UI: Call onUpdate({domain: {..., judgement, direction}})
UI->>ScoringEngine: scoreAllDomains(checklistState)
ScoringEngine->>Overall: Return overall score & isComplete
Overall->>UI: Render overall judgement & score badge
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Rationale: The changes span 40+ files with high heterogeneity—new deterministic scoring engine (677 lines with multi-branching domain logic), complete UI component hierarchy (7 components, 1,100+ lines), Yjs handler with Y.Map/Y.Text serialization (312 lines), comprehensive schema definitions (403 lines), decision diagrams (900+ lines of documentation), and integration across registry, types, and hooks. While many UI components follow repetitive patterns (sections, buttons), the scoring logic requires careful validation of decision tree branches across 5 domains, and the Yjs handler demands understanding of concurrent-edit semantics. Test coverage (804 lines) partially mitigates review burden but scoring edge cases (NA→NI normalization, incomplete handling, domain filtering) warrant close inspection. Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/src/components/checklist/ChecklistWithPdf.jsx (1)
25-28: Add missinggetRob2Textto ChecklistWithPdf.jsx prop documentation and update GenericChecklist.jsx JSDoc.The prop is correctly wired—GenericChecklist receives and passes
getRob2Textto ROB2Checklist. However, both files have documentation gaps:
- ChecklistWithPdf.jsx (lines 25-28): Add
// props.getRob2Text - function to get Y.Text for a ROB-2 free-text field- GenericChecklist.jsx (JSDoc lines 28-29): Add
@param {Function} [props.getRob2Text] - Function to get Y.Text for a ROB-2 free-text fieldto match the ROBINS-I patternAlso applies to: 48-49
packages/web/src/components/checklist/ChecklistYjsWrapper.jsx (1)
444-449: Tooltip message doesn't account for ROB2 checklist type.The disabled button tooltip only handles
ROBINS_Ibut notROB2, even though both use domain-based completion.🐛 Suggested fix
title={ !isChecklistValid() ? - checklistType() === 'ROBINS_I' ? + checklistType() === 'ROBINS_I' || checklistType() === 'ROB2' ? 'Overall risk of bias must be set before marking complete' : 'All questions must have a final answer before marking complete' : undefined }
🤖 Fix all issues with AI agents
In @.claude/settings.json:
- Around line 4-5: Remove the personal Claude settings file from the repo by
unstaging and deleting .claude/settings.json from the commit/index and
committing that removal, then prevent future commits by adding the .claude/
directory to .gitignore and committing the updated .gitignore; ensure no
functional code references .claude/settings.json remain in the PR before pushing
the updated commit.
In @packages/shared/package.json:
- Around line 21-29: The package.json currently exposes entrypoints under
"./dist/..." but has no publish-time build step; add a prepublishOnly script
named "prepublishOnly" in packages/shared/package.json that runs your build
command (e.g., "pnpm build" or the repo's existing build script) so the dist/
artifacts are generated before npm publish and the exported paths like
"./checklists" and "./checklists/rob2" (pointing to ./dist/checklists/index.js
and ./dist/checklists/rob2/index.js) will exist in the published package.
In @packages/shared/src/checklists/rob2/create.ts:
- Around line 142-147: The ternary for the direction property is dead code:
"direction: domain?.hasDirection ? null : null" always yields null; fix by
either making the intent explicit (set direction: null) or make direction
conditional on domain?.hasDirection (e.g., only include the direction property
when domain?.hasDirection is true or return undefined when false). Update the
return in create.ts (the object with answers, judgement, direction) to remove
the redundant ternary and implement one of those two behaviors so direction's
presence correctly reflects domain?.hasDirection.
In @packages/web/src/components/checklist/ROB2Checklist/PreliminarySection.jsx:
- Around line 56-58: PreliminarySection is calling getRob2Text with only
section/field keys which misaligns parameters; update the three helpers
experimentalYText, comparatorYText, and numericalResultYText to pass
props.studyId and props.checklistId as the first two arguments before the
section ('preliminary') and field keys, and ensure the parent component passes
studyId and checklistId through props to this component so getRob2Text(studyId,
checklistId, sectionKey, fieldKey, ...) receives correct values.
In @packages/web/src/components/checklist/ROB2Checklist/ROB2Checklist.jsx:
- Around line 54-67: handleDomainClick uses setTimeout without clearing it,
which can fire after unmount and access DOM causing errors; store the timeout ID
(e.g., in a ref like domainScrollTimeoutRef) when calling setTimeout inside
handleDomainClick, clear any existing timeout before setting a new one, and add
a cleanup that calls clearTimeout(domainScrollTimeoutRef.current) when the
component unmounts (via useEffect return or onCleanup) so the callback cannot
run after unmount; update references to setCollapsedDomains and
document.getElementById usage remain unchanged.
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/domain-3.md:
- Line 58: Remove the stray page-number artifact "15" that appears as a
standalone line in the markdown content for the Decision Diagrams domain (the
lone "15" visible in domain-3.md) so the document contains only intended
headings and body text.
- Around line 108-132: The Mermaid flowchart is not enclosed in a fenced mermaid
code block so it won't render; wrap the entire diagram (starting with "flowchart
LR" and including nodes like Q31, Q32, Q33, Q34 and labels L/M/H) inside a
triple-backtick fence with "mermaid" (```mermaid ... ```), preserving all lines
and arrows exactly so it renders correctly.
- Line 8: Update phrasing to use consistent hyphenation: change “intention to
treat” to “intention-to-treat” and “follow up” to “follow-up” in this document;
specifically replace the sentence fragment shown ("The appropriate study
population for an analysis of the intention to treat effect is all randomized")
and the other occurrences referenced (lines containing the same phrases at the
other two locations) so all three instances use “intention-to-treat” and
“follow-up” for readability/consistency.
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/domain-4.md:
- Around line 76-78: Update the displayed judgement ordering in domain-4.md so
the labels read "Low / Some concerns / High" instead of "Low / High / Some
concerns"; locate the string "Risk-of-bias judgement See algorithm. Low / High /
Some concerns" and reorder the labels to "Risk-of-bias judgement See algorithm.
Low / Some concerns / High" (ensure any related usages of these labels in this
file follow the same normalized order).
- Around line 94-137: The Mermaid flowchart block is not fenced with a
```mermaid code block so it won't render; wrap the entire diagram (starting with
"flowchart LR" through the final node lines such as Q45b transitions) in a
fenced code block using ```mermaid at the start and ``` at the end, ensuring the
content including nodes like Q41, Q42, Q43a/Q43b and edges (e.g., Q41 -- "Y/PY"
--> H, Q45b -- "Y/PY/NI" --> H) remain unchanged.
- Line 58: A stray page-number artifact "18" was left in domain-4.md; remove
that standalone "18" entry (the orphan line showing just 18) from
packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/domain-4.md
and verify there are no other isolated page-number residues in that file.
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/domain-5.md:
- Around line 137-152: The Mermaid flowchart block (starting with nodes Q52 and
Q51 and arrows to L/M/H) must be fenced so it renders; wrap the entire diagram
text in a triple-backtick fenced code block that begins with "mermaid" and ends
with "```", ensure you include the full diagram (lines containing Q52, Q51,
L/M/H and the arrow lines like Q52 -- "Both N/PN" --> Q51 and Q51 -- "Y/PY" -->
L) inside the fence, and remove any stray diff markers (e.g. leading '+'
characters) so the fenced mermaid block is clean.
- Around line 119-120: Update the judgement label ordering in the string
"Risk-of-bias judgement See algorithm. Low / High / Some concerns" to read
"Risk-of-bias judgement See algorithm. Low / Some concerns / High" so the labels
follow the L / M / H sequence used by the diagram and ROB-2 convention; locate
and replace that exact text token in the domain-5.md content.
- Line 54: Remove the stray PDF page-number artifacts ("21" and "22") from the
markdown: locate standalone lines containing just "21" (e.g., the occurrence in
domain-5.md around the reported spot) and "22" (the other reported spot) and
delete those lines so the document contains only actual content; ensure no other
inline occurrences are removed unless they are likewise standalone page-number
artifacts.
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/overall.md:
- Around line 22-23: The sentence fragment "Or The study is judged to have" in
overall.md is incomplete; replace it with the full ROB-2 alternative for high
overall risk, e.g. "Or the study is judged to have a high risk of bias for at
least one domain, or multiple domains with some concerns that, in combination,
lead to an overall high risk of bias." Update the line containing that fragment
so the documentation reads as a complete criterion consistent with ROB-2
guidance.
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/preliminary.md:
- Line 23: Remove the stray page-number artifact by deleting the standalone "3"
on the file preliminary.md (the orphaned line containing only "3" at the top of
the decision-diagrams section); ensure no other PDF-extraction artifacts remain
and that surrounding Markdown formatting and headings remain unchanged.
In @packages/web/src/components/checklist/ROB2Checklist/ScoringSummary.jsx:
- Around line 121-131: The domain chip buttons currently remain interactive even
when props.onDomainClick is not provided; update the button inside the For loop
(in ScoringSummary.jsx) to pass disabled={!props.onDomainClick} and apply a
disabled style variant (e.g., muted colors/removed hover via getDomainChipColor
or an additional conditional class) so the button visually and functionally
reflects non-interactivity; ensure the onClick remains wrapped with optional
chaining (props.onDomainClick?.(domainKey)) and that
getDomainShortName/ROB2_CHECKLIST usage is unchanged.
🧹 Nitpick comments (10)
packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/preliminary.md (1)
1-37: Consider adding official ROB-2 reference and verifying Markdown formatting consistency.This guidance document is well-structured, but two improvements would strengthen its utility:
Documentation reference: Consider adding a reference link to the official ROB-2 publication/guidance (e.g., "See ROB-2 Guidance for full details") to ensure assessors can access authoritative source material when needed.
Markdown formatting: The document uses inconsistent spacing for list items. Consider standardizing to proper Markdown syntax (e.g.,
- Study design optionor- [ ] Checkbox option) for better rendering and consistency across the codebase.This ensures the guidance integrates smoothly with the UI components consuming ROB-2 preliminary state data and remains maintainable as ROB-2 standards evolve.
packages/web/src/components/checklist/ROB2Checklist/ScoringSummary.jsx (1)
3-4: Consider using @/ import aliases instead of relative imports for consistency.This aligns with the
packages/web/**/*.{js,jsx,ts,tsx}alias-import guideline.packages/web/src/checklist-registry/index.js (1)
107-110: Domain2a/domain2b are always initialized in ROB2 checklists—current detection logic is sound.The
createROB2Checklistfunction always initializes bothdomain2aanddomain2bas DomainState objects (never null/undefined). The detection logic ingetChecklistTypeFromStatecorrectly uses these fields as ROB2 discriminators, and there is no risk of missing fields in newly created ROB2 checklists.However, consider using the
preliminaryfield as a more explicit discriminator, as it is unique to ROB2 and semantically clearer:// Detect ROB-2 by unique preliminary section if (checklistState?.preliminary) { return CHECKLIST_TYPES.ROB2; }The current implementation is correct; this is an optional refactoring for improved clarity.
packages/web/src/components/checklist/ROB2Checklist/DomainSection.jsx (1)
71-78: Consider memoizingcompletionStatusto avoid repeated computation.
completionStatus()is called twice in the render (lines 97-98). While not a performance issue for small question sets, consider usingcreateMemofor consistency with other derived values in this component.♻️ Suggested refactor
- // Get completion status - const completionStatus = () => { + // Get completion status + const completionStatus = createMemo(() => { const qs = questions(); const answered = Object.keys(qs).filter( k => props.domainState?.answers?.[k]?.answer !== null, ).length; const total = Object.keys(qs).length; return { answered, total }; - }; + });Also applies to: 97-98
packages/web/src/components/checklist/ROB2Checklist/OverallSection.jsx (1)
19-31: Simplify redundant memos and consider memoizingcalculatedScore.
effectiveJudgement(lines 29-31) simply returnscalculatedDisplayJudgement()- this indirection adds no value. Also,calculatedScore(line 20) accesses a reactive memo but isn't itself memoized.♻️ Suggested simplification
- // Calculated overall score - const calculatedScore = () => smartScoring().overall; + // Calculated overall score + const calculatedScore = createMemo(() => smartScoring().overall); // Calculated overall in display format const calculatedDisplayJudgement = createMemo(() => { const score = calculatedScore(); return mapOverallJudgementToDisplay(score); }); - // Effective judgement: use calculated value - const effectiveJudgement = createMemo(() => { - return calculatedDisplayJudgement(); - });Then replace
effectiveJudgement()withcalculatedDisplayJudgement()throughout the component.packages/web/src/components/checklist/ROB2Checklist/SignallingQuestion.jsx (2)
21-29: NA-to-NI coercion via effect may cause confusion; consider normalizing at data ingestion.This effect silently mutates incoming NA answers to NI when NA isn't a valid option. While the logic prevents infinite loops, it creates an implicit side effect that may surprise users or cause unexpected state updates. Consider documenting this behavior or normalizing answers when data is first loaded/created rather than during render.
85-95: UseShowcomponent for conditional rendering per project guidelines.Per coding guidelines, prefer Solid's
Showcomponent over&&operators for conditional rendering.♻️ Suggested fix
{/* Comment field (optional) */} - {props.showComment && ( + <Show when={props.showComment}> <div class='mt-2'> <NoteEditor yText={commentYText()} placeholder='Comment (optional)' readOnly={props.disabled} inline={true} /> </div> - )} + </Show>packages/web/src/components/checklist/ROB2Checklist/DomainJudgement.jsx (1)
45-62: Consider extracting shared color utility.The
getJudgementColorfunction here is similar to the one inOverallSection.jsx(lines 40-57). Consider extracting a shared utility to reduce duplication, though the slight differences in behavior (auto mode cursor handling) may justify keeping them separate.packages/shared/src/checklists/types.ts (1)
212-225: DuplicateROB2Checklistinterface definition detected.This interface is also defined in
packages/shared/src/checklists/rob2/create.ts(lines 13-43). Having two separate interface definitions risks them diverging over time. Consider exporting the interface from one location and importing it in the other.♻️ Suggested approach
Define
ROB2Checklistintypes.tsonly (the canonical type definitions file) and import it increate.ts:// In packages/shared/src/checklists/rob2/create.ts +import type { ROB2Checklist } from '../types.js'; -export interface ROB2Checklist { - // ... duplicate definition -}Or vice versa - define in
create.tsand re-export fromtypes.ts.packages/shared/src/checklists/rob2/answers.ts (1)
14-26: Type assertion suggests interface mismatch with scoring module.The repeated
as unknown as Parameters<typeof scoreAllDomains>[0]casts (lines 17-19 and 170-172) indicate thatROB2Checklistand the expectedChecklistStateparameter type inscoreAllDomainsmay not be aligned. This could mask type errors.Consider ensuring
ROB2Checklistsatisfies theChecklistStatetype expected byscoreAllDomains, or export a proper type that both modules share, to eliminate the need forunknowncasts.
| "plugin-dev@claude-plugins-official": true, | ||
| "feature-dev@claude-plugins-official": true |
There was a problem hiding this comment.
Remove personal Claude AI configuration from version control.
This file appears to be a personal Claude AI assistant configuration that was accidentally committed. The .claude/settings.json file and directory are typically local development tool settings and should not be tracked in version control, as they:
- Are unrelated to the PR's ROB-2 functionality
- Could force personal Claude plugin preferences on all developers
- Serve no functional purpose for the codebase
🗑️ Recommended actions
- Remove this file from the commit
- Add
.claude/to.gitignoreto prevent future accidental commits
# Add to .gitignore
echo ".claude/" >> .gitignore🤖 Prompt for AI Agents
In @.claude/settings.json around lines 4 - 5, Remove the personal Claude
settings file from the repo by unstaging and deleting .claude/settings.json from
the commit/index and committing that removal, then prevent future commits by
adding the .claude/ directory to .gitignore and committing the updated
.gitignore; ensure no functional code references .claude/settings.json remain in
the PR before pushing the updated commit.
| }, | ||
| "./checklists": { | ||
| "types": "./dist/checklists/index.d.ts", | ||
| "import": "./dist/checklists/index.js" | ||
| }, | ||
| "./checklists/rob2": { | ||
| "types": "./dist/checklists/rob2/index.d.ts", | ||
| "import": "./dist/checklists/rob2/index.js" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the *source* entrypoints exist (build should emit corresponding dist files)
echo "=== Checking for index.ts in checklists directories ==="
fd -a '^index\.ts$' packages/shared/src/checklists packages/shared/src/checklists/rob2 2>/dev/null || echo "No matches found"
echo ""
echo "=== Inspecting tsconfig.json for packages/shared ==="
fd -a '^tsconfig\.json$' packages/shared -x cat
echo ""
echo "=== Checking exports keys in package.json ==="
cat packages/shared/package.json | jq '.exports | keys' 2>/dev/null || echo "Could not parse exports"Repository: InfinityBowman/corates
Length of output: 1238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Complete exports object from package.json ==="
cat packages/shared/package.json | jq '.exports'
echo ""
echo "=== Check for files field in package.json ==="
cat packages/shared/package.json | jq '.files' || echo "No files field"
echo ""
echo "=== Check for .npmignore ==="
if [ -f packages/shared/.npmignore ]; then
cat packages/shared/.npmignore
else
echo ".npmignore not found"
fi
echo ""
echo "=== Verify dist directory is tracked ==="
fd -a '^dist$' packages/shared --type d || echo "No dist directory found"
</parameter>Repository: InfinityBowman/corates
Length of output: 869
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check .gitignore for dist exclusion ==="
if [ -f .gitignore ]; then
cat .gitignore | grep -E "dist|/dist" || echo "No dist patterns in root .gitignore"
else
echo "No .gitignore found"
fi
echo ""
echo "=== Check packages/shared/.gitignore ==="
if [ -f packages/shared/.gitignore ]; then
cat packages/shared/.gitignore
else
echo "No .gitignore in packages/shared"
fi
echo ""
echo "=== Check for build/prepare scripts in package.json ==="
cat packages/shared/package.json | jq '.scripts | select(.build != null or .prepare != null or .prepublishOnly != null)'Repository: InfinityBowman/corates
Length of output: 452
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check root package.json for publish/prepare scripts ==="
cat package.json | jq '.scripts | with_entries(select(.key | test("publish|prepare|build")))' 2>/dev/null || echo "No matching scripts"
echo ""
echo "=== Check root package.json workspaces ==="
cat package.json | jq '.workspaces, .packageManager' 2>/dev/null || echo "No workspace config found"Repository: InfinityBowman/corates
Length of output: 386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for publish workflows ==="
fd -e yml -e yaml . .github/workflows 2>/dev/null | head -20
echo ""
echo "=== Check if .npmrc or pnpm publish config exists ==="
if [ -f .npmrc ]; then
cat .npmrc
fi
if [ -f pnpm-workspace.yaml ]; then
cat pnpm-workspace.yaml | head -50
fi
echo ""
echo "=== Check packages/shared/package.json for prepublishOnly ==="
cat packages/shared/package.json | jq 'if .scripts.prepublishOnly then . else "NO prepublishOnly SCRIPT" end'Repository: InfinityBowman/corates
Length of output: 416
Add a prepublishOnly script to build dist/ before publishing, or the exported entrypoints will fail at runtime.
The source files exist and TypeScript is properly configured, but dist/ is gitignored and has no build step before npm publish. Without a prepublishOnly script in packages/shared/package.json, the exported paths (./dist/checklists/index.js, ./dist/checklists/rob2/index.js) won't exist in the published artifact, causing 404 errors for consumers.
Add to packages/shared/package.json:
"scripts": {
"prepublishOnly": "pnpm build"
}🤖 Prompt for AI Agents
In @packages/shared/package.json around lines 21 - 29, The package.json
currently exposes entrypoints under "./dist/..." but has no publish-time build
step; add a prepublishOnly script named "prepublishOnly" in
packages/shared/package.json that runs your build command (e.g., "pnpm build" or
the repo's existing build script) so the dist/ artifacts are generated before
npm publish and the exported paths like "./checklists" and "./checklists/rob2"
(pointing to ./dist/checklists/index.js and ./dist/checklists/rob2/index.js)
will exist in the published package.
| return { | ||
| answers, | ||
| judgement: null, | ||
| direction: domain?.hasDirection ? null : null, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Dead code: ternary always returns null.
Line 145 has direction: domain?.hasDirection ? null : null which always evaluates to null regardless of the condition. This appears to be a placeholder or oversight.
🐛 Suggested fix
If direction should always be null initially:
return {
answers,
judgement: null,
- direction: domain?.hasDirection ? null : null,
+ direction: null,
};Or if hasDirection should control whether direction exists:
return {
answers,
judgement: null,
- direction: domain?.hasDirection ? null : null,
+ ...(domain?.hasDirection && { direction: null }),
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { | |
| answers, | |
| judgement: null, | |
| direction: domain?.hasDirection ? null : null, | |
| }; | |
| } | |
| return { | |
| answers, | |
| judgement: null, | |
| direction: null, | |
| }; | |
| } |
| return { | |
| answers, | |
| judgement: null, | |
| direction: domain?.hasDirection ? null : null, | |
| }; | |
| } | |
| return { | |
| answers, | |
| judgement: null, | |
| ...(domain?.hasDirection && { direction: null }), | |
| }; | |
| } |
🤖 Prompt for AI Agents
In @packages/shared/src/checklists/rob2/create.ts around lines 142 - 147, The
ternary for the direction property is dead code: "direction:
domain?.hasDirection ? null : null" always yields null; fix by either making the
intent explicit (set direction: null) or make direction conditional on
domain?.hasDirection (e.g., only include the direction property when
domain?.hasDirection is true or return undefined when false). Update the return
in create.ts (the object with answers, judgement, direction) to remove the
redundant ternary and implement one of those two behaviors so direction's
presence correctly reflects domain?.hasDirection.
| const experimentalYText = () => props.getRob2Text?.('preliminary', 'experimental'); | ||
| const comparatorYText = () => props.getRob2Text?.('preliminary', 'comparator'); | ||
| const numericalResultYText = () => props.getRob2Text?.('preliminary', 'numericalResult'); |
There was a problem hiding this comment.
Critical: Missing required parameters in getRob2Text calls.
The getRob2Text function signature is:
getRob2Text(studyId, checklistId, sectionKey, fieldKey, questionKey = null)However, these calls only pass 2 arguments:
props.getRob2Text?.('preliminary', 'experimental')This causes studyId and checklistId to be incorrectly mapped, breaking Y.Text retrieval and preventing collaborative editing of preliminary section notes.
🔧 Proposed fix
The component needs to receive studyId and checklistId from props and pass them to getRob2Text:
+/**
+ * @param {string} props.studyId - The study ID
+ * @param {string} props.checklistId - The checklist ID
+ */
export function PreliminarySection(props) {
// ... handlers ...
- const experimentalYText = () => props.getRob2Text?.('preliminary', 'experimental');
- const comparatorYText = () => props.getRob2Text?.('preliminary', 'comparator');
- const numericalResultYText = () => props.getRob2Text?.('preliminary', 'numericalResult');
+ const experimentalYText = () => props.getRob2Text?.(props.studyId, props.checklistId, 'preliminary', 'experimental');
+ const comparatorYText = () => props.getRob2Text?.(props.studyId, props.checklistId, 'preliminary', 'comparator');
+ const numericalResultYText = () => props.getRob2Text?.(props.studyId, props.checklistId, 'preliminary', 'numericalResult');Ensure the parent component passes these props when rendering PreliminarySection.
🤖 Prompt for AI Agents
In @packages/web/src/components/checklist/ROB2Checklist/PreliminarySection.jsx
around lines 56 - 58, PreliminarySection is calling getRob2Text with only
section/field keys which misaligns parameters; update the three helpers
experimentalYText, comparatorYText, and numericalResultYText to pass
props.studyId and props.checklistId as the first two arguments before the
section ('preliminary') and field keys, and ensure the parent component passes
studyId and checklistId through props to this component so getRob2Text(studyId,
checklistId, sectionKey, fieldKey, ...) receives correct values.
| // Handle domain chip click from summary - expand and scroll to domain | ||
| function handleDomainClick(domainKey) { | ||
| // Expand the domain if collapsed | ||
| setCollapsedDomains(prev => ({ | ||
| ...prev, | ||
| [domainKey]: false, | ||
| })); | ||
|
|
||
| // Scroll to domain section after a short delay for DOM update | ||
| setTimeout(() => { | ||
| const element = document.getElementById(`domain-section-${domainKey}`); | ||
| element?.scrollIntoView({ behavior: 'smooth', block: 'start' }); | ||
| }, 100); | ||
| } |
There was a problem hiding this comment.
Missing cleanup for setTimeout in handleDomainClick.
If the component unmounts within the 100ms delay, the callback will still execute, potentially causing errors when accessing unmounted DOM elements. Consider storing the timeout ID and clearing it in onCleanup.
🐛 Suggested fix
+import { createSignal, For, Show, createMemo, onCleanup } from 'solid-js';
+
+// Inside component:
+const [scrollTimeoutId, setScrollTimeoutId] = createSignal(null);
+
+onCleanup(() => {
+ const id = scrollTimeoutId();
+ if (id) clearTimeout(id);
+});
function handleDomainClick(domainKey) {
setCollapsedDomains(prev => ({
...prev,
[domainKey]: false,
}));
- setTimeout(() => {
+ const id = setTimeout(() => {
const element = document.getElementById(`domain-section-${domainKey}`);
element?.scrollIntoView({ behavior: 'smooth', block: 'start' });
+ setScrollTimeoutId(null);
}, 100);
+ setScrollTimeoutId(id);
}🤖 Prompt for AI Agents
In @packages/web/src/components/checklist/ROB2Checklist/ROB2Checklist.jsx around
lines 54 - 67, handleDomainClick uses setTimeout without clearing it, which can
fire after unmount and access DOM causing errors; store the timeout ID (e.g., in
a ref like domainScrollTimeoutRef) when calling setTimeout inside
handleDomainClick, clear any existing timeout before setting a new one, and add
a cleanup that calls clearTimeout(domainScrollTimeoutRef.current) when the
component unmounts (via useEffect return or onCleanup) so the callback cannot
run after unmount; update references to setCollapsedDomains and
document.getElementById usage remain unchanged.
| Risk-of-bias judgement See algorithm. Low / High / Some | ||
| concerns |
There was a problem hiding this comment.
Normalize judgement label ordering (“Low / Some concerns / High”).
Current line reads “Low / High / Some concerns”, which is inconsistent with the L/M/H mapping used in the diagram and typical ROB-2 presentation.
🤖 Prompt for AI Agents
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/domain-5.md
around lines 119 - 120, Update the judgement label ordering in the string
"Risk-of-bias judgement See algorithm. Low / High / Some concerns" to read
"Risk-of-bias judgement See algorithm. Low / Some concerns / High" so the labels
follow the L / M / H sequence used by the diagram and ROB-2 convention; locate
and replace that exact text token in the domain-5.md content.
| flowchart LR | ||
| Q52["Result selected from…\n\n5.2 …multiple outcome measurements?\n\n5.3 …multiple analyses of the data?"] | ||
| Q51["5.1 Trial analysed in accordance with a pre-specified plan?"] | ||
|
|
||
| L["Low risk"] | ||
| M["Some concerns"] | ||
| H["High risk"] | ||
|
|
||
| %% From result selection questions | ||
| Q52 -- "Both N/PN" --> Q51 | ||
| Q52 -- "At least one NI,\nbut neither Y/PY" --> M | ||
| Q52 -- "Either Y/PY" --> H | ||
|
|
||
| %% From pre-specified analysis plan | ||
| Q51 -- "Y/PY" --> L | ||
| Q51 -- "N/PN/NI" --> M |
There was a problem hiding this comment.
Fence Mermaid diagram in a ```mermaid code block to ensure it renders.
Most markdown renderers only render Mermaid when explicitly fenced, otherwise users will see raw text.
Proposed fix
-flowchart LR
+```mermaid
+flowchart LR
Q52["Result selected from…\n\n5.2 …multiple outcome measurements?\n\n5.3 …multiple analyses of the data?"]
Q51["5.1 Trial analysed in accordance with a pre-specified plan?"]
@@
%% From pre-specified analysis plan
Q51 -- "Y/PY" --> L
Q51 -- "N/PN/NI" --> M
+```🤖 Prompt for AI Agents
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/domain-5.md
around lines 137 - 152, The Mermaid flowchart block (starting with nodes Q52 and
Q51 and arrows to L/M/H) must be fenced so it renders; wrap the entire diagram
text in a triple-backtick fenced code block that begins with "mermaid" and ends
with "```", ensure you include the full diagram (lines containing Q52, Q51,
L/M/H and the arrow lines like Q52 -- "Both N/PN" --> Q51 and Q51 -- "Y/PY" -->
L) inside the fence, and remove any stray diff markers (e.g. leading '+'
characters) so the fenced mermaid block is clean.
| Or | ||
| The study is judged to have |
There was a problem hiding this comment.
Complete the incomplete sentence.
The documentation ends with an incomplete sentence fragment: "Or The study is judged to have". This appears to be cut off and should be completed to properly document the high risk of bias criteria.
📝 Likely completion based on ROB-2 guidance
The typical ROB-2 overall guidance includes an alternative criterion for "High risk":
Or
-The study is judged to have
+The study is judged to have some concerns for multiple domains in a way that substantially lowers confidence in the result.Would you like me to verify the official ROB-2 documentation to confirm the exact wording?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Or | |
| The study is judged to have | |
| Or | |
| The study is judged to have some concerns for multiple domains in a way that substantially lowers confidence in the result. |
🤖 Prompt for AI Agents
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/overall.md
around lines 22 - 23, The sentence fragment "Or The study is judged to have" in
overall.md is incomplete; replace it with the full ROB-2 alternative for high
overall risk, e.g. "Or the study is judged to have a high risk of bias for at
least one domain, or multiple domains with some concerns that, in combination,
lead to an overall high risk of bias." Update the line containing that fragment
so the documentation reads as a complete criterion consistent with ROB-2
guidance.
| failures in implementing the intervention that could have affected the outcome | ||
| non-adherence to their assigned intervention by trial participants | ||
|
|
||
| 3 |
There was a problem hiding this comment.
Remove stray page number artifact.
Line 23 contains a standalone "3" which appears to be a leftover from PDF extraction and should be removed.
🧹 Proposed fix
non-adherence to their assigned intervention by trial participants
-3
-
Which of the following sources were obtained to help inform the risk-of-bias assessment? (tick as many as apply)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 3 | |
| non-adherence to their assigned intervention by trial participants | |
| Which of the following sources were obtained to help inform the risk-of-bias assessment? (tick as many as apply) |
🤖 Prompt for AI Agents
In
@packages/web/src/components/checklist/ROB2Checklist/scoring/decision-diagrams/preliminary.md
at line 23, Remove the stray page-number artifact by deleting the standalone "3"
on the file preliminary.md (the orphaned line containing only "3" at the top of
the decision-diagrams section); ensure no other PDF-extraction artifacts remain
and that surrounding Markdown formatting and headings remain unchanged.
| <For each={activeDomains()}> | ||
| {domainKey => ( | ||
| <button | ||
| type='button' | ||
| onClick={() => props.onDomainClick?.(domainKey)} | ||
| title={`${ROB2_CHECKLIST[domainKey]?.name}: ${getDomainStatusText(domainKey)}`} | ||
| class={`inline-flex items-center gap-1 rounded border px-2 py-0.5 text-xs font-medium transition-colors hover:opacity-80 ${getDomainChipColor(domainKey)}`} | ||
| > | ||
| <span>{getDomainShortName(domainKey)}</span> | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
If onDomainClick is optional, consider disabling the domain chips when it’s not provided.
Right now the chips render as interactive buttons even when clicking does nothing. A simple disabled={!props.onDomainClick} + disabled styles would avoid confusion.
🤖 Prompt for AI Agents
In @packages/web/src/components/checklist/ROB2Checklist/ScoringSummary.jsx
around lines 121 - 131, The domain chip buttons currently remain interactive
even when props.onDomainClick is not provided; update the button inside the For
loop (in ScoringSummary.jsx) to pass disabled={!props.onDomainClick} and apply a
disabled style variant (e.g., muted colors/removed hover via getDomainChipColor
or an additional conditional class) so the button visually and functionally
reflects non-interactivity; ensure the onClick remains wrapped with optional
chaining (props.onDomainClick?.(domainKey)) and that
getDomainShortName/ROB2_CHECKLIST usage is unchanged.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | 2e66983 | Commit Preview URL | Jan 11 2026, 08:07 AM |
2 participants
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.