Skip to content

refactor(rename): export NormalizedLabel/RefDefBracketBytes, remove lsp duplicates - #690

Merged
jeduden merged 4 commits into
mainfrom
claude/youthful-knuth-hsz74x
Jun 24, 2026
Merged

refactor(rename): export NormalizedLabel/RefDefBracketBytes, remove lsp duplicates#690
jeduden merged 4 commits into
mainfrom
claude/youthful-knuth-hsz74x

Conversation

@jeduden

@jeduden jeduden commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • Export NormalizedLabel and RefDefBracketBytes from internal/rename, deleting the identical private copies in internal/lsp/rename.go (flagged by the 2026-06-24 arch audit)
  • LinkRef now normalizes oldLabel internally so callers may pass raw or pre-normalized text; the CLI no longer needs to call NormalizedLabel before calling LinkRef
  • Delete the now-redundant NormalizeLabel(string) string wrapper from heading.go (confusable with NormalizedLabel([]byte) string, differing only by trailing d)
  • Update ValidRefDefBodyLines / contentBlockLines return types from map[int]bool to map[int]struct{} consistent with the repo's map[K]bool → map[K]struct{} migration
  • Fix two gofmt double-blank-line violations in lsp/rename.go left by the function deletion

Test plan

  • go test ./... is green (3 commits, each passing)
  • TestLinkRef_NormalizesOldLabel covers the new internal normalization (red → green TDD)
  • TestNormalizedLabel covers NormalizedLabel([]byte) directly
  • TestRefDefBracketBytes / TestRefDefBracketBytesEdgeCases cover the exported helper
  • gofmt -l internal/lsp/rename.go produces no output

Code review rounds

3 sequential /code-review xhigh rounds completed. All confirmed findings addressed:

  • Round 1: nil guard on RefDefBracketBytes (added then removed as dead code after analysis); duplicate test removed; confusable NormalizeLabel API deleted
  • Round 2: nil guard confirmed dead code and removed; LinkRef normalization internalized; plan/2606240212 stale entries removed
  • Round 3: gofmt double blank lines fixed (only confirmed finding introduced by this PR)

🤖 Generated with Claude Code


Generated by Claude Code

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.62%. Comparing base (11b905e) to head (30fdf68).

Additional details and impacted files
Components Coverage Δ
Go 98.61% <100.00%> (-0.01%) ⬇️
TypeScript 99.54% <ø> (ø)

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

claude added 4 commits June 24, 2026 05:24
…sp copies

Export `normalizedLabel` → `NormalizedLabel` and `refDefBracketBytes` →
`RefDefBracketBytes` from `internal/rename`. Delete the identical private
copies from `internal/lsp/rename.go` and update all lsp call sites to use
the shared exports. Adds dedicated tests for both exported helpers.

Also adds plan files for the two remaining test-debt items from the
2026-06-24 architecture audit (lsp/rename.go helpers and export.go helpers).

Closes plan/2606240214_arch-fix-rename-dedup.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQQBtYXUNuyr4HmoGGk26T
…el API duplicate

- Add nil guard after RefDefBracketBytes(row) in refDefEditsInBody: the
  function returns nil for non-ref-def rows and was indexed unconditionally,
  matching the pattern the LSP caller already follows.
- Delete NormalizeLabel(string) from heading.go: its sole caller
  (cmd/mdsmith/rename.go) now calls NormalizedLabel([]byte(oldName)) directly,
  eliminating a second confusably-named exported symbol from the package.
- Remove TestRefDefBracketBytes_Exported from rename_test.go: it duplicated
  TestRefDefBracketBytes in helpers_test.go with no new coverage.
- Remove TestNormalizeLabel from heading_test.go (function deleted).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQQBtYXUNuyr4HmoGGk26T
…x gofmt

- LinkRef now normalizes oldLabel internally at the function entry so
  callers can pass raw label text or a pre-normalized form; both
  produce identical results (normalization is idempotent).
- Remove the nil guard on RefDefBracketBytes that was added in the
  prior commit: the regex in validRefDefMatches and RefDefBracketBytes
  are semantically equivalent on splitLines-processed rows, so the
  guard is dead code. Removing it avoids silent partial renames if
  the guard ever fired (continue would have dropped the def-edit while
  use-edits were still included).
- CLI: remove pre-normalization of oldName before calling LinkRef;
  LinkRef normalizes internally now.
- lsp/rename.go: remove two double blank lines left by the function
  deletion in the prior commit (gofmt violation).
- Update plan/2606240212 to remove stale references to the two lsp
  helpers that were deleted in this PR branch.
- Red/Green TDD: TestLinkRef_NormalizesOldLabel added first (failing),
  then the oldLabel = NormalizedLabel(...) line added to make it pass.
PLAN.md catalog was out of date (MDS019): regenerated to include the
three new plan files added in the rename-dedup work.

plan/2606240212: paragraph at line 26 exceeded the readability index
(MDS023). Rewrote to use shorter sentences without changing meaning.
@jeduden
jeduden force-pushed the claude/youthful-knuth-hsz74x branch from 6f43065 to 30fdf68 Compare June 24, 2026 05:26
@jeduden jeduden added queue Add to a PR to enqueue it queue:active Applied automatically when a PR is in an active batch and removed queue Add to a PR to enqueue it labels Jun 24, 2026
@jeduden

jeduden commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

🟢 Merge Queue — picked up

This PR is in the queue and will be batched with other queue-labelled PRs.

Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run.

@jeduden

jeduden commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

🔵 Merge Queue — CI running

Merged into batch branch merge-queue/batch-690-1782280363. View CI run.

Next: No action needed — you'll be notified when CI completes.

@jeduden jeduden removed the queue:active Applied automatically when a PR is in an active batch label Jun 24, 2026
@jeduden
jeduden merged commit 6cc286a into main Jun 24, 2026
32 checks passed
@jeduden

jeduden commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

Merge Queue — merged

This PR landed on main via commit 6cc286a. CI run that validated the merge.

Next: Done — nothing more to do here.

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.

2 participants