Skip to content

Unify checklist text refs behind discriminated union (#483)#492

Merged
InfinityBowman merged 9 commits into
mainfrom
typed-text-refs
Apr 18, 2026
Merged

Unify checklist text refs behind discriminated union (#483)#492
InfinityBowman merged 9 commits into
mainfrom
typed-text-refs

Conversation

@InfinityBowman
Copy link
Copy Markdown
Owner

@InfinityBowman InfinityBowman commented Apr 18, 2026

Summary

  • Replaces the loose getTextRef(params: {sectionKey?, fieldKey?, questionKey?}) shape on ChecklistOperations with a discriminated TextRef union (AMSTAR2 | ROBINS_I | ROB2), and applies the same shape to setTextValue.
  • Bridges the reconciliation wrapper's existing loose-params callback into the new typed shape so the primitive can be tightened independently of the per-checklist subtree migrations.

This is the text-ref portion of #483. Subsequent commits will:

  1. Expose getTextRef/setTextValue on project.checklist.
  2. Migrate AMSTAR2, ROBINS-I, and ROB2 component subtrees off the three legacy methods.
  3. Migrate the reconciliation engine + adapters to thread TextRef end-to-end.
  4. Delete the three legacy getQuestionNote / getRobinsText / getRob2Text methods.

The other two parts of #483 (typed project.* singleton, ConnectionPhase enum) are already in place on main.

Test plan

  • pnpm --filter web typecheck
  • Open an AMSTAR2, ROBINS-I, and ROB2 checklist and confirm note/comment fields read and persist.
  • Run a reconciliation flow for each type and confirm setTextValue still copies reviewer comments into the reconciled checklist.

Refs #483.

Summary by CodeRabbit

  • Refactor
    • Consolidated checklist text retrieval across AMSTAR2, ROBINS_I, and ROB2 checklist types by unifying multiple text-access methods into a single reference-based approach, improving code consistency and maintainability without affecting user-facing functionality.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 18, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 126273d0-4d5a-48d7-aeac-181197276e53

📥 Commits

Reviewing files that changed from the base of the PR and between 57aaca9 and 9c33822.

📒 Files selected for processing (28)
  • packages/web/src/components/checklist/AMSTAR2Checklist/AMSTAR2Checklist.tsx
  • packages/web/src/components/checklist/ChecklistWithPdf.tsx
  • packages/web/src/components/checklist/ChecklistYjsWrapper.tsx
  • packages/web/src/components/checklist/GenericChecklist.tsx
  • packages/web/src/components/checklist/LocalChecklistView.tsx
  • packages/web/src/components/checklist/ROB2Checklist/DomainSection.tsx
  • packages/web/src/components/checklist/ROB2Checklist/PreliminarySection.tsx
  • packages/web/src/components/checklist/ROB2Checklist/ROB2Checklist.tsx
  • packages/web/src/components/checklist/ROB2Checklist/SignallingQuestion.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/DomainSection.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/PlanningSection.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/ROBINSIChecklist.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/SectionA.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/SectionB.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/SectionC.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/SectionD.tsx
  • packages/web/src/components/checklist/ROBINSIChecklist/SignallingQuestion.tsx
  • packages/web/src/components/project/completed-tab/PreviousReviewersView.tsx
  • packages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsx
  • packages/web/src/components/project/reconcile-tab/amstar2-reconcile/adapter.tsx
  • packages/web/src/components/project/reconcile-tab/engine/ReconciliationEngine.tsx
  • packages/web/src/components/project/reconcile-tab/engine/types.ts
  • packages/web/src/components/project/reconcile-tab/engine/useReconciliationEngine.ts
  • packages/web/src/components/project/reconcile-tab/rob2-reconcile/adapter.tsx
  • packages/web/src/components/project/reconcile-tab/rob2-reconcile/pages/PreliminaryPage.tsx
  • packages/web/src/components/project/reconcile-tab/robins-i-reconcile/adapter.tsx
  • packages/web/src/primitives/useProject/checklists/index.ts
  • packages/web/src/project/actions.ts

📝 Walkthrough

Walkthrough

This pull request refactors the checklist text-access API by consolidating three separate callback functions (getQuestionNote, getRobinsText, getRob2Text) into a single getTextRef function that accepts a discriminated union TextRef object. The new API is applied consistently across 22 components spanning checklist UI, wrappers, reconciliation engine, and public operations.

Changes

Cohort / File(s) Summary
Type System & Operations
packages/web/src/primitives/useProject/checklists/index.ts, packages/web/src/project/actions.ts
Introduced TextRef discriminated union type with AMSTAR2, ROBINS\_I, and ROB2 variants. Removed getQuestionNote, getRobinsText, getRob2Text from ChecklistOperations interface and updated getTextRef/setTextValue signatures to use TextRef parameter instead of optional section/field/question keys.
AMSTAR2 Checklist
packages/web/src/components/checklist/AMSTAR2Checklist/AMSTAR2Checklist.tsx
Replaced optional getQuestionNote prop with required getTextRef(TextRef) => Y.Text | null. Updated StandardQuestion and Question9/Question11 to compute noteYText via getTextRef unconditionally and derive question keys directly from question.text.
ROB2 Checklist Components
packages/web/src/components/checklist/ROB2Checklist/..., DomainSection.tsx, PreliminarySection.tsx, SignallingQuestion.tsx, ROB2Checklist.tsx
Replaced getRob2Text callback with getTextRef(TextRef). Made domainKey and questionKey required in SignallingQuestion. Updated all text derivations to construct TextRef objects with type: 'ROB2' and use getTextRef unconditionally.
ROBINS-I Checklist Components
packages/web/src/components/checklist/ROBINSIChecklist/..., PlanningSection.tsx, SectionA.tsx, SectionB.tsx, SectionC.tsx, SectionD.tsx, SignallingQuestion.tsx, DomainSection.tsx, ROBINSIChecklist.tsx
Replaced getRobinsText callback with getTextRef(TextRef). Made domainKey and questionKey required in SignallingQuestion. Updated all useMemo and NoteEditor bindings to construct TextRef objects with type: 'ROBINS_I' and call getTextRef without conditional guards.
Checklist Wrapper Components
packages/web/src/components/checklist/ChecklistWithPdf.tsx, ChecklistYjsWrapper.tsx, GenericChecklist.tsx, LocalChecklistView.tsx
Updated prop interfaces to accept getTextRef(TextRef) instead of three separate getters. ChecklistYjsWrapper and LocalChecklistView now forward a single getTextRef callback that constructs TextRef objects from study/checklist IDs.
Reconciliation Engine & Types
packages/web/src/components/project/reconcile-tab/engine/types.ts, useReconciliationEngine.ts, ReconciliationEngine.tsx
Updated EngineContext, ReconciliationEngineProps, and UseReconciliationEngineOptions to use strongly-typed getTextRef(TextRef) => Y.Text | null and setTextValue(TextRef, string) => void. Removed getTextRef from engine context; setTextValue is now required (non-nullable).
Reconciliation Adapters
packages/web/src/components/project/reconcile-tab/.../adapter.tsx, pages/PreliminaryPage.tsx (AMSTAR2, ROB2, ROBINS-I)
Updated adapters and page components to construct and pass TextRef objects to getTextRef and setTextValue. Removed optional _getTextRef parameter from autoFillFromReviewer1. Made setTextValue required and updated all call sites to pass TextRef with appropriate type discriminator.
Reconciliation Wrapper
packages/web/src/components/project/reconcile-tab/ReconciliationWrapper.tsx
Replaced branching logic on checklist type (isRobinsI/isRob2) with unified getTextRef and setTextValue callbacks that forward to ops.checklist. Removed per-type getter logic; now delegates all text access through generic operations.
Public API Integration
packages/web/src/components/project/completed-tab/PreviousReviewersView.tsx
Updated local callback naming and GenericChecklist props from getQuestionNote to getTextRef; functionality remains the same (returns null when checklist ID unavailable).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • 340 local checklist notes #341: Introduced the initial per-checklist text-getter pattern (getQuestionNote/getRobinsText/getRob2Text) that this PR consolidates and unifies into a single TextRef-based API.
  • 207 reconcile for robins #243: Added reconciliation adapter integration for ROBINS-I, using the older getRobinsText callback API that this PR refactors to use TextRef.
  • 74 amstar2 user notes #90: Introduced getQuestionNote and NoteEditor wiring for AMSTAR2, which this PR generalizes into a unified TextRef/getTextRef contract.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch typed-text-refs

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.

❤️ Share

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

@InfinityBowman InfinityBowman marked this pull request as ready for review April 18, 2026 16:50
@InfinityBowman InfinityBowman merged commit 788b2cc into main Apr 18, 2026
1 of 2 checks passed
@InfinityBowman InfinityBowman deleted the typed-text-refs branch April 29, 2026 20:37
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.

2 participants