263 add rob 2 reconcile#304
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a ROB‑2 checklist comparison engine and a complete reconciliation UI (multi‑page editor, navbar/progress utilities, PDF split view). Exposes comparison APIs in the shared package and implements per‑item pages, panels, navbar utilities, and summary/save/reset flows in the web app. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ROB2Reconciliation
participant Compare as ComparisonEngine
participant Store as NavStore/LocalStorage
participant PDF as EmbedPdfViewer
User->>UI: Load reconciliation (checklist1, checklist2, pdfData)
UI->>Compare: compareChecklists(checklist1, checklist2)
Compare-->>UI: ComparisonResult
UI->>Store: init navItems, viewMode, currentPage
UI-->>User: Render current page
User->>UI: Edit final answer or "Use This"
UI->>Store: update finalAnswers & persist
UI->>Compare: recompute (scores/summary)
UI-->>User: Update UI badges/predictions
User->>UI: Toggle/select PDF
UI->>PDF: load/select pdfData
PDF-->>UI: render PDF panel
User->>UI: Click Save
UI->>UI: validate items
UI-->>User: prompt & confirm
UI->>User: call onSaveReconciled(reconciledChecklist)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting 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 |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ⛔ Deployment terminated View logs |
corates | d4a10a1 | Commit Preview URL | Jan 17 2026, 06:26 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/shared/src/checklists/rob2/compare.ts`:
- Around line 205-223: The current comparison logic in the loop (using
questionKeys, domain1, domain2, and building the comparison object with
isAgreement) treats two missing answers (both ans1 and ans2 null) as an
agreement; change the isAgreement calculation to only be true when both answers
are present and equal (e.g., isAgreement = ans1 != null && ans2 != null && ans1
=== ans2) so missing answers are not counted as agreements, and ensure
downstream pushes to agreements use that updated flag.
In
`@packages/web/src/components/project/reconcile-tab/rob2-reconcile/ROB2Reconciliation.jsx`:
- Around line 119-133: Clamp the persisted current page index whenever nav items
change by adding a reactive effect that reads the stored/currentPage value,
computes a clamped index between 0 and totalPages()-1, and writes it back via
the currentPage setter; then resync the expanded domain to the new
navItems()[clampedIndex] (use navItems(), totalPages(), setCurrentPage and
setExpandedDomain inside a createEffect or equivalent reactive watcher). Ensure
the effect runs when navItems() or props.reconciledChecklist (if expanded domain
depends on it) change so currentNavItem() cannot become undefined after aim
changes or stale localStorage.
In
`@packages/web/src/components/project/reconcile-tab/rob2-reconcile/ROB2ReconciliationWithPdf.jsx`:
- Around line 59-87: headerContent is a static JSX value so
props.reviewer1Name/props.reviewer2Name and navbarStore.navItems are captured
once and won’t update; change headerContent into a reactive getter (e.g., const
headerContent = () => ( ... )) or inline the JSX inside the component return so
it reads reactive props and store values, and wherever headerContent is used
call headerContent() (or pass the function if SplitScreenLayout expects a
function); keep the same structure and component references (ROB2Navbar, Show,
navbarStore.navItems, props.reviewer1Name/props.reviewer2Name) so reactivity
works.
🧹 Nitpick comments (10)
packages/web/src/components/project/reconcile-tab/amstar2-reconcile/AnswerPanel.jsx (1)
193-202: Inconsistent radio button naming pattern between views.The full view uses
${props.title}-${props.questionKey}-finalfor radio names (line 195), while the compact view uses a more robust pattern withpanelIdfallback:${props.panelId || props.title || 'panel'}-...(line 91).Consider using the same fallback pattern for consistency and to avoid potential
undefinedin the name ifprops.titleis not provided.Suggested fix
<input type='radio' - name={`${props.title}-${props.questionKey}-final`} + name={`${props.panelId || props.title || 'panel'}-${props.questionKey}-final`} checked={isChecked()} disabled={props.readOnly}packages/web/src/components/project/reconcile-tab/rob2-reconcile/panels/DirectionPanel.jsx (1)
31-93: Drop narrating section comments.Comments like "Panel Header", "Direction Options", and "Selected Badge" restate the JSX structure; consider removing or replacing with intent-focused notes. As per coding guidelines, comments should explain why, not what.
packages/web/src/components/project/reconcile-tab/robins-i-reconcile/NavbarDomainPill.jsx (2)
83-87: Use Show instead of a ternary for the expand/collapse icon.This is conditional rendering, so a Show keeps SolidJS patterns consistent and avoids ternary usage in JSX. As per coding guidelines, use Show for conditional rendering.
Proposed change
- <Show when={props.sectionKey !== 'overall'}> - {props.isExpanded ? - <FiChevronDown class='h-3 w-3 opacity-60' /> - : <FiChevronRight class='h-3 w-3 opacity-60' />} - </Show> + <Show when={props.sectionKey !== 'overall'}> + <Show + when={props.isExpanded} + fallback={<FiChevronRight class='h-3 w-3 opacity-60' />} + > + <FiChevronDown class='h-3 w-3 opacity-60' /> + </Show> + </Show>
33-59: Avoid section comments that restate structure.Comments like "Container style" and "Label button style" narrate what the code does; consider removing or rewriting to capture intent. As per coding guidelines, comments should explain why, not what.
packages/web/src/components/project/reconcile-tab/rob2-reconcile/pages/SignallingQuestionPage.jsx (1)
42-83: Remove narrating section comments.The "Header", "Info text", and "Three-column comparison" comments restate the JSX structure; consider removing or converting to intent-focused notes. As per coding guidelines, comments should explain why, not what.
packages/web/src/components/project/reconcile-tab/rob2-reconcile/pages/DomainDirectionPage.jsx (1)
46-117: Keep section comments intent-focused.Labels like "Header", "Auto-calculated Judgement Section", and "Direction Section" describe structure rather than intent; consider removing or rewriting. As per coding guidelines, comments should explain why, not what.
packages/web/src/components/project/reconcile-tab/rob2-reconcile/pages/PreliminaryPage.jsx (1)
243-262: Consider memoizing derived values for better performance.The
fieldDef,fieldLabel,getFieldType, andgetOptionsfunctions are re-evaluated on each access. While this works, consider usingcreateMemoforfieldDefsince it depends onprops.fieldKey:- const fieldDef = () => PRELIMINARY_SECTION[props.fieldKey]; - const fieldLabel = () => fieldDef()?.label || props.fieldKey; + const fieldDef = createMemo(() => PRELIMINARY_SECTION[props.fieldKey]); + const fieldLabel = createMemo(() => fieldDef()?.label || props.fieldKey);This ensures the lookup only happens when
props.fieldKeychanges.packages/web/src/components/project/reconcile-tab/rob2-reconcile/NavbarDomainPill.jsx (1)
172-180: Simplify the spacing class logic.The current
pillSpacingClassimplementation has overlapping conditions that make it hard to reason about. Consider simplifying:Suggested simplification
const pillSpacingClass = () => { - let spacing = ''; - if (props.isFirst) spacing += 'ml-0.5 '; - if (props.isLast) spacing += 'mr-0.5 '; - if (!props.isFirst && !props.isLast) spacing += 'mx-0.5 '; - if (props.isFirst && !props.isLast) spacing += 'mr-0.5 '; - if (!props.isFirst && props.isLast) spacing += 'ml-0.5 '; - return spacing; + if (props.isFirst && props.isLast) return 'mx-0.5'; + if (props.isFirst) return 'ml-0.5 mr-0.5'; + if (props.isLast) return 'ml-0.5 mr-0.5'; + return 'mx-0.5'; };packages/web/src/components/project/reconcile-tab/rob2-reconcile/ROB2SummaryView.jsx (2)
67-88: Consider memoizing getDisplayValue for repeated access.
getDisplayValueis called inside aForloop for each item, and it accessesprops.finalAnswerswhich could be a large object. Consider memoizing or caching the results if performance becomes an issue with many items.
189-202: Complex ternary chain for badge styling could be extracted.The nested ternary for determining badge colors based on answer values is hard to read. Consider extracting to a helper function:
Suggested extraction
function getAnswerBadgeStyle(itemType, displayValue) { if (itemType !== NAV_ITEM_TYPES.DOMAIN_QUESTION) { return 'bg-blue-100 text-blue-700'; } switch (displayValue) { case 'Y': return 'bg-green-100 text-green-700'; case 'PY': return 'bg-lime-100 text-lime-700'; case 'PN': return 'bg-amber-100 text-amber-700'; case 'N': return 'bg-red-100 text-red-700'; default: return 'bg-gray-100 text-gray-700'; } }
| const headerContent = ( | ||
| <> | ||
| {/* Back button */} | ||
| <button | ||
| onClick={() => props.onCancel?.()} | ||
| class='shrink-0 rounded-lg p-2 transition-colors hover:bg-gray-100' | ||
| title='Go back' | ||
| > | ||
| <FiArrowLeft class='h-5 w-5 text-gray-600' /> | ||
| </button> | ||
|
|
||
| {/* Title */} | ||
| <div class='shrink-0'> | ||
| <h1 class='text-lg font-bold text-gray-900'>ROB-2 Reconciliation</h1> | ||
| <p class='text-xs text-gray-500'> | ||
| {props.reviewer1Name || 'Reviewer 1'} vs {props.reviewer2Name || 'Reviewer 2'} | ||
| </p> | ||
| </div> | ||
|
|
||
| <div class='h-8 w-px shrink-0 bg-gray-200' /> | ||
|
|
||
| {/* Navbar - navigation pills */} | ||
| <Show when={navbarStore.navItems?.length > 0}> | ||
| <div class='flex min-w-0 flex-1 items-center overflow-x-auto'> | ||
| <ROB2Navbar store={navbarStore} /> | ||
| </div> | ||
| </Show> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
headerContent is defined as a static JSX block, not a reactive getter.
In SolidJS, JSX defined outside of a component's return statement or a reactive context captures values at definition time. This means props.reviewer1Name and props.reviewer2Name won't update reactively, and navbarStore.navItems changes won't trigger re-renders of the Show condition.
Consider wrapping this in a function or moving it inline:
Option 1: Use a getter function
- const headerContent = (
+ const headerContent = () => (
<>
{/* ... */}
</>
);Then use headerContent={headerContent()} or pass the function if SplitScreenLayout supports it.
🤖 Prompt for AI Agents
In
`@packages/web/src/components/project/reconcile-tab/rob2-reconcile/ROB2ReconciliationWithPdf.jsx`
around lines 59 - 87, headerContent is a static JSX value so
props.reviewer1Name/props.reviewer2Name and navbarStore.navItems are captured
once and won’t update; change headerContent into a reactive getter (e.g., const
headerContent = () => ( ... )) or inline the JSX inside the component return so
it reads reactive props and store values, and wherever headerContent is used
call headerContent() (or pass the function if SplitScreenLayout expects a
function); keep the same structure and component references (ROB2Navbar, Show,
navbarStore.navItems, props.reviewer1Name/props.reviewer2Name) so reactivity
works.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/shared/src/checklists/rob2/compare.ts`:
- Around line 232-266: The compare functions call scoreRob2Domain and
scoreAllDomains on partial data which may have undefined domains/answers; modify
the calls in compareDomain (where scoring1/scoring2 and judgement1/judgement2
are derived) and in compareOverall (where scoring1/scoring2 and
judgement1/judgement2 are derived) to first check inputs (e.g., domain?.answers
or checklist?.domains) and either short-circuit to null judgements or pass a
safe default structure expected by scoreRob2Domain/scoreAllDomains; ensure
judgement1/judgement2 are set to null when scoring is skipped and preserve
existing fields (direction1/direction2, judgementMatch, directionMatch) to
handle nulls safely.
🧹 Nitpick comments (1)
packages/shared/src/checklists/rob2/compare.ts (1)
333-350: Agreement stats exclude judgement matches.
judgementMatchis computed but not included instatsorgetReconciliationSummary. If judgements are part of reconciliation scope, consider counting them; otherwise confirm that judgements are intentionally excluded to avoid confusion.Possible adjustment
for (const domainKey of activeDomains) { const domain = domains[domainKey]; totalItems += domain.questions.agreements.length + domain.questions.disagreements.length; agreedItems += domain.questions.agreements.length; + // Count domain judgement as an item if reconciled + totalItems += 1; + if (domain.judgementMatch) agreedItems += 1; + // Count direction as an item totalItems += 1; if (domain.directionMatch) agreedItems += 1; } + // Count overall judgement + totalItems += 1; + if (overall.judgementMatch) agreedItems += 1; + // Count overall direction totalItems += 1; if (overall.directionMatch) agreedItems += 1;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/shared/src/checklists/rob2/compare.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/yjs-sync.mdc)
**/*.{js,jsx,ts,tsx}: Use theuseProjecthook for managing Yjs connections with reference counting instead of creating Y.Doc instances directly
Never create Y.Doc instances directly; always use the connection registry managed by useProject
Access Y.Doc connection state via projectStore.getConnectionState(projectId) instead of checking connection state directly
Read Yjs-synced data from projectStore (using getStudies, getChecklist, etc.) rather than accessing Y.Doc maps/arrays directly
Use operation functions from useProject or projectActionsStore for writing data instead of directly modifying Y.Doc
Don't store Y.Doc references in component state; always retrieve the connection through useProject
Handle connection cleanup via onCleanup() in Solid.js components or equivalent cleanup patterns, calling disconnect() when the component unmounts
Files:
packages/shared/src/checklists/rob2/compare.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
**/*.{js,ts,jsx,tsx}: Prefer modern ES6+ syntax and features
Comments should explain why something is being done, not what the code is doing
Reserve comments for explaining intent, context, workarounds, assumptions, edge cases, or limitations
Use TODO(agent): prefix for incomplete work or flagged items with brief description and relevant doc references
Files:
packages/shared/src/checklists/rob2/compare.ts
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use emojis or unicode symbols anywhere in code, comments, documentation, plan files, commit messages, or examples
Files:
packages/shared/src/checklists/rob2/compare.ts
packages/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/*.{ts,tsx,js,jsx}: Prefer modern ES6+ syntax and features
Prefer config files over hardcoding values
Keep files small, focused, and modular - extract large files into sub-modules or separate utilities
Each file should handle one coherent responsibility
Comments should explain WHY something is being done or provide context, not repeat what the code is saying
Do NOT narrate what code is doing, don't duplicate function/variable names in comments, and don't leave stale comments that contradict the code
Use the Agent TODO convention// TODO(agent): Brief descriptionfor incomplete work, flagging items for future attention, known limitations, and documentation section referencesUse TODO(agent) convention with brief description and relevant doc section references for incomplete work or flagged issues
Files:
packages/shared/src/checklists/rob2/compare.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/lib/checklist-domain.js,packages/web/src/AMSTAR2/checklist-compare.js} : Store final answers in the reconciled checklist itself, not in reconciliation progress metadata
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/AMSTAR2/checklist-compare.js} : Use compareChecklists utility from checklist-compare.js for comparing reviewer checklists
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/lib/checklist-domain.js,packages/web/src/AMSTAR2/checklist-compare.js} : Reconciliation structure should contain checklist1Id, checklist2Id, and reconciledChecklistId
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/AMSTAR2/**,packages/web/src/ROBINS-I/**,packages/web/src/lib/checklist-domain.js : Checklist status transitions follow sequence: in_progress → completed (when all questions answered) → reconciled (after reconciliation complete)
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/lib/checklist-domain.js} : Store reconciliation progress metadata (IDs and optional currentPage) in the study's Y.Map reconciliation property
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to packages/web/src/components/checklist-ui/compare/** : Use Y.Text objects for collaborative editing of question notes in reconciliation
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/AMSTAR2/**,packages/web/src/ROBINS-I/**,packages/web/src/lib/checklist-domain.js : Maintain separate answer format implementations for each checklist type (AMSTAR2, ROBINS-I, Generic) to prevent data corruption
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/lib/checklist-domain.js} : Multi-part answers (q9a, q9b, q11a, q11b) should be stored as objects with answer matrix and critical flag properties
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/ROBINS-I/** : Use scoreChecklist utility from `@/ROBINS-I/checklist.js` to determine risk of bias rating: 'Low' | 'Moderate' | 'Serious' | 'Critical'
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to packages/web/src/components/checklist-ui/compare/** : Use MultiPartQuestionPage component for multi-part questions (q9 and q11)
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/AMSTAR2/checklist-compare.js} : Use compareChecklists utility from checklist-compare.js for comparing reviewer checklists
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/lib/checklist-domain.js,packages/web/src/AMSTAR2/checklist-compare.js} : Reconciliation structure should contain checklist1Id, checklist2Id, and reconciledChecklistId
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/lib/checklist-domain.js,packages/web/src/AMSTAR2/checklist-compare.js} : Store final answers in the reconciled checklist itself, not in reconciliation progress metadata
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/lib/checklist-domain.js} : Multi-part answers (q9a, q9b, q11a, q11b) should be stored as objects with answer matrix and critical flag properties
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to packages/web/src/components/checklist-ui/compare/** : Use Y.Text objects for collaborative editing of question notes in reconciliation
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:01:06.933Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/AMSTAR2/**,packages/web/src/ROBINS-I/**,packages/web/src/lib/checklist-domain.js : Maintain separate answer format implementations for each checklist type (AMSTAR2, ROBINS-I, Generic) to prevent data corruption
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:01:06.933Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/ROBINS-I/** : Use scoreChecklist utility from `@/ROBINS-I/checklist.js` to determine risk of bias rating: 'Low' | 'Moderate' | 'Serious' | 'Critical'
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to packages/web/src/components/checklist-ui/compare/** : Use multiPartAnswersEqual helper to compare multi-part answers between reviewers
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:01:06.933Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/AMSTAR2/**,packages/web/src/ROBINS-I/**,packages/web/src/lib/checklist-domain.js,packages/web/src/primitives/useProject/checklists.js : Use checklist operations from useProject hook (createChecklist, updateChecklistAnswer, getChecklistData) instead of manually updating checklist structure
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to {packages/web/src/components/checklist-ui/compare/**,packages/web/src/lib/checklist-domain.js} : Store reconciliation progress metadata (IDs and optional currentPage) in the study's Y.Map reconciliation property
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to packages/web/src/components/checklist-ui/compare/** : Auto-fill final answer with reviewer1's answer when reviewers agree and no final answer exists
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:01:06.933Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/AMSTAR2/** : Handle multi-part AMSTAR2 questions correctly: q9 (parts a, b), q11 (parts a, b) require separate answer entries
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:02:14.854Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/reconciliation.mdc:0-0
Timestamp: 2025-12-27T03:02:14.854Z
Learning: Applies to packages/web/src/components/checklist-ui/compare/** : Use SingleQuestionPage component for standard single-part questions (q1-q8, q10, q12-q16)
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:01:06.933Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/AMSTAR2/**,packages/web/src/ROBINS-I/**,packages/web/src/lib/checklist-domain.js : Use getChecklistStatus utility from `@/lib/checklist-domain.js` to determine current checklist status before performing operations
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
📚 Learning: 2025-12-27T03:01:06.933Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/checklist-operations.mdc:0-0
Timestamp: 2025-12-27T03:01:06.933Z
Learning: Applies to packages/web/src/components/checklist-ui/**,packages/web/src/lib/checklist-domain.js : Register and retrieve checklists using checklist-registry: getChecklistType and getChecklistComponent functions
Applied to files:
packages/shared/src/checklists/rob2/compare.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: corates
🔇 Additional comments (2)
packages/shared/src/checklists/rob2/compare.ts (2)
150-165: [Your rewritten review comment text here]
[Exactly ONE classification tag]
316-329: Active domain selection ignores checklist2 aim and aim mismatches.Using only checklist1's aim can omit domains that are active for checklist2 or when aims differ. This can hide disagreements and skew stats. Consider using the union of domains when aims differ.
Proposed fix
- const isAdhering = checklist1.preliminary?.aim === 'ADHERING'; - const activeDomains = getActiveDomainKeys(isAdhering); + const aim1 = checklist1.preliminary?.aim; + const aim2 = checklist2.preliminary?.aim; + const isAdhering1 = aim1 === 'ADHERING'; + const isAdhering2 = aim2 === 'ADHERING'; + const activeDomains = + aim1 && aim2 && aim1 === aim2 + ? getActiveDomainKeys(isAdhering1) + : Array.from( + new Set([ + ...getActiveDomainKeys(isAdhering1), + ...getActiveDomainKeys(isAdhering2), + ]), + );
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Get auto-calculated judgements | ||
| const scoring1 = scoreRob2Domain(domainKey, domain1?.answers); | ||
| const scoring2 = scoreRob2Domain(domainKey, domain2?.answers); | ||
|
|
||
| const judgement1 = scoring1.judgement; | ||
| const judgement2 = scoring2.judgement; | ||
|
|
||
| const direction1 = domain1?.direction ?? null; | ||
| const direction2 = domain2?.direction ?? null; | ||
|
|
||
| return { | ||
| questions: { agreements, disagreements }, | ||
| judgement1, | ||
| judgement2, | ||
| judgementMatch: judgement1 === judgement2, | ||
| direction1, | ||
| direction2, | ||
| directionMatch: direction1 === direction2, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Compare overall judgement between two checklists | ||
| */ | ||
| function compareOverall( | ||
| checklist1: PartialROB2Checklist, | ||
| checklist2: PartialROB2Checklist, | ||
| ): OverallComparison { | ||
| // Get auto-calculated overall judgements | ||
| // Cast to ChecklistState since the structures are compatible for scoring purposes | ||
| const scoring1 = scoreAllDomains(checklist1 as unknown as ChecklistState); | ||
| const scoring2 = scoreAllDomains(checklist2 as unknown as ChecklistState); | ||
|
|
||
| const judgement1 = scoring1.overall; | ||
| const judgement2 = scoring2.overall; |
There was a problem hiding this comment.
Guard scoring calls when domains or answers are missing.
scoreRob2Domain and scoreAllDomains are called even when answers or entire domains may be undefined in partial checklists. If the scoring functions expect fully-populated structures, this can throw and break comparison on incomplete data. Consider short-circuiting to null judgements or passing safe defaults.
Proposed fix (example)
- const scoring1 = scoreRob2Domain(domainKey, domain1?.answers);
- const scoring2 = scoreRob2Domain(domainKey, domain2?.answers);
+ const scoring1 = domain1?.answers
+ ? scoreRob2Domain(domainKey, domain1.answers)
+ : { judgement: null };
+ const scoring2 = domain2?.answers
+ ? scoreRob2Domain(domainKey, domain2.answers)
+ : { judgement: null };- const scoring1 = scoreAllDomains(checklist1 as unknown as ChecklistState);
- const scoring2 = scoreAllDomains(checklist2 as unknown as ChecklistState);
+ const scoring1 = scoreAllDomains(checklist1 as unknown as ChecklistState);
+ const scoring2 = scoreAllDomains(checklist2 as unknown as ChecklistState);
+ // If scoreAllDomains does not tolerate partial checklists, consider guarding with
+ // a presence check for required domains and returning null judgements instead.To verify expected inputs for scoring, inspect the scoring implementation:
#!/bin/bash
rg -n "function scoreRob2Domain|function scoreAllDomains" packages/shared/src/checklists/rob2/scoring.ts
sed -n '1,220p' packages/shared/src/checklists/rob2/scoring.ts🤖 Prompt for AI Agents
In `@packages/shared/src/checklists/rob2/compare.ts` around lines 232 - 266, The
compare functions call scoreRob2Domain and scoreAllDomains on partial data which
may have undefined domains/answers; modify the calls in compareDomain (where
scoring1/scoring2 and judgement1/judgement2 are derived) and in compareOverall
(where scoring1/scoring2 and judgement1/judgement2 are derived) to first check
inputs (e.g., domain?.answers or checklist?.domains) and either short-circuit to
null judgements or pass a safe default structure expected by
scoreRob2Domain/scoreAllDomains; ensure judgement1/judgement2 are set to null
when scoring is skipped and preserve existing fields (direction1/direction2,
judgementMatch, directionMatch) to handle nulls safely.
2 participants
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.