Fix diff line counts and GitHub-style stats#243
Merged
cooper (czxtm) merged 2 commits intoJun 3, 2026
Conversation
Co-authored-by: cooper <czxtm@users.noreply.github.com>
Co-authored-by: cooper <czxtm@users.noreply.github.com>
Contributor
📋 PR Overview
🔬 Coverage
|
cooper (czxtm)
approved these changes
Jun 3, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the widget diff UI to compute additions/deletions from actual hunk bodies (including new/removed files) and display GitHub-style green/red line stats in file headers, with corresponding Storybook snapshot updates.
Changes:
- Introduce shared diff line-stat parsing utilities (
countDiffLineStats,sumDiffLineStats) and a reusableDiffLineStatsBadge. - Update diff UI to show per-file line stats in the file header and use the new stats badge for hunk pills when no semantic summary title is available.
- Add unit tests for diff line-stat parsing and update Storybook snapshots accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/native/src/components/widget/summaries/hunk-pill.tsx | Switch hunk-pill counting to shared diff stats + render a stats badge fallback. |
| apps/native/src/components/widget/summaries/full-file-diff-editor.tsx | Add per-file stats badge in diff headers; tweak hunk pill behavior for multi-hunk files. |
| apps/native/src/components/widget/summaries/diff-line-stats.tsx | New shared diff line-stat parser and small badge component for +/− display. |
| apps/native/src/components/widget/summaries/diff-line-stats.test.tsx | Unit tests covering header-vs-hunk counting, new/removed files, and edge cases. |
| apps/native/src/components/widget/summaries/snapshots/hunk-pill.stories.tsx.snap | Snapshot updates reflecting new hunk-pill rendering. |
| apps/native/src/components/widget/summaries/snapshots/full-file-diff-editor.stories.tsx.snap | Snapshot updates reflecting per-file stats badge in headers. |
| apps/native/src/components/widget/summaries/snapshots/diff-section.stories.tsx.snap | Snapshot updates reflecting per-file stats badge in list rendering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
65
to
+66
| const changeType = displayChange.changeType; | ||
| const fileStats = sumDiffLineStats(changes); |
Comment on lines
+8
to
+12
| export function countDiffLineStats(diff: string): DiffLineStats { | ||
| let added = 0; | ||
| let removed = 0; | ||
| let inHunk = false; | ||
|
|
cooper (czxtm)
deleted the
cursor/ENG-518-linear-issue-triage-automation-60cf
branch
cooper (czxtm)
added a commit
that referenced
this pull request
Jun 3, 2026
* chore(beads): nixmac-srn nightly release feature * chore(beads): record PR #195 on nixmac-srn * chore(beads): file nixmac-62s (TS build blocker) discovered triaging PR #195 * chore(beads): nixmac-srn shipping depends on nixmac-62s (TS build fix) * fix(native): drop unused Plus and DropdownMenu imports (nixmac-62s) Both imports were only referenced inside JSX comments, so tsc flagged them (TS6133/TS6192) and broke the build check on main since bd658af. The env.ts BooleanFromString errors mentioned in the issue do not reproduce: apps/native resolves effect to 3.21.0 (transitive) which still exports BooleanFromString. Only the root workspace pulls effect@4.0.0-beta.62, but env.ts lives under apps/native. Closes nixmac-62s. * chore(beads): close nixmac-62s * fix(native): migrate env.ts off Schema.BooleanFromString (effect 4.0) The repo's bun.lock pins effect@4.0.0-beta.62 (no transitive effect@3.x). 4.0 removed Schema.BooleanFromString — which env.ts was using — so a fresh `bun install --frozen-lockfile` (what CI does) produces a tree where the import fails to compile. The earlier "doesn't reproduce locally" diagnosis on nixmac-62s was misled by a stale symlink at apps/native/node_modules/effect pointing into node_modules/.bun/effect@3.21.0/ left over from an older install run. Locally tsc resolved against the stale 3.x version, where BooleanFromString still exists. Migration: - Replace `Schema.BooleanFromString` with `Schema.Literals(["true", "false"])` (4.0's plural array form; 3.x had variadic `Schema.Literal(...)` — these are not interchangeable). - Coerce the validated string to `boolean | undefined` in code so the exported settings type matches the old BooleanFromString shape and downstream consumers (utils.ts:19's `!== true` check) don't need to change. - Drop the auto-derived `Schema.Schema.Type<typeof Settings>` because the runtime shape we expose (with the string→bool coercion) differs from the decoded shape. Strict parsing is preserved: any value other than "true"/"false" still fails at decode time, same as the original. Note: existing local installs with a stale apps/native/node_modules/effect symlink will need `bun install` to refresh — the stale 3.x effect doesn't have `Schema.Literals` (plural). One-time cost; matches CI from then on. Refs nixmac-62s. * chore(beads): file nixmac-rva (Storybook flake) + link as nixmac-srn blocker * chore(beads): file nixmac-cmu (Rust dead_code) + link as nixmac-srn blocker * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * chore(beads): file nixmac-k8d (Monaco platform drift) + link as nixmac-srn blocker * fix(beads): clean up stash-conflict markers in issues.jsonl Previous commit 0895711 accidentally included unresolved 'Updated upstream / Stashed changes' markers from a botched git stash pop. Re-exporting via bd export produces the correct merged state including nixmac-k8d as the fourth nixmac-srn blocker. * feat(ci): nightly develop→main release with minor version bump (#195) * feat(ci): nightly develop→main release with minor version bump Adds a GitHub Actions cron (09:00 UTC daily) that merges develop into main and tags a vMAJ.(MIN+1).0 release. Exits silently when develop has no new commits vs main. The actual ship work is delegated to build.yaml's existing `tag` mode — the nightly job just produces the merge commit and the tag, then atomically pushes both. compute-version.sh gets a small defensive edit: when HEAD is already tagged with a v* tag, `release` mode is demoted to `branch` so the main-push event doesn't trigger a duplicate (patch-bumped) ship alongside the tag-push event. Prerequisites for the cron to push successfully: - RELEASE_BOT_TOKEN secret (PAT with bypass), OR - repo ruleset bypass for github-actions[bot] on main Without either, `git push --atomic` fails on the protected main branch. Workflow falls back to GITHUB_TOKEN so cron + script wiring can be tested before bypass is granted. Files: - .github/workflows/nightly-release.yaml (new) - ops/scripts/release/nightly-release.sh (new; supports DRY_RUN=1) - ops/scripts/release/compute-version.sh (tag-aware release-mode skip) * feat(ci): scope nightly release trigger to native build affecteds Replace the simple "any commit on develop" trigger with the no-turbo equivalent of `turbo run build --affected --filter=native`. Releases now fire only when develop has changes inside the native workspace, its transitive workspace deps (currently @nixmac/ui), or global build inputs (root package.json, bun.lock, root tsconfig, Cargo.toml/lock). This skips nights where develop only got CI/docs/release-script changes so a minor version isn't burned on commits that wouldn't ship anything different to users. Adds ops/scripts/release/affected-paths.mjs which dynamically resolves the affected path set by reading the root workspaces config and walking workspace:* deps — no hardcoded list to maintain when new packages appear. Verified locally: - Real diff (apps/native/*, Cargo.lock, etc.): release - Synthetic diff (ops/, .github/, docs/, *.md): skip - Synthetic diff (bun.lock only): release (global input) * fix(ci): address Copilot review feedback on nightly-release - next_minor_version: filter to stable vMAJ.MIN.PATCH only so disposable -test.N tags (used by build.yaml for signing rehearsals) can't pollute the bump base - Remote tag existence check: use `git ls-remote --exit-code` with an exact ref pattern instead of `grep -q "${tag}"`, which would false- positive when a longer tag like v1.2.0-test.1 contains the candidate as a substring - Docstring: align step 2 with actual behavior (logs a one-liner then exits 0, not strictly silent) - Workflow: add concurrency group so a scheduled run + manual dispatch can't race on the same next-tag computation. cancel-in-progress=false to avoid aborting a half-completed merge+tag mid-flight * docs(ci): align skip-message wording with affected-paths policy Copilot follow-up: three places still described the skip condition as "no new commits vs main" or "skip silently". The actual policy is the affected-paths filter (no-turbo equivalent of `turbo run build --affected --filter=native`), which can return false even when develop has commits — they just don't touch the native build graph. - should_release docstring: "skip silently" → "skip (caller logs reason)" - main() skip log: "develop has no new work" → "no changes affect the native build graph" - workflow header: "no new commits vs main" → explanation of the affected-paths filter and what kinds of develop activity will no-op No behavior change. * fix(ci): nightly-release cwd-independence and ..→... range Copilot follow-up. Two real bugs: 1. The script claimed cwd-independence via REPO_ROOT but only used it for the affected-paths.mjs invocation. Every git command and the `node -p require('./package.json')` fallback ran relative to the caller's cwd, so invoking the script from anywhere other than the repo root would fail. main() now `cd`s into REPO_ROOT before doing any work, which makes the cwd-independence claim actually true. 2. should_release used `git diff origin/main..origin/develop` (two-dot range), which is tip-to-tip. If main has a hotfix not on develop, two-dot treats it as a develop-side deletion and flags it as a change — falsely triggering a release every night until the hotfix gets merged back. Three-dot (`origin/main...origin/develop`) diffs from the merge-base, so only commits new on develop are counted. Verified locally: running the script from /tmp now produces the same dry-run plan as running from the repo root. Two-dot vs three-dot on current origin/main vs origin/develop differs by one file, confirming there is currently a hotfix-style commit on main that the old check would have erroneously counted. * docs/fix(ci): nightly-release token fallback + optional-env docstring Copilot follow-up: 1. Workflow checkout's token fallback used `secrets.GITHUB_TOKEN` as the `||` second operand. While that *does* work in normal expression contexts (it's GitHub's documented way to reference the auto-injected token), the `github.token` context form is more reliable in fallback positions and `if:` evaluations. Switched to `secrets.RELEASE_BOT_TOKEN || github.token` for that defensive reason. 2. Header docstring claimed GIT_USER_NAME / GIT_USER_EMAIL were "Required env" but the code uses `if [[ -n "${VAR:-}" ]]` guards and silently skips git config when unset. They're actually optional CI overrides that fall back to the caller's existing git config — fine for local rehearsals, expected to be set in CI for bot-identity attribution. Updated the comment to match the actual code. No behavior change. * fix(ci): defensive Copilot follow-ups on nightly-release 1. DRY_RUN expression now gates on `github.event_name == 'workflow_dispatch'` before reading `inputs.dry_run`. The `inputs.*` context is only populated for dispatch events; gating ensures scheduled cron runs always get '0' regardless of how GitHub resolves missing-input refs. 2. compute-version.sh tag-skip regex now anchored with `$` so disposable `v0.22.0-test.N` tags (used for signing/notarization rehearsals) don't suppress legitimate `main`-push releases. Test tags don't ship (publish/R2/Linear steps in build.yaml skip them), so main-push must still bump+ship normally if such a tag happens to be at HEAD. Verified: `v1.2.3` matches the regex (correctly skips release mode), `v1.2.3-test.1` does not match (correctly does not suppress). 3. affected-paths.mjs now normalizes `package.json#workspaces` to handle both the npm/bun array form and the Yarn object form (`{ packages: [...] }`). nixmac uses the array form today; supporting both is cheap future-proofing if the repo ever switches package manager. * docs/fix(ci): nightly-release docstring + unconditional fetch Copilot follow-up: 1. Header step 4 said "Fast-forward / no-ff merge" but the implementation always uses `git merge --no-ff`. Updated the comment to match the actual behavior (always produces a merge commit so the release boundary stays visible in git log --first-parent). 2. `git fetch` was DRY_RUN-gated, but should_release and next_minor_version both query local refs (origin/main..origin/develop diff + `git tag --list` for the latest stable). On a stale checkout the dry-run could compute a wrong next version or false-positive "nothing to release". Unwrap the fetch from `run` so it always executes — fetch is read-only from the project's perspective (only updates local origin/* refs), so unconditional execution is safe and makes dry-run output reflect real remote state. Verified: the unconditional fetch pulled a fresh v0.23.2 tag on this run that the local checkout didn't have, validating the fix on its first execution. * fix(native): fix Chromatic Storybook Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(native): env.ts — drop version-skewed Schema.Literal for plain String PR #195 build broke at TS2554 ("Expected 1 arguments, but got 2") after 80a19c6 reverted NIX_INSTALLED_OVERRIDE from `Schema.Literals([...])` to `Schema.Literal(...)` (variadic). The problem: `Schema.Literal`'s signature is fundamentally version-skewed — effect 3.x: `Literal<L>(...values: L)` variadic, multi-value effect 4.0-beta: `Literal<L>(value: L)` single value only, use `Literals([...])` for sets So neither form source-compiles in both: - `Schema.Literal("true", "false")` works in 3.x, fails 4.0 (this PR) - `Schema.Literals(["true", "false"])` works in 4.0, fails 3.x (Chromatic) The repo's lockfile pins effect 4.0-beta, so tsc sees that — but the Chromatic/storybook test environment can resolve 3.x via workspace symlink hoisting, which is why 80a19c6 picked the 3.x form to fix Chromatic and broke the build instead. Fix: validate with plain `Schema.String` (signature is identical across versions) and coerce to boolean in code via `raw.NIX_INSTALLED_OVERRIDE === "true" ? true : undefined`. The only downstream consumer is `settings.NIX_INSTALLED_OVERRIDE !== true` in widget/utils.ts, which treats any non-"true" value as functionally false — so silent acceptance of unexpected strings is observationally equivalent to coercing them to undefined. We lose schema-level "must be 'true' or 'false'" strictness; we gain working CI on both the build and Chromatic paths. Verified against both effect@3.21.0 and effect@4.0.0-beta.62 symlinks in isolation: no TS2554, no missing-Literals errors. * fix(ci): nightly-release DRY_RUN final-message clarity Copilot follow-up: the final `echo "Released ${tag}"` ran unconditionally, so workflow_dispatch dry-runs printed "Released v0.24.0" even though the merge/tag/push steps were only echoed. Now branches on DRY_RUN: prints "Dry run complete — would have released ${tag}" in dry mode, "Released ${tag}" in real mode. Behavior preserved; just the operator-facing log line distinguishes the two modes. Did not touch the `local changed paths file path` declaration in should_release — Copilots claim that `path` is unused is incorrect. `path` is the inner loops read variable on line 85 and is used in the path-matching comparisons on lines 87, 89, 91, and 92. * fix(ci): nightly-release working-tree guard + setup-node pin Copilot follow-up: 1. Add a clean-working-tree guard before the destructive `git reset --hard origin/${MAIN_BRANCH}`. Local accidental invocations would otherwise silently obliterate uncommitted work. CI runners start clean by construction so the guard never fires in CI; DRY_RUN=1 bypasses it since dry-mode does not mutate the tree. Error path prints `git status --short` so the operator can see what would have been lost. 2. Add an actions/setup-node@v6 step pinned to node-version: 20 (matching build.yaml's convention). The release script invokes node for affected-paths.mjs and the package.json version fallback in next_minor_version. Without an explicit pin the cron would depend on whatever node the ubuntu-latest runner image ships, which is not stable across image updates. Verified: bash -n passes, dry-run still works, guard simulation triggers correctly on dirty tree and stays out of the way in DRY_RUN=1. --------- Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * Require configured local AI model names (#237) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cooper <czxtm@users.noreply.github.com> * fix(summarize): constrain hunk summary prompts (#242) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cooper <czxtm@users.noreply.github.com> * Fix diff line counts and GitHub-style stats (#243) * Fix diff line stats display Co-authored-by: cooper <czxtm@users.noreply.github.com> * Update diff UI snapshots Co-authored-by: cooper <czxtm@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cooper <czxtm@users.noreply.github.com> * fix critical evolve and setup regressions (#257) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cooper <czxtm@users.noreply.github.com> * Add configurable evolution output token limit (#261) * Add configurable evolution output token limit Co-authored-by: cooper <czxtm@users.noreply.github.com> * Update AI settings snapshot for output token limit Co-authored-by: cooper <czxtm@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cooper <czxtm@users.noreply.github.com> * Settings: show untracked settings copy (#269) * fix(settings): rename untracked settings indicator Co-authored-by: cooper <czxtm@users.noreply.github.com> * test(settings): update untracked settings snapshots Co-authored-by: cooper <czxtm@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cooper <czxtm@users.noreply.github.com> * ci(storybook): deploy previews before snapshot tests (#285) Reorder workflow steps so build/deploy/comment run BEFORE snapshot tests. Previously, when snapshot tests failed, the build, deploy, and PR comment steps were skipped—no preview URL was posted. Also includes: - Add deployments:write + issues:write permissions for deployment API - Add deploy URL validation (fail loudly on missing URL) - Add Publish Storybook deployment link step (GitHub Deployments API) - Fix comment step: use always() condition so it runs even if snapshot tests fail, and link the URL as markdown - Add develop to push triggers - Show preview link in step summary regardless of test outcome --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> Co-authored-by: cursor[bot] <206951365+cursor[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cooper <czxtm@users.noreply.github.com>
Juanpe Bolívar (arximboldi)
added a commit
that referenced
this pull request
Jun 3, 2026
Prior to this commit, both this branch (since a92e61b) and the merge result failed `cargo check` with 22 errors. The user asked for cargo + bun validation against the merge resolution, so this commit fixes the preexisting structural breakage that the merge inherited, plus a few follow-ups uncovered by the test suites. Rust: - git/mod.rs: restore the develop-side mod.rs that declares `pub mod query` / `pub mod init` and re-exports the right names from each. The orphan query.rs / init.rs files were already on disk; only the mod entry was missing after a92e61b accidentally gutted it. - evolve/mod.rs: route `repo_root` through `git::query` (its new home), pull `FileEdit` from `shared_types`, re-export `Evolution` from `shared_types::Evolution` (types.rs only imports it, doesn't re-export it), and fix the `made_edit_or_build_check` typo that develop's PR #272 introduced (variable never defined; should be `made_edit || made_build_check`). - shared_types/git.rs: re-add `WatcherEvent` alongside the unused `GitState` slice type — `state/watcher.rs` still constructs it and needs the symbol back. Marked `GitState` `#[allow(dead_code)]` until the watcher migration finishes. - summarize/queue_summarizer.rs: `#[allow(dead_code)]` on the not-yet-wired worker scaffolding (a92e61b carryover). - commands/settings_io.rs: update an EvolutionLimits test fixture to include the new `max_token_budget` / `max_output_tokens` fields. Frontend (post-merge tweaks): - ipc/api.ts: drop the duplicate `devConfigs` entry the auto-merge produced (it appeared once near `settings` and again after `promptHistory`). - ipc/types.ts: regenerated via `cargo run --example specta_gen_ts` to drop duplicated `ConfigField` / `ConfigurableSchema` / `EnumVariant` / `FieldType` blocks the auto-merge dual-inserted. - viewmodel/git.ts + viewmodel.test.ts: switch the `git_state_changed` payload type from `GitState` (unused TS-side) to `WatcherEvent` (what the Rust watcher actually emits). - hooks/use-evolve.ts: drop the eager `mirrorChangeMapState(null)` at the top of `handleEvolve`. Line 109 already correctly avoids overwriting the change map for conversational responses; the eager clear contradicted that and broke develop's PR #243 test that exercises conversational follow-ups. - Tests (`use-evolve.test.ts`, `use-rollback.test.ts`, `prompt-input.test.tsx`, `system-defaults-cta.test.tsx`): swap the removed `setGitStatus` / `setEvolveState` / `setChangeMap` / `setSummaryAvailable` widget-store setters for the viewmodel mirror fns (`mirrorGitState`, `mirrorEvolveState`, `mirrorChangeMapState`). `summaryAvailable` no longer exists on the widget store; the related assertions are dropped. - Storybook snapshots: 30 stories were regenerated after the AI Models tab field split, the system-defaults icon swap, and the hunk-pill render going through `DiffLineStatsBadge`. Validation: - `cargo check`: clean - `cargo test`: 366 passed - `bun run test:unit`: 116 passed - `bun run test:storybook`: 165 passed - `bun run build` (tsc + vite): clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Resolves ENG-518
Test Plan
bun -F native test:unit -- diff-line-stats.test.tsxbun -F native lintbun -F native buildbun -F native test:storybook -- src/components/widget/summaries/hunk-pill.stories.tsx src/components/widget/summaries/full-file-diff-editor.stories.tsx src/components/widget/summaries/diff-section.stories.tsxNote: full
bun -F native test:storybookstill has unrelated pre-existing snapshot flakes in animated rebuild/file-view stories after installing Playwright Chromium.Docs