Skip to content

fix: complete UUID primary keys cleanup#2597

Merged
steveyegge merged 1 commit into
gastownhall:mainfrom
trillium:uuid-cleanup
Mar 15, 2026
Merged

fix: complete UUID primary keys cleanup#2597
steveyegge merged 1 commit into
gastownhall:mainfrom
trillium:uuid-cleanup

Conversation

@trillium

@trillium trillium commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #2575 (UUID primary keys cherry-pick). The core migration changed types and schema, but many files throughout the codebase still referenced the old AUTO_INCREMENT design. This PR makes the codebase internally consistent.

Changes

  • Fix backup restore bug: json.Number -> string for event/comment IDs in backup_restore.go — UUID strings can't unmarshal as json.Number, causing TestBackupRestoreRoundTrip to fail
  • Update embedded Dolt schema SQL files (5 files): BIGINT AUTO_INCREMENT PRIMARY KEY -> CHAR(36) NOT NULL PRIMARY KEY DEFAULT (UUID()) for comments, events, issue_snapshots, compaction_snapshots, wisp_events, wisp_comments
  • Remove dead LastEventID int64 from backup state struct — was never set during export, vestige of integer ID era
  • Update comments/docs referencing old AUTO_INCREMENT behavior:
    • issueops/create.go — PersistComments docs
    • git_remote_test.go — test name and comments (renamed TestAutoIncrementAfterPull -> TestCreateIssueAfterPull)
    • import_from_jsonl_test.go — comment dedup test description
    • docs/audit-sync-mode-complexity.md — mark auto-increment reset section as resolved
    • cmd/bd/info.go — version history entry

What was NOT changed (intentionally)

  • Migration 005 (005_wisp_auxiliary_tables.go): Still uses BIGINT schema because existing databases ran this migration already; migration 010 converts them
  • migrations_test.go: Test tables use BIGINT to represent pre-migration-010 state
  • concurrency_test.go and bd-example-extension-go: Use AUTO_INCREMENT on their own custom tables, not the 6 beads tables

Testing

  • go build ./... — passes
  • go vet ./... — passes
  • TestBackupRestoreRoundTrip — now passes (was broken by UUID commit)
  • 3 pre-existing test failures confirmed on base commit (not introduced by this PR)

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

@trillium @claude
fix: complete UUID primary keys cleanup across codebase
fca3d3c 
Follow-up to 1ce765e (UUID primary keys for federation-safe events).
The core migration changed types and schema, but many files still
referenced the old AUTO_INCREMENT design. This commit makes the
codebase internally consistent:

- Update embedded Dolt schema SQL files to use CHAR(36) UUID PKs
- Fix backup restore: json.Number -> string for UUID event/comment IDs
  (fixes TestBackupRestoreRoundTrip)
- Remove dead LastEventID field from backup state (never set)
- Update comments in issueops/create.go, import test, git_remote_test
- Update docs/audit-sync-mode-complexity.md (mark auto-increment resolved)
- Update version history in info.go

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@trillium

trillium commented Mar 14, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@trillium

trillium commented Mar 14, 2026

Copy link
Copy Markdown
Contributor Author

CodeRabbit CLI Review

Starting CodeRabbit review in plain text mode...

Connecting to review service
Setting up
Analyzing
Reviewing

Review completed: No findings ✔

This was referenced Mar 15, 2026
Closed
Merged
@garitar

garitar commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for digging through the UUID cleanup here.

While auditing the backup/restore behavior I confirmed the backup_restore.go json.Number -> string change in this PR does fix the separate round-trip restore failure on main.

I opened a narrower issue/PR pair for that specific regression in case it helps review or cherry-picking:

Not trying to step on the broader cleanup work here, just wanted the backup restore bug to have its own tight repro and passing regression proof.

@steveyegge steveyegge merged commit c748838 into gastownhall:main Mar 15, 2026
8 of 10 checks passed
steveyegge added a commit that referenced this pull request Mar 15, 2026
@steveyegge
Merge PR #2610: fix(backup): accept UUID comment and event IDs during…
b7ff054 
… restore

Adds test coverage for UUID-format comment and event IDs during restore
(code changes already landed in #2597).

PR: #2610
Author: garitar
@trillium

trillium commented Mar 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thanks for digging through the UUID cleanup here.

While auditing the backup/restore behavior I confirmed the backup_restore.go json.Number -> string change in this PR does fix the separate round-trip restore failure on main.

I opened a narrower issue/PR pair for that specific regression in case it helps review or cherry-picking:

Not trying to step on the broader cleanup work here, just wanted the backup restore bug to have its own tight repro and passing regression proof.

@garitar All good, best part about open source is we're all building toward the same goal. I'm not personally attached to my code being used as long as it drives the project forward :)

marcodelpin added a commit to marcodelpin/beads that referenced this pull request Mar 20, 2026
@marcodelpin
merge: upstream v0.61.0 into fork
4dc3708 
Resolves cherry-pick conflicts (backup_git_transport, backup_restore,
circuit_test) by taking upstream versions — our cherry-picks (gastownhall#2604,
gastownhall#2596, gastownhall#2599, 3034015) are now included in upstream.

Key changes in v0.61.0:
- Schema v8: no_history column (migration 011)
- UUID primary keys cleanup (gastownhall#2597)
- CLI credential routing for Dolt server (gastownhall#2543)
- --no-history flag (gastownhall#2622)
- --claim-next on close (gastownhall#2594)
- Doctor bare-parent fallback fixes
- Prime SSOT tasks 1-12
@maphew maphew mentioned this pull request Mar 29, 2026
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@trillium @garitar @steveyegge