Skip to content

fix(sync): make the reconcileSamePath winner HLC-monotonic (P4-QUAL-02 follow-up)#95

Merged
Reederey87 merged 1 commit into
mainfrom
fix/reconcile-same-path-hlc-monotonic
Jul 4, 2026
Merged

fix(sync): make the reconcileSamePath winner HLC-monotonic (P4-QUAL-02 follow-up)#95
Reederey87 merged 1 commit into
mainfrom
fix/reconcile-same-path-hlc-monotonic

Conversation

@Reederey87

@Reederey87 Reederey87 commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • reconcileSamePath now installs the highest (HLC, deviceID, eventID) coordinate as the same-path/different-remote winner (one-line comparison flip in internal/sync/events.go), matching same-remote LWW (decideUpsert) and snapshot import (importEntryTx). The previous lowest-coordinate winner was the root cause of both known order-dependence divergences (PR test(sync): property/model-check layer over the pure Decide seam (P4-QUAL-02) #94's handoff): the installed row's source HLC could sit below a dropped rival's, so a mid-HLC delete — or a same-remote LWW lift racing the cross-remote reconcile — flipped the terminal state by delivery order.
  • Both witness tripwires (TestDecideDifferentRemote{Delete,MultiEvent}DivergesWitness) fired exactly as designed and were deleted per their own failure-message protocol, together with both genEventSet exclusions: the property/model-check layer now runs the full event space (deletes + multi-event remotes mixed freely across different-remote paths, 1-3 remote pool per path).
  • Winner-direction test assertions inverted (hlc_test.go ×3, decide_property_test.go, conflicts_test.go); the apply_test.go conflict.resolved fingerprint fixtures are literal-match only and needed no change.
  • Specs: spec/07 (Decide-seam winner rule + model-check section marked fixed + conflict-replay §), spec/16 (generator exclusion-free; tripwire pattern kept as methodology), spec/13 (conflicts resolve names the interim installed winner), ledger rows P5-ARCH-01/P4-QUAL-02 (residual/follow-up SHIPPED), work-log entry.

Validation

  • gofmt clean; golangci-lint run 0 issues; spec-drift pass (20 specs, 12 changed files).
  • go test -race ./... pass — the widened generator drives the rapid convergence, import≡replay, and 3-replica model suites over the previously-excluded classes.
  • go test -fuzz=FuzzDecideConvergence -fuzztime=60s ./internal/sync/: 1.48M execs, no crash.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Conflict resolution for same-path/different-remote cases now consistently picks the highest-ranked change, making results stable regardless of arrival order.
    • Related conflict handling in the CLI now matches the updated winner selection and keeps the chosen project variant as expected.
  • Documentation

    • Updated sync, CLI, and test-plan docs to describe the new conflict-resolution behavior.
    • Noted broader event coverage and removal of outdated exclusions in the reconciliation model.

…2 follow-up)

The same-path/different-remote winner is now the HIGHEST (HLC, deviceID,
eventID) coordinate — consistent with same-remote last-writer-wins and
snapshot import — instead of the lowest. The lowest-coordinate rule let the
installed row's source HLC sit below a dropped rival's, making delete/
different-remote mixes and multi-event-per-remote mixes diverge by delivery
order. Both witness tripwires fired as designed and were retired with their
generator exclusions; genEventSet now draws the full event space (deletes +
multi-event remotes across different-remote paths).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Reederey87 Reederey87 enabled auto-merge (squash) July 4, 2026 17:50
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 30db24c3-743a-4780-80ec-c1efedaecec7

📥 Commits

Reviewing files that changed from the base of the PR and between bf69cfb and 7025811.

📒 Files selected for processing (12)
  • docs/audits/README.md
  • internal/cli/conflicts_test.go
  • internal/sync/decide.go
  • internal/sync/decide_property_test.go
  • internal/sync/decide_rapid_test.go
  • internal/sync/events.go
  • internal/sync/hlc_test.go
  • internal/sync/property_helpers_test.go
  • spec/07_NAMESPACE_AND_SYNC_MODEL.md
  • spec/13_CLI_DAEMON_API.md
  • spec/16_TEST_PLAN.md
  • spec/18_WORK_LOG.md

📝 Walkthrough

Walkthrough

This PR fixes reconcileSamePath's same-path/different-remote conflict resolution to select the highest (HLC, deviceID, eventID) coordinate as winner instead of the lowest, retires previously excluded divergence-witness tests, widens the property-test event generator, and updates dependent test expectations and spec/documentation.

Changes

HLC-monotonic reconciliation fix

Layer / File(s) Summary
Core winner selection logic
internal/sync/events.go, internal/sync/decide.go
reconcileSamePath's samePathLess comparison is flipped so the highest (HLC, deviceID, eventID) coordinate becomes canonical winner; comments updated to describe HLC-monotonic behavior.
Unit test expectation updates
internal/sync/hlc_test.go, internal/cli/conflicts_test.go
Assertions updated to expect the higher-coordinate "other" remote as winner (remote keys, SourceEventHLC/SourceEventID, incomingWins flags, WinnerKey/LoserKey).
Property test and generator updates
internal/sync/decide_property_test.go, internal/sync/decide_rapid_test.go, internal/sync/property_helpers_test.go
genEventSet now uses a per-path remote pool covering the full event space (deletes, multi-event remotes); two witness tripwire tests and their exclusions are removed; winner assertions updated to highest-coordinate expectations.
Spec and documentation updates
spec/07_NAMESPACE_AND_SYNC_MODEL.md, spec/13_CLI_DAEMON_API.md, spec/16_TEST_PLAN.md, spec/18_WORK_LOG.md, docs/audits/README.md
Spec text, CLI docs, test plan, work log, and audit README updated to describe HLC-monotonic winner selection and removal of generator exclusions/witness tests.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Event as Incoming Event
  participant Reconcile as reconcileSamePath
  participant Payload as ProjectPayload
  participant Conflict as Conflict Record

  Event->>Reconcile: submit event/remote for same path
  Reconcile->>Reconcile: compare via samePathLess(existing, incoming)
  Reconcile->>Payload: select highest (HLC, deviceID, eventID) as canonical winner
  Reconcile->>Conflict: record WinnerKey/LoserKey and incomingWins flag
  Reconcile-->>Event: return canonical payload
Loading

Compact metadata
Related issues: Not specified in the provided information.
Related PRs: Not specified in the provided information.
Suggested labels: bugfix, sync, tests
Suggested reviewers: Not specified in the provided information.

Poem
A rabbit hopped through HLC's maze,
Picked the highest coordinate, changed its ways.
Witnesses retired, their duty now done,
Generators freed to explore every one.
Winner or loser, monotonic and true—
Convergence achieved, the bug bids adieu. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers summary and validation, but it omits the required Tests checklist and Safety Checklist sections from the template. Add the missing Tests checklist items and the full Safety Checklist sections, keeping the template headings and checkboxes intact.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: making reconcileSamePath choose the HLC-monotonic winner.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/reconcile-same-path-hlc-monotonic

Comment @coderabbitai help to get the list of available commands.

@Reederey87 Reederey87 merged commit 9575735 into main Jul 4, 2026
6 of 7 checks passed
@Reederey87 Reederey87 deleted the fix/reconcile-same-path-hlc-monotonic branch July 4, 2026 17:55
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