fix(KEN-1353): Home's edited row promises remedies it cannot judge - #2566
Merged
Conversation
Home's attention row aggregates edited packages across places, while the remedies it named are decided per package on the package page from three inputs: whether the harness is forkable, whether a discard is available, and which harnesses are edited. A single sentence on the aggregate row is a second judge of that question, and it leaves the mixed and no-discard cases unnamed. Drop the remedies sentence. Home keeps the paused-updates statement, and its action already lands on the page that names the remedies. Claude-Session: https://claude.ai/code/session_01BX6AXXaMFhzUCc9umAfJ6v
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
🔵 Needs a closer look
The package-page destination and changelog can still claim discard is available when canDiscard is false.
Pull request overview
This PR removes misleading Keep/Discard guidance from Home’s edited-packages attention row and updates related tests and documentation.
Changes:
- Simplifies Home’s edited-package copy.
- Updates attention-row assertions.
- Adds a changelog entry.
File summaries
| File | Summary |
|---|---|
ui/src/lib/copy.ts |
Removes remedy guidance; remaining package-page guidance may still overclaim discard availability when canDiscard is false. |
ui/src/components/home/attention-rows.test.ts |
Verifies paused updates remain and remedy wording is absent. |
changelog.d/fixed/1353-edited-row-remedies.md |
Documents the correction but repeats the discard-availability claim. |
Review details
Suppressed comments (1)
ui/src/lib/copy.ts:252
- This now directs users to the package page as the source of the available remedies, but that page still renders
FORK_NOTICE_DETAILwheneverforkableHarnessis set whilecanDiscardonly controls the button atfork-notice.tsx:116. The normal removed-upstream path setscan_discardfalse (crates/core/src/package/updates/eval.rs:233) and can still have a forkable edited rendering, so the destination can tell users to discard edits even though no discard action exists; the same overclaim is repeated inchangelog.d/fixed/1353-edited-row-remedies.md:1. Make the detail conditional oncanDiscard(or otherwise remove that claim) before relying on this destination.
// which packages, where, and what that means. The remedies are named on
// the package's own page, where each package's own answer is known. The
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The comment above editedAttentionTitle said the remedies are named on the package's own page, where each package's own answer is known, and the changelog line said that page names what it can do for that package. The page holds the per-package inputs, but its own detail sentence does not always state them: a forkable edited package whose source is gone renders a sentence offering to discard while the Discard button is gated off. State where the remedies belong instead of asserting the destination page's copy is accurate. Comment and changelog wording only. Claude-Session: https://claude.ai/code/session_01BX6AXXaMFhzUCc9umAfJ6v
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
forkableHarness,canDiscard, and which harnesses are edited;ui/src/components/package/fork-notice.tsx:86,114), and the Home row aggregates packages across places (ui/src/components/home/attention-rows.ts:137). For a package kind that cannot be forked, Home promised a Keep the page never offers.editedAttentionDetail(ui/src/lib/copy.ts). Home keeps the paused-updates statement, and its action already lands on the page that holds the per-package answers.ui/src/components/home/attention-rows.test.ts.Why not conditioned copy
KEN-1353's original Done-when asked for Home copy conditioned on the same forkable answer the package page reads. The root adjustment on the issue (2026-09-11) rejects that, and this PR follows the adjustment.
Conditioning on one forkable answer would make the Home row a second judge of a question it does not hold the inputs for. The row covers a set of packages that can mix forkable and non-forkable kinds, and Keep and Discard turn on two different inputs, so a single sentence leaves the mixed case and the no-discard case unnamed. Dropping the sentence removes the false promise without adding a second judge.
A related defect this PR does not fix
crates/core/src/package/updates/eval.rs:233setscan_discard: falseon the removed-upstream path whileforkable_harnessis computed independently.ui/src/components/package/fork-notice.tsx:82rendersFORK_NOTICE_DETAILonforkableHarnessalone, and that sentence says "or discard the edits and go back to the catalog's version" while:114withholds the Discard button. So a forkable package a person edited, whose source was then removed from its catalog, sees an offer to discard with no Discard button.That is the same defect class as KEN-1353, one screen deeper. It is pre-existing, this diff neither introduces nor arms it, and it does not block this change from working, so it is not fixed here. It is raised with the repository owner as separate work.
The second commit (
c012d2bf7) does correct what this PR itself said about that page: the comment aboveeditedAttentionTitleand the changelog line asserted the page states each package's own answer. They now say only where the remedies belong.Completed Issues
Size
No allowance is stated on the issue. Measured at the first commit: 5 production lines added, 5 test lines added, 0 render-mirror lines. The second commit changes one comment and one changelog line.
Validation
uivitest: theattention-rowssuite (15 tests) and the full UI suite (202 files, 1504 tests) pass.not.toMatch(/own copy|discard/i)assertion; deleting the ", so updates are paused there" clause reddens the paused-updates assertion.tsc --noEmitexits 0.env -u TMPDIR tools/guard --fullon each head,add59af3candc012d2bf7: exit 0, no failure lines.preflight --base origin/main: clean, no findings.Review
One local correctness round: verdict pass, no blockers, no suggestions. It confirmed no stale reference to the dropped sentence remains in the repository, and that the new row assertion is non-vacuous.
https://claude.ai/code/session_01BX6AXXaMFhzUCc9umAfJ6v