Skip to content

test(sidebar): golden for accented, mixed-case and numbered names - #29966

Merged
Jinwoo-H merged 1 commit into
mainfrom
jinwoo/sidebar-golden-name-order
Oct 17, 2026
Merged

Jinwoo-H merged 1 commit into
mainfrom
jinwoo/sidebar-golden-name-order

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​29 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​29
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​293 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​293

ELI5

The sidebar list has saved "golden" snapshots that fail when the list order changes. None of the saved snapshots used names with accents, capital letters or numbers, so the recent change to how the sidebar sorts names (#29946) did not show up in them. This adds one snapshot that does.

What Changed

Test-only. No production code changes and no change users can see.

  • New fixture sidebar-list-golden-name-order.test-fixture.ts: three projects ("Éclair", "ångström", and an SSH project "eclair"), workspaces named éclair, Eclair, eclair, éclair written with a separate accent mark, zebra, w2, W2, w10, ångström, angstrom, the same workspace id on two hosts, and the same labels on a local and an SSH project. Project groups and folder workspaces use the same kinds of names, all with equal manual order so only the name decides.
  • New golden scenario-name-order.txt, captured for no grouping and grouping by project, across Name, Recent and project-name sort, and both project orders (identical captures are folded into "same as" pointers).

Recorded order (Name sort): angstrom, angstrom (same id, second host), ångström, Eclair, eclair (SSH), eclair, éclair (separate accent), éclair, W2, w2 (SSH), w2, w10, zebra. Recent sort puts the two recently active rows (w10, zebra) first, then the rest by name. Project groups: angstrom, ångström, eclair, Éclair, zebra. Folders: Eclair, éclair, W2, w2, w2, w10, zebra, with the two w2 folders ordered by id. This matches the comparator: case and accents only break exact ties, digit runs compare by value, and full ties fall back to the host-qualified id.

Why

The existing goldens are the reference for any change to the sidebar list pipeline. Without these names in them, a sorting change can pass every golden. One extra scenario in the existing harness covers this without adding a parallel test setup.

Linked Issue

Follow-up to #29946.

Visual Proof

N/A. Test-only change; nothing on screen changes.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

pnpm tc, pnpm test src/renderer/src/components/sidebar/sidebar-list-goldens.test.tsx src/shared/sidebar-list/text-order.test.ts, pnpm run check:code-quality:changed, pnpm format all pass. The golden was reviewed by hand against the rules above.

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…ames

Records the post-#29946 text order for workspace rows (Name, Recent and
project-name sort), project group names and folder workspace names, ungrouped
and grouped by project, so a future ordering change shows up as a golden diff.
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

The change adds fixture data for sidebar name ordering and snapshots its rendered output across two grouping modes, three sort modes, and both project-order settings. The expected output records workspace ordering, grouped headers, selection order, and command mappings.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 2be1c

The change is mergeable with a bounded test-coverage gap: the new snapshots do not distinguish manual from recent repository ordering within a project group.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: adding sidebar golden coverage for accented, mixed-case, and numbered names.
Description check Passed The description is complete and explains the change, rationale, linked follow-up issue, visual-proof exception, testing, AI disclosure, scope considerations, and checklist status. It provides sufficie…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is complete and explains the change, rationale, linked follow-up issue, visual-proof exception, testing, AI disclosure, scope considerations, and checklist status. It provides sufficient detail for review.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments: 1
src/renderer/src/components/sidebar/sidebar-list-goldens.test.tsx: 1

313-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make one grouped fixture distinguish projectOrderBy.

projectOrderBy changes the order of repository sections inside a project group. The name-order fixture has only one repository in each project group, so manual and recent must produce identical grouped output. Add a second repository to one group with different manual order and recent activity, then update the golden. Keep project-group tabOrder unchanged because project-group headers intentionally do not depend on projectOrderBy.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 58982aa3-238a-409c-b3dd-4da5cac42fb4
📥 Commits

Reviewing files that changed from the base of the PR and between 002320c and 2be1c5c.

📒 Files selected for processing: 3
  • src/renderer/src/components/sidebar/__goldens__/sidebar-list/scenario-name-order.txt
  • src/renderer/src/components/sidebar/sidebar-list-golden-name-order.test-fixture.ts
  • src/renderer/src/components/sidebar/sidebar-list-goldens.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

@Jinwoo-H
Jinwoo-H merged commit 442491b into main Oct 17, 2026
31 checks passed
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.

1 participant