add GitHub Actions workflow for Postgres memory tests by AmitKarnam · Pull Request #95 · Siddhant-K-code/distill

AmitKarnam · 2026-05-09T12:48:54Z

Plan to work on Issue #74 . Starting with a CI workflow, will continue with postgres.go and postgres_test.go in the memory package

▎ pgx/v5 is a direct dependency in go.mod even though Postgres support is gated behind //go:build postgres. The Go module system doesn't support build-tag-conditional dependencies, so this is intentional — it's a small overhead accepted in exchange for keeping the module structure simple. A separate module would be needed to fully isolate it.

AmitKarnam · 2026-05-09T12:50:25Z

@Siddhant-K-code, this is first on the resolution, going forward i will work on the core implementation on PostgreSQL. Do let me know if there are any conflicts with this plan

Siddhant-K-code

Thanks for picking up #74! A CI workflow is the right starting point, but this PR can't merge without the actual implementation alongside it.

Blockers

go test -tags=postgres ./... will find nothing to run -- there's no postgres.go or postgres_test.go in pkg/memory/. The workflow would either silently pass (no-op) or fail. Please include the implementation in this PR.
Issue #74 has specific requirements before merging: OnLifecycleEvent must be implemented (not a no-op), and the //go:build postgres tag is needed to keep the default binary lean (pgx adds ~5MB).

Workflow issues

Actions are unpinned (actions/checkout@v4, actions/setup-go@v4). The repo pins all actions to full commit SHAs -- please match that convention.
The "Wait for Postgres" step is redundant -- the service container already has --health-cmd pg_isready with retries. Remove it.

Happy to review again once postgres.go, postgres_test.go, and the lifecycle event implementation are included.

…es readiness

…nflict handling

…PostgresStore

…tgresStore tests - Removed `buildCacheBoundaryHint` and `buildSensitivityMetadata` functions from SQLiteStore as they were not utilized. - Introduced comprehensive test suite for PostgresStore covering core functionalities, including storing, recalling, forgetting, and expiring memories. - Added tests for lifecycle events, deduplication, and TTL handling in PostgresStore.

AmitKarnam · 2026-05-24T12:55:10Z

@Siddhant-K-code PTAL

Siddhant-K-code · 2026-05-24T15:32:37Z

+          --health-interval 5s --health-timeout 5s --health-retries 5
+
+    steps:
+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1


Wrong action version — actions/checkout v6 doesn't exist.

The current stable release is v4. This SHA either belongs to a different action or was fabricated. Use the correct pin:

- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

Or simply actions/checkout@v4 if you're not pinning by SHA.

@Siddhant-K-code this is the same checkout version used in the CI.yml

Siddhant-K-code · 2026-05-24T15:32:43Z

+      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+
+      - name: Set up Go
+        uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0


Same issue — actions/setup-go v6 doesn't exist. Current stable is v5.

- uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0

Points to the same commit hash used in the ci.yml to setup go 1.24. Would you want me to downgrade this?

Siddhant-K-code · 2026-05-24T15:32:49Z

+        env:
+          POSTGRES_DSN: postgres://postgres:postgres@localhost:5432/distill_test?sslmode=disable
+        run: |
+          go test -tags=postgres ./... -v


This will pass vacuously right now — there are no *_test.go files with the postgres build tag yet, so go test -tags=postgres ./... runs zero postgres-specific tests and exits 0.

That's fine for a draft, but worth noting: the CI green light here doesn't mean the implementation is tested. Tests need to land before this workflow is meaningful.

@Siddhant-K-code The postgres_test.go ( ref ) is present with the postgres tag, go running the tag will run the required test suite for postgres package.

Siddhant-K-code · 2026-05-24T15:33:00Z

 go 1.24.0

 require (
+	github.com/jackc/pgx/v5 v5.7.2


pgx/v5 is added as an unconditional direct dependency, but the Postgres code is behind a //go:build postgres tag.

This means every default build — including CI, local dev, and releases — downloads and links pgx even when Postgres support isn't used. That's unnecessary bloat for users who only need SQLite.

Options:

Accept it (simplest — pgx is a well-maintained library and the overhead is small).

Move pgx to an indirect comment with a note explaining it's only active with -tags=postgres.

Either way, please add a comment in the PR description explaining the decision so it's not a surprise to reviewers.

Added in PR description

Siddhant-K-code · 2026-05-24T15:33:12Z

@@ -150,19 +157,12 @@ go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.40.0 h1:MzfofMZN8ulNqob
 go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.40.0/go.mod h1:E73G9UFtKRXrxhBsHtG00TB5WxX57lpsQzogDkqBTz8=


Unrelated change — the OTel v1.43.0 removals in this file don't belong in this PR.

These are a rebase artifact from PR #75 (the Go version fix). Please rebase onto main to drop them — they make the diff harder to review and have nothing to do with Postgres support.

Merged latest main into this branch

Siddhant-K-code · 2026-05-24T15:33:23Z

+
+// buildCacheBoundaryHint derives a hint from recalled memories.
+// Entries with relevance >= 0.7 are treated as stable this turn.
+func buildCacheBoundaryHint(memories []RecalledMemory) *CacheBoundaryHint {


These two helpers are only used by postgres.go — why are they in the shared helpers.go?

buildCacheBoundaryHint and buildSensitivityMetadata are not called by sqlite.go. Adding them to a shared file without a shared consumer is confusing — a reader of helpers.go will wonder where they're used.

Move them into postgres.go, or (better) call them from sqlite.go too so the SQLite backend gets the same behaviour. The SQLite Recall path likely has the same gap.

Both these methods were being used in sqlite.go ref and also were needed in postgres.go, hence I moved them to helpers.go

Siddhant-K-code · 2026-05-24T15:33:31Z

+	CREATE INDEX IF NOT EXISTS idx_memories_referenced ON memories(last_referenced);
+	CREATE INDEX IF NOT EXISTS idx_memories_expired   ON memories(expired);
+	`
+	_, err := ps.dbPool.Exec(context.Background(), schema)


Run each DDL statement separately, not as one concatenated string.

When multiple statements are batched into a single Exec, a failure mid-way gives you an error message that doesn't tell you which statement failed. Separate calls make failures obvious:

stmts := []string{ `CREATE TABLE IF NOT EXISTS memories (...)`, `CREATE TABLE IF NOT EXISTS memory_tags (...)`, `CREATE INDEX IF NOT EXISTS idx_memory_tags_tag ON ...`, // ... } for _, s := range stmts { if _, err := ps.dbPool.Exec(ctx, s); err != nil { return err } }

Addressed in commit

Siddhant-K-code · 2026-05-24T15:33:52Z

Good start on the Postgres backend. Since this is a draft I'm not blocking, but flagging the things that need to be fixed before it's ready to merge.

Must fix before merge

Wrong action versions in the CI workflow — actions/checkout and actions/setup-go are pinned to non-existent v6 SHAs. See inline comments on the workflow file.
Unrelated go.sum changes — OTel v1.43.0 removals are a rebase artifact from fix: revert go directive to 1.24.0 after dependabot bump #75. Rebase onto main to drop them.
buildCacheBoundaryHint / buildSensitivityMetadata in helpers.go — these are only used by postgres.go. Either move them there, or wire them into sqlite.go too (which probably has the same gap). See inline comment.

Worth addressing before merge

pgx/v5 as an unconditional direct dep — the Postgres code is behind a build tag but the dependency is always present. Decide whether that's intentional and document it in the PR description. See inline comment on go.mod.
migrate() batches all DDL into one Exec call — split into separate calls so failures are identifiable. See inline comment.

Known / expected for a draft

No postgres-tagged tests yet — the CI workflow will pass vacuously until they land. That's fine, just make sure tests arrive before this comes out of draft.

…L statements

AmitKarnam · 2026-05-25T16:09:53Z

@Siddhant-K-code have addressed the review comments' PTAL

AmitKarnam · 2026-06-15T08:36:15Z

@Siddhant-K-code is this feature for the project still being actively pursued? Wanted to confirm to dig deeper into the project.

Thank you!

Siddhant-K-code · 2026-06-15T11:53:23Z

Hey @AmitKarnam, I've been occupied with multiple things at the moment, will come back to review it ASAP!

add GitHub Actions workflow for Postgres memory tests

8d9187d

Siddhant-K-code reviewed May 10, 2026

View reviewed changes

chore(tests): update actions versions and remove wait step for Postgr…

9bfd396

…es readiness

Siddhant-K-code marked this pull request as draft May 14, 2026 07:25

AmitKarnam added 4 commits May 15, 2026 01:29

feat(postgres): implement PostgresStore with memory management and co…

9c3682d

…nflict handling

feat(postgres): add findSimilar method for cosine distance search in …

89bf319

…PostgresStore

feat(postgres): implement Recall and Forget methods in PostgresStore

46dd3ec

Siddhant-K-code reviewed May 24, 2026

View reviewed changes

AmitKarnam and others added 2 commits May 25, 2026 21:37

refactor(postgres): streamline migration logic by using a loop for SQ…

fb781b4

…L statements

Merge branch 'main' into feature/postgres-memory-store

f033709

Siddhant-K-code marked this pull request as ready for review June 15, 2026 11:53

Siddhant-K-code self-requested a review June 15, 2026 11:53

		@@ -150,19 +157,12 @@ go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.40.0 h1:MzfofMZN8ulNqob
		go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.40.0/go.mod h1:E73G9UFtKRXrxhBsHtG00TB5WxX57lpsQzogDkqBTz8=

Uh oh!

Conversation

AmitKarnam commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmitKarnam commented May 9, 2026

Uh oh!

Siddhant-K-code left a comment

Choose a reason for hiding this comment

Uh oh!

AmitKarnam commented May 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmitKarnam May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Siddhant-K-code commented May 24, 2026

Uh oh!

AmitKarnam commented May 25, 2026

Uh oh!

AmitKarnam commented Jun 15, 2026

Uh oh!

Siddhant-K-code commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AmitKarnam commented May 9, 2026 •

edited

Loading

AmitKarnam May 25, 2026 •

edited

Loading