fix(risks): treatment plan disappears when switching strategies#2860
Merged
Conversation
Root cause: DescriptionEditor stored `draft` and `mode` as independent
state synced to the `value` prop via a chain of 4 useEffects. When
switching from a strategy with content to one without, mode flipped to
'edit' and the resync effect's edit-mode guard blocked reloading the
original plan when switching back.
Rewrote the component to use a single `draft: string | null` state —
null means preview mode (render value directly from props), string
means edit mode. This eliminates the 4 sync effects entirely and makes
the state model trivial: enter edit → copy value into draft, save →
push draft up and clear, cancel → clear.
Strategy changes remount the editor via key={strategy} so internal
state initializes fresh from the new value.
Also hides the "Generate treatment plan" button for non-Mitigate
strategies since regeneration always produces a mitigation plan.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
cubic analysis
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Linked issue analysis
Linked issue: CS-402: [BUG] - Risk treatment plan UI bug
| Status | Acceptance criteria | Notes |
|---|---|---|
| Treatment plan reappears when switching Mitigate → Accept → Mitigate (no disappearance or unexpected mode flip). | The DescriptionEditor was rewritten to use a single discriminated draft state and TreatmentPlanTab now remounts the editor via key={strategy}, which addresses the root cause described in the PR. However the PR's manual verification for this scenario is still unchecked and there is no automated test explicitly asserting the Mitigate→Accept→Mitigate reappearance behavior. | |
| ✅ | Hide the 'Generate treatment plan' / 'Regenerate with AI' button for non‑Mitigate strategies. | PR changes TreatmentPlanTab to pass onRegenerate only for Mitigate strategies and adds a unit test that asserts the regenerate button is hidden for non‑Mitigate strategies. |
| ✅ | Switching strategies shows the correct description from strategyDescriptions. | A unit test was added that renders the component with strategyDescriptions and asserts the shown text updates when the strategy radio is clicked. |
| Regenerate treatment plan displays the new AI content in the preview after completion. | DescriptionEditor now clears its draft on regenerate (setDraft(null)) so the parent-provided value/regenRun flow can surface the new content; RegenProgress hooks into run status. But there is no automated test validating that a completed regen updates the preview, and the PR's manual check 'regenerate treatment plan, verify new content appears in preview' is unchecked. | |
| Rapidly switching strategies does not cause data loss. | Refactor (single draft state + remount by key) is designed to prevent the previous flaky sync behavior, but there is no automated test for rapid switching and the corresponding manual check remains unchecked. |
Contributor
|
🎉 This PR is included in version 3.55.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DescriptionEditorstoreddraftandmodeas independent state synced to thevalueprop via 4 chaineduseEffects. When switching from a strategy with content to one without,modeflipped to'edit'and the resync effect's edit-mode guard blocked reloading the original plan when switching back.DescriptionEditorto use a singledraft: string | nullstate —null= preview mode (rendervaluedirectly from props),string= edit mode. Eliminates all sync effects. Strategy changes remount the editor viakey={strategy}so internal state initializes fresh.What changed
useEffects + 2 refs for draft/mode syncuseEffects in DescriptionEditor (1useLayoutEffectfor textarea sizing)draft+modeas separate statedraft: string | nulldiscriminated statehandleRegenerate)Test plan
strategyDescriptions🤖 Generated with Claude Code
Summary by cubic
Fixes disappearing treatment plans when switching strategies by simplifying the editor state and remounting per strategy. Also hides regeneration for non‑Mitigate strategies to prevent generating the wrong plan. Addresses CS‑402.
Bug Fixes
draft: string | nullstate and remounts withkey={strategy}to always reflect the correct description.Refactors
DescriptionEditorto remove four syncuseEffects; only auseLayoutEffectremains for textarea sizing.onRegenerate/regeneratingare optional, andRegenProgressnow has correct effect dependencies.Written for commit 64978cb. Summary will update on new commits. Review in cubic