improve ux for robins and add missing sections by InfinityBowman · Pull Request #66 · InfinityBowman/corates

InfinityBowman · 2025-12-16T16:52:08Z

Summary by CodeRabbit

New Features
- Added Planning, Section A, Section C, and Section D to the ROBINS‑I checklist for richer data capture
- Added judgement label "Low (except for concerns about uncontrolled confounding)"
Improvements
- Replaced radio inputs with clickable buttons that toggle on/off and removed per‑section Clear controls
- Updated information sources with clearer examples and expanded registry/regulatory references
Documentation
- Removed the architecture-goals.md document

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-16T16:52:23Z

Caution

Review failed

The pull request is closed.

Walkthrough

Removes an architecture-goals.md design document and substantially refactors the ROBINS‑I checklist: updates checklist-map entries, adds Planning/SectionA/SectionC/SectionD components, replaces radio inputs with toggle buttons in judgement/direction UIs, and integrates new sections into the main checklist component.

Changes

Cohort / File(s)	Summary
Documentation Removal `architecture-goals.md`	Deleted the Durable Object / Yjs CRDT sync architecture specification, including data model, storage notes (D1/R2), DO responsibilities, client pseudocode, and implementation roadmap.
Checklist Data Model `packages/web/src/ROBINS-I/checklist-map.js`	Updated `INFORMATION_SOURCES` entries with clearer examples; removed "Other"; added new exports `SECTION_D`, `PLANNING_SECTION`, `SECTION_A`, and `SECTION_C` describing new fields and controls.
UI Control Refactoring `packages/web/src/components/checklist-ui/ROBINSIChecklist/DomainJudgement.jsx`, `packages/web/src/components/checklist-ui/ROBINSIChecklist/OverallSection.jsx`	Replaced radio inputs with clickable toggle buttons for judgement and direction (toggle-to-null behavior), removed `createUniqueId` usage and Clear buttons, added a new judgement label ("Low (except for concerns about uncontrolled confounding)") and color mapping, adjusted styling for selected/disabled states.
New Section Components `packages/web/src/components/checklist-ui/ROBINSIChecklist/PlanningSection.jsx`, `packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionA.jsx`, `packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionC.jsx`, `packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionD.jsx`	Added four Solid.js components (PlanningSection, SectionA, SectionC, SectionD). Each renders form fields (textareas, checkboxes, radio group), supports `disabled`, manages updates via `onUpdate` callbacks, and exports as named/default where applicable.
Checklist Integration & Exports `packages/web/src/components/checklist-ui/ROBINSIChecklist/ROBINSIChecklist.jsx`, `packages/web/src/components/checklist-ui/ROBINSIChecklist/index.js`	Replaced old Protocol Type Toggle UI with new Planning section and "Preliminary Considerations" header; wired new section components into checklist state via dedicated handlers; re-exported PlanningSection, SectionA, SectionC, SectionD from the module index.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25–30 minutes

Pay attention to mapping between SECTION_* keys in checklist-map.js and each component's state keys/onUpdate payloads.
Verify toggle-to-null behavior and keyboard/accessibility for button-based judgement/direction controls.
Confirm disabled/read-only rendering across new components and integration in ROBINSIChecklist.jsx.

Possibly related PRs

improve ux for robins and add missing sections #66 — Appears to modify the same ROBINS‑I checklist files and introduce identical/new UI components and checklist-map changes.
Robins i #50 — Overlaps ROBINS‑I checklist additions and exports; likely another iteration of these changes.

Poem

🐰 I hopped through maps and buttons bright,
New sections sprung up overnight.
Radios fled, toggles took their place,
Planning, A, C, D — a snug new space.
Cheers from a rabbit, nimble and light! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: UX improvements to the ROBINS-I checklist (replacing radio inputs with button-based controls) and addition of missing sections (Planning, Section A, Section C, Section D).

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f29fad6 and 952ecda.

📒 Files selected for processing (3)

packages/web/src/ROBINS-I/checklist-map.js (3 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/DomainJudgement.jsx (5 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionA.jsx (1 hunks)

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cloudflare-workers-and-pages · 2025-12-16T16:53:46Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	corates	`952ecda`	Commit Preview URL	Dec 16 2025, 05:03 PM

coderabbitai

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/web/src/components/checklist-ui/ROBINSIChecklist/DomainJudgement.jsx (1)
7-7: Remove unused domainId prop.

The domainId prop is documented and accepted but never used in the component body.

Apply this diff:
 /**
  * Domain judgement selector with risk of bias level and optional direction
  * @param {Object} props
- * @param {string} props.domainId - Unique domain identifier
  * @param {string} props.judgement - Current judgement value
Also applies to: 16-16

🧹 Nitpick comments (4)

packages/web/src/components/checklist-ui/ROBINSIChecklist/DomainJudgement.jsx (1)
42-69: Consider using createMemo for the isSelected computation.

While the current arrow function approach works, using createMemo would be more explicit about reactive tracking and align with SolidJS best practices.

As per coding guidelines, when computing values based on props, prefer createMemo:
+import { For, createMemo } from 'solid-js';
   <For each={ROB_JUDGEMENTS}>
     {judgement => {
-      const isSelected = () => props.judgement === judgement;
+      const isSelected = createMemo(() => props.judgement === judgement);
       return (
Apply the same pattern to the direction buttons (lines 82-107).
packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionD.jsx (1)
12-21: Potential issue: Unnecessary spread may cause performance issues.

The handleSourceToggle function spreads props.sectionDState on every toggle. If sectionDState contains large nested objects beyond sources and otherSpecify, this could be inefficient. However, based on the JSDoc (line 7), it appears the state structure is simple, so this is likely fine.

If sectionDState ever grows to include large nested data, consider updating only the sources object:
 function handleSourceToggle(sourceName) {
-  const newSources = {
-    ...props.sectionDState?.sources,
-    [sourceName]: !props.sectionDState?.sources?.[sourceName],
-  };
   props.onUpdate({
     ...props.sectionDState,
-    sources: newSources,
+    sources: {
+      ...props.sectionDState?.sources,
+      [sourceName]: !props.sectionDState?.sources?.[sourceName],
+    },
   });
 }
packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionA.jsx (1)
13-17: Consider co-locating field-to-state mapping.

The fieldToStateKey mapping is hardcoded here but could be defined in checklist-map.js alongside SECTION_A for better maintainability. Based on the relevant snippets, SECTION_A items in checklist-map.js don't have a stateKey property, unlike other sections (e.g., SECTION_C.c1.stateKey).

To improve consistency across sections and reduce duplication, consider adding stateKey to each field in SECTION_A within checklist-map.js:
// In checklist-map.js
export const SECTION_A = {
  a1: {
    id: 'a1',
    label: 'A1',
    text: 'Specify the numerical result being assessed',
    placeholder: 'e.g., OR = 1.5 (95% CI: 1.2-1.9)',
    type: 'textarea',
    stateKey: 'numericalResult', // Add this
  },
  // ... similar for a2 and a3
};
Then in SectionA.jsx, you can access it directly:
- const fieldToStateKey = {
-   a1: 'numericalResult',
-   a2: 'furtherDetails',
-   a3: 'outcome',
- };
- const stateKey = fieldToStateKey[key];
+ const stateKey = field.stateKey;
packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionC.jsx (1)
13-16: Minor: Functions could be constants.

The textFields() and c4Field() functions don't access reactive state, so they could be constants or moved outside the component for clarity. However, wrapping them as functions is harmless and maintains consistency.

For slight performance improvement, consider:
- const textFields = () =>
-   Object.entries(SECTION_C).filter(([_key, field]) => field.type === 'textarea');
- const c4Field = () => SECTION_C.c4;
+ const textFields = Object.entries(SECTION_C).filter(([_key, field]) => field.type === 'textarea');
+ const c4Field = SECTION_C.c4;
And update usages to remove ():
- <For each={textFields()}>
+ <For each={textFields}>

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2392857 and f29fad6.

📒 Files selected for processing (10)

architecture-goals.md (0 hunks)
packages/web/src/ROBINS-I/checklist-map.js (3 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/DomainJudgement.jsx (5 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/OverallSection.jsx (4 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/PlanningSection.jsx (1 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/ROBINSIChecklist.jsx (3 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionA.jsx (1 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionC.jsx (1 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/SectionD.jsx (1 hunks)
packages/web/src/components/checklist-ui/ROBINSIChecklist/index.js (1 hunks)

💤 Files with no reviewable changes (1)

architecture-goals.md

🧰 Additional context used

📓 Path-based instructions (7)

packages/web/src/components/**/*.{jsx,tsx,js,ts}