fix(store): prevent duplicate observations on import

[ardelperal]

Linked Issue

Closes #531

---

PR Type

• type:bug - Bug fix
• type:feature - New feature
• type:docs - Documentation only
• type:refactor - Code refactoring (no behavior change)
• type:chore - Maintenance, dependencies, tooling
• type:breaking-change - Breaking change

---

Summary

• Prevent repeated imports from inserting observations with an existing sync_id.
• Report only observation rows actually inserted in ObservationsImported.
• Add regression coverage for the reported 3 -> 6 -> 9 scenario.

Changes

File	Change
internal/store/store.go	Insert an observation only when its normalized sync_id does not already exist in the import transaction.
internal/store/store_test.go	Import three observations three times and verify the store remains at three rows.

Test Plan

• Unit tests pass locally: go test ./...
• E2E tests pass locally: go test -tags e2e ./internal/server/...
• Manually tested the affected functionality (covered by the focused automated regression test)

Focused verification:

• go test ./internal/store -run TestImportSkipsObservationWithExistingSyncID -count=1
• go vet ./internal/store
• TDD red phase confirmed before the fix: the second import reported one imported observation instead of zero.

The full Windows unit suite currently has unrelated baseline failures in cmd/engram, internal/setup, internal/store, and internal/sync. The failing internal/store, internal/setup, and internal/sync cases were reproduced unchanged from a clean origin/main worktree at be4b613.

---

Automated Checks

Check	What it verifies	Status
Check Issue Reference	PR body contains Closes #N / Fixes #N / Resolves #N	Pending
Check Issue Has status:approved	Linked issue has status:approved label	Pending
Check PR Has type: Label*	PR has exactly one type:* label	Pending maintainer permission
Unit Tests	go test ./... passes	Pending CI
E2E Tests	go test -tags e2e ./internal/server/... passes	Pending CI

---

Contributor Checklist

• I linked an approved issue above (Closes #531)
• I added exactly one type:* label to this PR (pending maintainer permission)
• I ran unit tests locally: go test ./... (ran; pre-existing Windows failures remain)
• I ran e2e tests locally: go test -tags e2e ./internal/server/...
• Docs updated (not required; no user-facing workflow or API contract changed)
• Commits follow conventional commits format
• No Co-Authored-By trailers in commits

---

Notes for Reviewers

This stays within the scope approved in #531: dedupe observations during import. It does not repair historical duplicates, change prompt import behavior, or implement the LWW redesign discussed in #244.

The schema currently has a non-unique index on observations.sync_id. The implementation therefore uses an atomic INSERT ... SELECT ... WHERE NOT EXISTS inside the existing import transaction rather than adding a unique constraint that could fail migration for databases already containing duplicates.

Summary by CodeRabbit

• Bug Fixes
  • Prevented duplicate observations when importing records with an existing synchronization ID.
  • Import results now accurately report only newly added observations.
  • Repeated imports no longer create additional duplicate observation records.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c5255d8e-61a7-4018-abe8-ed680a52f9d6

📥 Commits

Reviewing files that changed from the base of the PR and between be4b613 and f54ddfe.

📒 Files selected for processing (2)

• internal/store/store.go
• internal/store/store_test.go

---

📝 Walkthrough

Walkthrough

Observation imports now derive normalized sync IDs, skip existing observations, count only inserted rows, and return contextual insertion errors. A regression test verifies repeated imports preserve the original three observations.

Changes

Observation import deduplication

Layer / File(s)	Summary
Deduplicate observation inserts internal/store/store.go	Observation imports conditionally insert by sync_id, increment ObservationsImported using affected rows, and add observation ID context to insertion errors.
Validate repeated imports internal/store/store_test.go	TestImportSkipsObservationWithExistingSyncID verifies the first import adds three observations, repeated imports add none, and exactly three matching rows remain.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: gentleman-programming, alan-thegentleman

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main fix: preventing duplicate observations during import.
Linked Issues check	✅ Passed	The import path now deduplicates observations by sync_id and the test verifies repeated imports stay at three rows.
Out of Scope Changes check	✅ Passed	The changes stay focused on import deduplication and regression coverage, with no unrelated behavior changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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