Repository navigation
fix(desktop): upgrade @pierre/diffs to fix line-number ordering (SUPER-804) - #4848
Conversation
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Ready to review this PR? Stage has broken it down into 2 individual chapters for you:
Chapters generated by Stage for commit db31201 on May 23, 2026 7:17am UTC. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe desktop application's ChangesDependency Update
🎯 1 (Trivial) | ⏱️ ~2 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.spec/improvements/SUPER-804/root_cause_analysis.md:
- Line 31: The document's downgrade reason text currently claims a 7-day
minimum-release-age ("Minimum-release-age policy (7 days required)"), which
conflicts with the PR scope that uses a 3-day minimum-release-age (259200s);
update the line to reflect the 3-day policy by replacing the "7 days" wording
with "3 days (259200s)" or equivalent phrasing so the root_cause_analysis.md
entry matches the PR's policy value.
- Around line 8-11: The fenced code block containing the release-downgrade note
("fix(ci): downgrade `@pierre/diffs` to pass minimum-release-age policy" /
"`@pierre/diffs` 1.1.7 → 1.1.3 (released 2026-03-21)") needs a language tag;
update that triple-backtick fence in root_cause_analysis.md to use a language
such as text (e.g., change ``` to ```text) so markdown linting passes and the
docs remain consistent.
In @.spec/improvements/SUPER-804/SCOPE.md:
- Around line 40-41: The SCOPE acceptance criteria currently require
`@pierre/diffs` version 1.2.2 but the PR upgraded apps/desktop/package.json to
1.1.22; update the SCOPE.md entries that mention `@pierre/diffs` and the bun
install check to expect `1.1.22` (and ensure the bullet for bun install reflects
that v1.1.22 meets minimumReleaseAge/no-conflict), and apply the same correction
to the duplicate entries referenced on lines 49-50 so the documented scope
matches the implemented dependency version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b5cf227-d3c5-4fc5-9152-de5e632e8d0a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.spec/improvements/SUPER-804/BRIEF.md.spec/improvements/SUPER-804/SCOPE.md.spec/improvements/SUPER-804/evidence_analysis.md.spec/improvements/SUPER-804/follow-ups.md.spec/improvements/SUPER-804/investigation_findings.md.spec/improvements/SUPER-804/root_cause_analysis.mdapps/desktop/package.json
| ``` | ||
| fix(ci): downgrade @pierre/diffs to pass minimum-release-age policy | ||
| @pierre/diffs 1.1.7 → 1.1.3 (released 2026-03-21) | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced code block.
This block should declare a language (for example text or bash) to satisfy markdown linting and keep docs consistent.
Proposed doc fix
-```
+```text
fix(ci): downgrade `@pierre/diffs` to pass minimum-release-age policy
`@pierre/diffs` 1.1.7 → 1.1.3 (released 2026-03-21)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.spec/improvements/SUPER-804/root_cause_analysis.md around lines 8 - 11, The
fenced code block containing the release-downgrade note ("fix(ci): downgrade
`@pierre/diffs` to pass minimum-release-age policy" / "`@pierre/diffs` 1.1.7 → 1.1.3
(released 2026-03-21)") needs a language tag; update that triple-backtick fence
in root_cause_analysis.md to use a language such as text (e.g., change ``` to
```text) so markdown linting passes and the docs remain consistent.
| ### Version Analysis | ||
| - **Current version**: 1.1.3 (has line number bug) | ||
| - **Fixed version**: 1.1.7 (likely contains fix) | ||
| - **Downgrade reason**: Minimum-release-age policy (7 days required) |
There was a problem hiding this comment.
Fix policy duration: it conflicts with documented repo policy.
Line 31 states a 7-day requirement, but this PR context and scope record a 3-day minimum-release-age (259200s). Please correct this to keep the root-cause chain consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.spec/improvements/SUPER-804/root_cause_analysis.md at line 31, The
document's downgrade reason text currently claims a 7-day minimum-release-age
("Minimum-release-age policy (7 days required)"), which conflicts with the PR
scope that uses a 3-day minimum-release-age (259200s); update the line to
reflect the 3-day policy by replacing the "7 days" wording with "3 days
(259200s)" or equivalent phrasing so the root_cause_analysis.md entry matches
the PR's policy value.
| - [ ] `@pierre/diffs` version updated to `1.2.2` in `apps/desktop/package.json` | ||
| - [ ] `bun install` completes without errors (v1.2.2 is >60 days old, no `minimumReleaseAge` conflict) |
There was a problem hiding this comment.
Update scope targets to match the implemented dependency version.
This scope still requires 1.2.2, but the PR actually upgrades apps/desktop/package.json to 1.1.22. Please align acceptance criteria and in-scope file expectations to avoid false verification outcomes and stale follow-up assumptions.
Also applies to: 49-50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.spec/improvements/SUPER-804/SCOPE.md around lines 40 - 41, The SCOPE
acceptance criteria currently require `@pierre/diffs` version 1.2.2 but the PR
upgraded apps/desktop/package.json to 1.1.22; update the SCOPE.md entries that
mention `@pierre/diffs` and the bun install check to expect `1.1.22` (and ensure
the bullet for bun install reflects that v1.1.22 meets
minimumReleaseAge/no-conflict), and apply the same correction to the duplicate
entries referenced on lines 49-50 so the documented scope matches the
implemented dependency version.
Greptile SummaryThis PR upgrades
Confidence Score: 3/5The dependency change itself is narrow and low-risk, but the implementation diverges from its own binding specification without a recorded amendment, and the lockfile bundles in unrelated workspace version bumps that aren't accounted for. The
|
| Filename | Overview |
|---|---|
| apps/desktop/package.json | Single-line version bump @pierre/diffs 1.1.3→1.1.22; package version shows 1.11.0 (minor bump, not a patch) — the version bump itself is unrelated to the diff but was swept in by the lockfile regeneration |
| bun.lock | Lock updated for @pierre/diffs 1.1.3→1.1.22 and @pierre/theme 0.0.22→0.0.28; also captures pre-existing but previously unrecorded version bumps for @superset/desktop (1.10.3→1.11.0) and @superset/host-service (0.8.11→0.8.12) — these are unrelated to the stated PR scope |
| .spec/improvements/SUPER-804/SCOPE.md | Binding scope requires upgrade to v1.2.2 and asserts it passes minimumReleaseAge; implementation delivers v1.1.22 with no formal scope amendment recorded |
| .spec/improvements/SUPER-804/root_cause_analysis.md | Investigation artifact documenting the root cause; correctly identifies the 1.1.7→1.1.3 downgrade as the source of the bug |
| .spec/improvements/SUPER-804/follow-ups.md | Deferred follow-up documentation, no code changes |
| .spec/improvements/SUPER-804/BRIEF.md | New ticket brief — documentation only, no code impact |
| .spec/improvements/SUPER-804/evidence_analysis.md | Investigation evidence document, no code changes |
| .spec/improvements/SUPER-804/investigation_findings.md | Investigation findings document, no code changes |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["@pierre/diffs v1.1.3\n(line-number bug present)"]
B["@pierre/diffs v1.1.22\n(this PR — line-number bug fixed)"]
C["@pierre/diffs v1.2.2\n(SCOPE.md binding target)"]
D["@pierre/diffs v1.2.1\n(Chrome gutter fix — PR #729)"]
E["minimumReleaseAge policy\n(bunfig.toml — 3 days)"]
F["Follow-up PR\n(once v1.2.x ages past 3 days)"]
A -->|"bun install / this PR"| B
B -->|"passes age check\n10.6 days old"| G["apps/desktop deployed"]
C -->|"blocked — SCOPE says >60d old\nbut PR says only ~1.9d old"| E
D -->|"released May 20, 2026\nonly ~2 days old"| E
E -->|"deferred"| F
F -->|"upgrade to v1.2.1+"| D
Comments Outside Diff (1)
-
bun.lock, line 769-778 (link)Unrelated workspace version bumps swept in by lockfile regeneration
Running
bun installregenerated the lock file from all workspacepackage.jsonfiles, which captured pre-existing but previously unrecorded version bumps:@superset/desktop1.10.3→1.11.0 (minor) and@superset/host-service0.8.11→0.8.12. Neither change is mentioned in the PR description, theapps/desktop/package.jsondiff only shows the@pierre/diffsline, and"Scope amendments: (none)"in SCOPE.md makes the desktop minor bump especially surprising. If these version strings were already committed to their respectivepackage.jsonfiles, the lockfile is simply catching up — but it would be worth calling that out explicitly so reviewers know these aren't accidental changes bundled into this PR.Prompt To Fix With AI
This is a comment left during a code review. Path: bun.lock Line: 769-778 Comment: **Unrelated workspace version bumps swept in by lockfile regeneration** Running `bun install` regenerated the lock file from all workspace `package.json` files, which captured pre-existing but previously unrecorded version bumps: `@superset/desktop` 1.10.3→1.11.0 (minor) and `@superset/host-service` 0.8.11→0.8.12. Neither change is mentioned in the PR description, the `apps/desktop/package.json` diff only shows the `@pierre/diffs` line, and `"Scope amendments: (none)"` in SCOPE.md makes the desktop minor bump especially surprising. If these version strings were already committed to their respective `package.json` files, the lockfile is simply catching up — but it would be worth calling that out explicitly so reviewers know these aren't accidental changes bundled into this PR. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
.spec/improvements/SUPER-804/SCOPE.md:40-41
**Binding scope not satisfied and not formally amended**
The SCOPE document has `status: binding` and its first acceptance criterion is `@pierre/diffs` upgraded to `1.2.2`. The document also explicitly claims "v1.2.2 is >60 days old, no `minimumReleaseAge` conflict". The PR delivers `1.1.22` instead, citing that v1.2.0 is only 1.9 days old — directly contradicting the scope's own age claim. Since `## Scope amendments` still reads "(none)", this deviation isn't formally captured. The scope document should be amended to explain why `1.1.22` is the interim target, to keep the binding record accurate and prevent confusion when the follow-up upgrade to v1.2.x is eventually filed.
### Issue 2 of 3
bun.lock:769-778
**Unrelated workspace version bumps swept in by lockfile regeneration**
Running `bun install` regenerated the lock file from all workspace `package.json` files, which captured pre-existing but previously unrecorded version bumps: `@superset/desktop` 1.10.3→1.11.0 (minor) and `@superset/host-service` 0.8.11→0.8.12. Neither change is mentioned in the PR description, the `apps/desktop/package.json` diff only shows the `@pierre/diffs` line, and `"Scope amendments: (none)"` in SCOPE.md makes the desktop minor bump especially surprising. If these version strings were already committed to their respective `package.json` files, the lockfile is simply catching up — but it would be worth calling that out explicitly so reviewers know these aren't accidental changes bundled into this PR.
### Issue 3 of 3
bun.lock:2114-2117
**`@pierre/theme` transitive bump not called out in the test plan**
`@pierre/theme` moves from `0.0.22` to `0.0.28` (6 releases) as a transitive dependency of `@pierre/diffs@1.1.22`. The PR's test plan doesn't include verifying that custom-theme registration (`registerCustomTheme`) and the theme's color-variable contract still behave correctly with the new theme version. Given that `SCOPE.md` explicitly lists `registerCustomTheme` as a component to verify, it would be good to confirm this was actually exercised rather than relying solely on typecheck passing.
Reviews (1): Last reviewed commit: "fix(desktop): upgrade @pierre/diffs 1.1...." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - [ ] `@pierre/diffs` version updated to `1.2.2` in `apps/desktop/package.json` | ||
| - [ ] `bun install` completes without errors (v1.2.2 is >60 days old, no `minimumReleaseAge` conflict) |
There was a problem hiding this comment.
The SCOPE document has status: binding and its first acceptance criterion is @pierre/diffs upgraded to 1.2.2. The document also explicitly claims "v1.2.2 is >60 days old, no minimumReleaseAge conflict". The PR delivers 1.1.22 instead, citing that v1.2.0 is only 1.9 days old — directly contradicting the scope's own age claim. Since ## Scope amendments still reads "(none)", this deviation isn't formally captured. The scope document should be amended to explain why 1.1.22 is the interim target, to keep the binding record accurate and prevent confusion when the follow-up upgrade to v1.2.x is eventually filed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .spec/improvements/SUPER-804/SCOPE.md
Line: 40-41
Comment:
**Binding scope not satisfied and not formally amended**
The SCOPE document has `status: binding` and its first acceptance criterion is `@pierre/diffs` upgraded to `1.2.2`. The document also explicitly claims "v1.2.2 is >60 days old, no `minimumReleaseAge` conflict". The PR delivers `1.1.22` instead, citing that v1.2.0 is only 1.9 days old — directly contradicting the scope's own age claim. Since `## Scope amendments` still reads "(none)", this deviation isn't formally captured. The scope document should be amended to explain why `1.1.22` is the interim target, to keep the binding record accurate and prevent confusion when the follow-up upgrade to v1.2.x is eventually filed.
How can I resolve this? If you propose a fix, please make it concise.| "@pierre/diffs": ["@pierre/diffs@1.1.22", "", { "dependencies": { "@pierre/theme": "0.0.28", "@shikijs/transformers": "^3.0.0", "diff": "8.0.3", "hast-util-to-html": "9.0.5", "lru_map": "0.4.1", "shiki": "^3.0.0" }, "peerDependencies": { "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0" } }, "sha512-1Iv7kdl6OABFCd1n2HQbGUiRHouXPaHoIjcb7Lwg8zeJKY5ph+cESFcGEyIiwW0NCbKGtYS2bTnXmI+Eze5dwg=="], | ||
|
|
||
| "@pierre/theme": ["@pierre/theme@0.0.22", "", {}, "sha512-ePUIdQRNGjrveELTU7fY89Xa7YGHHEy5Po5jQy/18lm32eRn96+tnYJEtFooGdffrx55KBUtOXfvVy/7LDFFhA=="], | ||
| "@pierre/theme": ["@pierre/theme@0.0.28", "", {}, "sha512-1j/H/fECBuc9dEvntdWI+l435HZapw+RCJTlqCA6BboQ5TjlnE005j/ROWutXIs8aq5OAc82JI2Kwk4A1WWBgw=="], | ||
|
|
There was a problem hiding this comment.
@pierre/theme transitive bump not called out in the test plan
@pierre/theme moves from 0.0.22 to 0.0.28 (6 releases) as a transitive dependency of @pierre/diffs@1.1.22. The PR's test plan doesn't include verifying that custom-theme registration (registerCustomTheme) and the theme's color-variable contract still behave correctly with the new theme version. Given that SCOPE.md explicitly lists registerCustomTheme as a component to verify, it would be good to confirm this was actually exercised rather than relying solely on typecheck passing.
Prompt To Fix With AI
This is a comment left during a code review.
Path: bun.lock
Line: 2114-2117
Comment:
**`@pierre/theme` transitive bump not called out in the test plan**
`@pierre/theme` moves from `0.0.22` to `0.0.28` (6 releases) as a transitive dependency of `@pierre/diffs@1.1.22`. The PR's test plan doesn't include verifying that custom-theme registration (`registerCustomTheme`) and the theme's color-variable contract still behave correctly with the new theme version. Given that `SCOPE.md` explicitly lists `registerCustomTheme` as a component to verify, it would be good to confirm this was actually exercised rather than relying solely on typecheck passing.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".spec/improvements/SUPER-804/root_cause_analysis.md">
<violation number="1" location=".spec/improvements/SUPER-804/root_cause_analysis.md:31">
P2: Incorrect policy threshold in root-cause analysis: states '7 days' when actual bunfig.toml enforces a 3-day minimum release age (259200s)</violation>
</file>
<file name=".spec/improvements/SUPER-804/SCOPE.md">
<violation number="1" location=".spec/improvements/SUPER-804/SCOPE.md:40">
P2: SCOPE.md binding acceptance criteria target @pierre/diffs@1.2.2 but the PR actually upgrades to 1.1.22, creating a misalignment that can drive incorrect implementation and validation</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ### Version Analysis | ||
| - **Current version**: 1.1.3 (has line number bug) | ||
| - **Fixed version**: 1.1.7 (likely contains fix) | ||
| - **Downgrade reason**: Minimum-release-age policy (7 days required) |
There was a problem hiding this comment.
P2: Incorrect policy threshold in root-cause analysis: states '7 days' when actual bunfig.toml enforces a 3-day minimum release age (259200s)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .spec/improvements/SUPER-804/root_cause_analysis.md, line 31:
<comment>Incorrect policy threshold in root-cause analysis: states '7 days' when actual bunfig.toml enforces a 3-day minimum release age (259200s)</comment>
<file context>
@@ -0,0 +1,63 @@
+### Version Analysis
+- **Current version**: 1.1.3 (has line number bug)
+- **Fixed version**: 1.1.7 (likely contains fix)
+- **Downgrade reason**: Minimum-release-age policy (7 days required)
+
+## Resolution Path
</file context>
| @@ -0,0 +1,78 @@ | |||
| --- | |||
There was a problem hiding this comment.
P2: SCOPE.md binding acceptance criteria target @pierre/diffs@1.2.2 but the PR actually upgrades to 1.1.22, creating a misalignment that can drive incorrect implementation and validation
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .spec/improvements/SUPER-804/SCOPE.md, line 40:
<comment>SCOPE.md binding acceptance criteria target @pierre/diffs@1.2.2 but the PR actually upgrades to 1.1.22, creating a misalignment that can drive incorrect implementation and validation</comment>
<file context>
@@ -0,0 +1,78 @@
+## Binding scope (chosen: moderate)
+
+### Acceptance criteria
+- [ ] `@pierre/diffs` version updated to `1.2.2` in `apps/desktop/package.json`
+- [ ] `bun install` completes without errors (v1.2.2 is >60 days old, no `minimumReleaseAge` conflict)
+- [ ] `bun run typecheck` passes
</file context>
| --- | |
| - [ ] `@pierre/diffs` version updated to `1.1.22` in `apps/desktop/package.json` |
fa9e389 to
db31201
Compare
Summary
@pierre/diffsfrom1.1.3→1.1.22to fix out-of-order line numbers in the diff viewer1.1.7→1.1.3in commit4a2bcb116(pane-redesign PR feat: redesign pane layout system (@superset/panes) #3088) to satisfybunfig.toml'sminimumReleaseAge = 259200(3-day) policyRoot Cause
bunfig.tomlenforces a 3-day minimum release age for all packages. On April 1,@pierre/diffs@1.1.7was within the age window for CI, so it was downgraded to v1.1.3 to unblock the pane-redesign PR. v1.1.3 contains gutter rendering bugs documented in:Why v1.1.22 instead of v1.2.2?
v1.2.0+ is currently blocked by the same
minimumReleaseAgepolicy (v1.2.0 is only 1.9 days old). v1.1.22 is the newest version that passes the 3-day check (10.6 days old). Once v1.2.1+ ages past the 3-day window, a follow-up PR can upgrade to capture the Chrome-specific gutter fix from PR #729.Impact
apps/desktopdepends on@pierre/diffs(no other apps/packages affected)fileGap,enableHoverUtility,renderHoverUtility,renderCustomMetadataare not imported anywhere inapps/desktop/src/Linear
Fixes SUPER-804
Test plan
bun installsucceedsbun run typecheckpassesbun run lintpassesshould see this:
Summary by cubic
Upgrades the desktop diff viewer to fix out-of-order line numbers, restoring correct gutter numbering and resolving SUPER-804.
@pierre/diffsfrom1.1.3to1.1.22(newest allowed bybunfig.tomlminimumReleaseAge); onlyapps/desktopuses it; no breaking APIs used.Written for commit db31201. Summary will update on new commits. Review in cubic
Summary by CodeRabbit