refactor: structured DuplicateContentError and promote.go helpers

[nvandessel]

Summary

• Structured error type: Replace fragile extractDuplicateNodeID string-parsing with store.DuplicateContentError struct. Both sqlite.go and memory.go now return &DuplicateContentError{ExistingID: id} instead of fmt.Errorf wrapping. Callers use errors.As to extract the existing node ID.
• getTargetNode helper: Extract repeated fetch-and-nil-check boilerplate from executeAbsorb, executeSupersede, and executeSupplement into a shared helper.
• mergeSourceEvents helper: Extract duplicated source-event deduplication loops from executeAbsorb and executeSupersede into a shared helper.

Test plan

• go test ./internal/store/... ./internal/consolidation/... -v — all pass
• go test ./... — only pre-existing lancedb linker failures (unrelated)
• go fmt ./... — clean

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

❌ Patch coverage is 88.13559% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.78%. Comparing base (686b17f) to head (69b66cf).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/consolidation/promote.go	86.79%	3 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   78.69%   78.78%   +0.08%     
==========================================
  Files         185      186       +1     
  Lines       18702    18685      -17     
==========================================
+ Hits        14718    14721       +3     
+ Misses       2752     2739      -13     
+ Partials     1232     1225       -7     

☔ View full report in Codecov by Sentry.
📢 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.

[greptile-apps]

Greptile Summary

This PR cleanly replaces the fragile extractDuplicateNodeID string-parsing approach with a proper store.DuplicateContentError struct, and extracts two shared helpers (getTargetNode, mergeSourceEvents) that eliminate repeated boilerplate from the three execute-merge functions. The DuplicateContentError.Is hook preserves full backward compatibility with errors.Is(err, ErrDuplicateContent) for existing callers while enabling errors.As extraction of ExistingID. Both store implementations (memory.go, sqlite.go) are updated consistently, and the new tests are thorough.

Key changes:

• store.DuplicateContentError struct with Error() and Is() — ErrDuplicateContent sentinel kept for backward compat
• memory.go and sqlite.go each return &DuplicateContentError{ExistingID: id} instead of a fmt.Errorf-wrapped sentinel
• promote.go main path uses errors.As to get ExistingID directly; supersede/supplement paths keep errors.Is (no ID needed there)
• getTargetNode helper consolidates the three identical fetch-and-nil-check blocks
• mergeSourceEvents helper deduplicates source-event lists with consistent []interface{} → []string conversion in executeSupersede
• Minor: the empty-ID defensive guard on pendingToActual was removed (see inline comment); multi-field struct literals on one line in two edge declarations are not gofmt-idiomatic

Confidence Score: 5/5

Safe to merge — refactoring is correct, all tests pass, and the two findings are minor style suggestions

All identified issues are P2 (a missing defensive empty-string guard that was never exercised in practice, and multi-field struct literals that deviate from gofmt convention). No logic bugs, no data-integrity risks, and the new DuplicateContentError type is backward-compatible with the existing ErrDuplicateContent sentinel. Score stays at 5 per the guidance that P2-only PRs should not be scored below 5.

No files require special attention — all changes are consistent and well-tested

Important Files Changed

Filename	Overview
internal/store/store.go	Adds DuplicateContentError struct with Error() and Is() methods, preserving backward-compat with ErrDuplicateContent sentinel via the Is hook
internal/store/memory.go	AddNode now returns &DuplicateContentError{ExistingID: id} directly instead of a fmt.Errorf wrapping; clean change
internal/store/sqlite.go	Same structured-error swap as memory.go — returns &DuplicateContentError{ExistingID} from addBehaviorWith
internal/consolidation/promote.go	Replaces string-parsed ErrDuplicateContent with errors.As, extracts getTargetNode and mergeSourceEvents helpers, removes the empty-ID guard on pendingToActual mapping; minor: multi-field struct literals on one line
internal/consolidation/promote_test.go	Good coverage added for getTargetNode (not-found, found, store-error) and mergeSourceEvents (dedup, empty, non-string); plus integration tests for unknown strategy and skip path
internal/store/store_test.go	New test file verifying DuplicateContentError.Error(), errors.Is, and errors.As — comprehensive unit coverage for the new type

Sequence Diagram

sequenceDiagram
    participant P as Promote
    participant S as GraphStore

    P->>S: AddNode(ctx, node)
    alt duplicate canonical content
        S-->>P: &DuplicateContentError{ExistingID}
        P->>P: errors.As → dupErr.ExistingID
        P->>P: pendingToActual[pendingID] = existingID
    else success
        S-->>P: nodeID, nil
        P->>P: pendingToActual[pendingID] = node.ID
    end

    P->>P: getTargetNode(ctx, s, targetID)
    P->>S: GetNode(ctx, targetID)
    S-->>P: *Node or nil
    alt nil → not found
        P-->>P: error "target node not found"
    else store error
        P-->>P: fmt.Errorf wrap
    else found
        P-->>P: *existing (value copy)
    end

    P->>P: mergeSourceEvents(existing, incoming)
    Note over P: dedup via seen map, append new events
    P-->>P: []interface{} merged list

Loading

_{Reviews (4): Last reviewed commit: "Merge branch 'main' into refactor/promot..." | Re-trigger Greptile}