[schemas] Reflections — structured reasoning traces linked to thoughts

[alanshurafa]

Summary

• Adds reflections table with uuid PK, FK to thoughts(id), and 5 reflection types (decision, analysis, evaluation, planning, retrospective)
• upsert_reflection RPC for insert-or-update by ID with metadata merging
• match_reflections RPC for semantic similarity search over reflection embeddings (HNSW index)
• Service-role grants, RLS enabled, auto-updated updated_at trigger

Motivation

AI agents and personal knowledge systems benefit from recording not just decisions but the reasoning behind them. Reflections capture trigger context, options considered, factors weighed, and conclusions — making past deliberation searchable and reusable.

Test plan

• Run migration.sql in Supabase SQL Editor
• Verify reflections table exists via information_schema.tables
• Verify both RPCs exist via information_schema.routines
• Insert a test reflection via upsert_reflection and verify it returns a uuid
• Update an existing reflection by passing p_id and verify metadata merge
• Verify match_reflections returns results when embedding is provided

🤖 Generated with Claude Code

[matthallett1]

@claude review this PR

[github-actions]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[matthallett1]

@claude review this PR

[github-actions]

Claude finished @matthallett1's task in 1m 32s —— View job

---

PR Review: [schemas] Reflections — structured reasoning traces linked to thoughts

• Gather context and read changed files
• Review migration.sql for safety and design patterns
• Review README.md for clarity and completeness
• Review metadata.json for correctness
• Check alignment with OB1 design patterns and CONTRIBUTING.md
• Post final review

---

Summary

This is a well-conceived schema extension that fits squarely in OB1's mission. The core idea — capturing structured deliberation traces alongside thoughts — is genuinely useful for AI agents and knowledge systems. The SQL is safe, RLS is enabled, grants are applied, and the semantic search RPC follows OB1's established match_thoughts pattern. A few issues need addressing before this merges.

---

Issues

1. Inconsistent null-handling in upsert_reflection (migration.sql lines 82–90) — Should Fix

The update branch inconsistently handles nulls. options and factors always overwrite with the passed value (defaulting to '[]'), while most other fields use COALESCE to preserve existing values. confidence also overwrites with no coalesce.

This means calling upsert_reflection(p_id := some_id) — which is the natural "I just want to update the conclusion" use case — will silently reset options to '[]', factors to '[]', and confidence to null. That's destructive and surprising.

The PR description says "metadata merging" but only metadata actually merges. Every other field either clobbers or coalesces.

The fix should apply coalesce consistently or document explicitly which fields overwrite vs. preserve. Fix this →

2. "Coming soon" link in README (README.md line 119) — Should Fix

The "Works Well With" section references integrations/reflection-mcp/ which doesn't exist. Even labeled "coming soon," CI's internal link checker will likely flag it, and it can confuse users who try to navigate to it. Either remove this section until the integration exists, or note it without a folder path. Fix this →

3. metadata.json missing created and updated fields — Minor

The template and all CONTRIBUTING.md examples include created and updated fields. They're listed as optional but are present in every example. Fix this →

4. Misleading troubleshooting entry (README.md lines 114–116) — Minor

"upsert_reflection returns null — The thought ID you passed might not exist..."

The FK is on delete set null, not on insert reject. An insert with a non-existent thought_id will actually fail with a FK violation, not silently set thought_id to null. The on delete set null only applies when a referenced thought is deleted after the reflection exists. The troubleshooting tip conflates these two cases and gives incorrect guidance.

---

What's Working Well

• SQL is clean and safe — no destructive statements, no modifications to the thoughts table
• RLS enabled with service-role grants — correct pattern
• HNSW index with the right operator class (vector_cosine_ops) and sensible parameters (m=16, ef=64)
• match_reflections closely mirrors the match_thoughts pattern — consistent with the codebase
• The 5 reflection types are well-chosen and cover the useful space without over-engineering
• README has all required sections and the troubleshooting section is proactive
• Test plan in the PR description is thorough

---

Verdict: Approve with fixes. The two "should fix" items above are blocking — the destructive null behavior in upsert is a real data safety issue, and the broken forward reference will trip CI. The minor items are easy cleanup. Once those are addressed, this is ready to merge.

[matthallett1]

Good schema design for reflections. Two issues from Codex review:

P1: Partial updates silently erase data. upsert_reflection with an existing p_id but without p_options/p_factors/p_confidence will overwrite stored values with the defaults ([], [], NULL). A "change only the conclusion" update silently erases the original options/factors/confidence. Fix: use COALESCE(excluded.options, reflections.options) pattern or similar.

P2: Non-idempotent trigger. Everything else uses IF NOT EXISTS/CREATE OR REPLACE, but CREATE TRIGGER reflections_set_updated_at fails on re-run. Wrap in a DO block or use DROP TRIGGER IF EXISTS ... CREATE TRIGGER.