Sync applied migration versions, supersede invariant test, nav a11y, committed roadmap#141
Conversation
…v a11y, commit roadmap - Rename six migration files to the Supabase ledger versions recorded when they were applied to the Memory project on 2026-07-15 (adaptive profile versioning columns, operator action center, runner statuses, shadow context pack lab, shadow pack preflight, promotion request board), matching the precedent set for the adaptive log RLS migration. - Add the production-checkpoint regression test: repeated distills keep exactly one active pack per (user_id, namespace, pack_type), archive (never delete) older packs, and never touch other users' packs. - Mobile Queue-dot accessibility: mark the dot decorative and announce "Queue has attention items" via a visually-hidden span (new .pd-sr-only utility), since aria-label on a plain <i> is not reliably exposed. - Add docs/roadmap.md: the committed, evidence-backed roadmap that replaces the un-committed numbered list referenced by earlier commits/PRs. Validated with typecheck, lint, test (642 passed), build, and env:policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KuzYogRG6c6324Lq4N4aAC
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds screen-reader accessibility handling, profile supersession persistence and regression coverage, and Supabase schemas for Pandora operator actions, shadow packs, preflights, and promotion requests. The roadmap documents current status and gated follow-up work. ChangesNavigation accessibility
Pandora backend state and workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/unit/retrieval-eval.test.ts (1)
212-218: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSimplify and strengthen the active pack assertions.
The current loop only verifies that if an active pack exists for a key, its count is exactly 1. If a bug were to mistakenly archive or delete all packs for a user, the key wouldn't exist in
activeCounts, and the test would silently pass this check.You can make the test more robust and readable by removing the
Mapentirely and explicitly asserting the active count for the user's namespaces.♻️ Proposed refactor
- const activeCounts = new Map<string, number>(); - for (const p of store.memory_context_packs) { - if (p.status !== "active") continue; - const key = `${p.user_id}/${p.namespace}/${p.pack_type}`; - activeCounts.set(key, (activeCounts.get(key) ?? 0) + 1); - } - for (const [key, count] of activeCounts) expect(count, `active packs for ${key}`).toBe(1); + expect(store.memory_context_packs.filter(p => p.status === "active" && p.user_id === USER && p.namespace === "au")).toHaveLength(1); + expect(store.memory_context_packs.filter(p => p.status === "active" && p.user_id === USER && p.namespace === "real_life")).toHaveLength(1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/retrieval-eval.test.ts` around lines 212 - 218, Replace the activeCounts Map and iteration in the retrieval evaluation test with explicit assertions for each expected user namespace, counting active entries directly from store.memory_context_packs. Ensure every expected namespace is asserted to have exactly one active pack, including cases where the count is zero.supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql (2)
13-15: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider creating the index concurrently.
Since you are adding an index to an existing table, creating it
CONCURRENTLYgenerally prevents write-locking the table. However, since thesupersedes_profile_idcolumn was just added and is entirelyNULLfor existing rows, the partial index (where supersedes_profile_id is not null) will be populated instantly. The write-lock duration is near zero.If your deployment process strictly enforces concurrent index creation (e.g., to resolve the static analysis warning), remember that Supabase migrations run inside a transaction by default. You will need to explicitly commit the transaction first or disable the DDL transaction wrapper to use
CONCURRENTLY.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql` around lines 13 - 15, Update the memory_profiles_supersedes_idx creation to use concurrent index creation if the migration deployment policy requires it, and configure the migration to run outside the default transaction so CONCURRENTLY is valid. Otherwise, retain the current non-concurrent partial index because the newly added nullable column leaves existing rows unindexed.Source: Linters/SAST tools
10-10: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a foreign key constraint for data integrity.
Since
supersedes_profile_idreferences a previous active profile, enforce referential integrity to prevent orphaned references if a profile is deleted.♻️ Proposed refactor
-alter table public.memory_profiles add column if not exists supersedes_profile_id uuid; +alter table public.memory_profiles add column if not exists supersedes_profile_id uuid references public.memory_profiles(id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql` at line 10, Add a foreign key constraint for the supersedes_profile_id column introduced by the ALTER TABLE statement, referencing the primary key of public.memory_profiles. Preserve the existing nullable column behavior and ensure the constraint prevents references to deleted profiles.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/globals.css`:
- Line 204: Update the .pd-sr-only rule by removing the deprecated clip
declaration and adding clip-path: inset(50%) to preserve its visually hidden
behavior. Keep the remaining accessibility styles unchanged.
In `@supabase/migrations/20260715200058_pandora_operator_action_center.sql`:
- Line 16: Following the existing trigger pattern used by
pandora_shadow_context_packs, create and attach a before-update updated_at
trigger for public.pandora_operator_actions in
supabase/migrations/20260715200058_pandora_operator_action_center.sql (line 16)
and public.pandora_shadow_pack_preflights in
supabase/migrations/20260715200149_pandora_shadow_pack_preflight.sql (line 15),
using the established timestamp-refresh function.
In `@supabase/migrations/20260715200130_pandora_shadow_context_pack_lab.sql`:
- Around line 1-2: Update the pandora_operator_actions_action_type_check
constraint to include prepare_shadow_pack_preflight and
prepare_promotion_request alongside the existing action types, so proposals
using these OperatorActionType values satisfy the database contract.
---
Nitpick comments:
In `@supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql`:
- Around line 13-15: Update the memory_profiles_supersedes_idx creation to use
concurrent index creation if the migration deployment policy requires it, and
configure the migration to run outside the default transaction so CONCURRENTLY
is valid. Otherwise, retain the current non-concurrent partial index because the
newly added nullable column leaves existing rows unindexed.
- Line 10: Add a foreign key constraint for the supersedes_profile_id column
introduced by the ALTER TABLE statement, referencing the primary key of
public.memory_profiles. Preserve the existing nullable column behavior and
ensure the constraint prevents references to deleted profiles.
In `@tests/unit/retrieval-eval.test.ts`:
- Around line 212-218: Replace the activeCounts Map and iteration in the
retrieval evaluation test with explicit assertions for each expected user
namespace, counting active entries directly from store.memory_context_packs.
Ensure every expected namespace is asserted to have exactly one active pack,
including cases where the count is zero.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 108fec00-8dd6-4418-958b-78be8b61efc6
📒 Files selected for processing (10)
app/globals.csscomponents/pandora/MobileBottomNav.tsxdocs/roadmap.mdsupabase/migrations/20260715200029_adaptive_profile_versioning_columns.sqlsupabase/migrations/20260715200058_pandora_operator_action_center.sqlsupabase/migrations/20260715200108_pandora_operator_action_runner_statuses.sqlsupabase/migrations/20260715200130_pandora_shadow_context_pack_lab.sqlsupabase/migrations/20260715200149_pandora_shadow_pack_preflight.sqlsupabase/migrations/20260715200514_pandora_promotion_request_board.sqltests/unit/retrieval-eval.test.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/unit/retrieval-eval.test.ts (1)
212-218: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSimplify and strengthen the active pack assertions.
The current loop only verifies that if an active pack exists for a key, its count is exactly 1. If a bug were to mistakenly archive or delete all packs for a user, the key wouldn't exist in
activeCounts, and the test would silently pass this check.You can make the test more robust and readable by removing the
Mapentirely and explicitly asserting the active count for the user's namespaces.♻️ Proposed refactor
- const activeCounts = new Map<string, number>(); - for (const p of store.memory_context_packs) { - if (p.status !== "active") continue; - const key = `${p.user_id}/${p.namespace}/${p.pack_type}`; - activeCounts.set(key, (activeCounts.get(key) ?? 0) + 1); - } - for (const [key, count] of activeCounts) expect(count, `active packs for ${key}`).toBe(1); + expect(store.memory_context_packs.filter(p => p.status === "active" && p.user_id === USER && p.namespace === "au")).toHaveLength(1); + expect(store.memory_context_packs.filter(p => p.status === "active" && p.user_id === USER && p.namespace === "real_life")).toHaveLength(1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/retrieval-eval.test.ts` around lines 212 - 218, Replace the activeCounts Map and iteration in the retrieval evaluation test with explicit assertions for each expected user namespace, counting active entries directly from store.memory_context_packs. Ensure every expected namespace is asserted to have exactly one active pack, including cases where the count is zero.supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql (2)
13-15: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider creating the index concurrently.
Since you are adding an index to an existing table, creating it
CONCURRENTLYgenerally prevents write-locking the table. However, since thesupersedes_profile_idcolumn was just added and is entirelyNULLfor existing rows, the partial index (where supersedes_profile_id is not null) will be populated instantly. The write-lock duration is near zero.If your deployment process strictly enforces concurrent index creation (e.g., to resolve the static analysis warning), remember that Supabase migrations run inside a transaction by default. You will need to explicitly commit the transaction first or disable the DDL transaction wrapper to use
CONCURRENTLY.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql` around lines 13 - 15, Update the memory_profiles_supersedes_idx creation to use concurrent index creation if the migration deployment policy requires it, and configure the migration to run outside the default transaction so CONCURRENTLY is valid. Otherwise, retain the current non-concurrent partial index because the newly added nullable column leaves existing rows unindexed.Source: Linters/SAST tools
10-10: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a foreign key constraint for data integrity.
Since
supersedes_profile_idreferences a previous active profile, enforce referential integrity to prevent orphaned references if a profile is deleted.♻️ Proposed refactor
-alter table public.memory_profiles add column if not exists supersedes_profile_id uuid; +alter table public.memory_profiles add column if not exists supersedes_profile_id uuid references public.memory_profiles(id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql` at line 10, Add a foreign key constraint for the supersedes_profile_id column introduced by the ALTER TABLE statement, referencing the primary key of public.memory_profiles. Preserve the existing nullable column behavior and ensure the constraint prevents references to deleted profiles.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/globals.css`:
- Line 204: Update the .pd-sr-only rule by removing the deprecated clip
declaration and adding clip-path: inset(50%) to preserve its visually hidden
behavior. Keep the remaining accessibility styles unchanged.
In `@supabase/migrations/20260715200058_pandora_operator_action_center.sql`:
- Line 16: Following the existing trigger pattern used by
pandora_shadow_context_packs, create and attach a before-update updated_at
trigger for public.pandora_operator_actions in
supabase/migrations/20260715200058_pandora_operator_action_center.sql (line 16)
and public.pandora_shadow_pack_preflights in
supabase/migrations/20260715200149_pandora_shadow_pack_preflight.sql (line 15),
using the established timestamp-refresh function.
In `@supabase/migrations/20260715200130_pandora_shadow_context_pack_lab.sql`:
- Around line 1-2: Update the pandora_operator_actions_action_type_check
constraint to include prepare_shadow_pack_preflight and
prepare_promotion_request alongside the existing action types, so proposals
using these OperatorActionType values satisfy the database contract.
---
Nitpick comments:
In `@supabase/migrations/20260715200029_adaptive_profile_versioning_columns.sql`:
- Around line 13-15: Update the memory_profiles_supersedes_idx creation to use
concurrent index creation if the migration deployment policy requires it, and
configure the migration to run outside the default transaction so CONCURRENTLY
is valid. Otherwise, retain the current non-concurrent partial index because the
newly added nullable column leaves existing rows unindexed.
- Line 10: Add a foreign key constraint for the supersedes_profile_id column
introduced by the ALTER TABLE statement, referencing the primary key of
public.memory_profiles. Preserve the existing nullable column behavior and
ensure the constraint prevents references to deleted profiles.
In `@tests/unit/retrieval-eval.test.ts`:
- Around line 212-218: Replace the activeCounts Map and iteration in the
retrieval evaluation test with explicit assertions for each expected user
namespace, counting active entries directly from store.memory_context_packs.
Ensure every expected namespace is asserted to have exactly one active pack,
including cases where the count is zero.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 108fec00-8dd6-4418-958b-78be8b61efc6
📒 Files selected for processing (10)
app/globals.csscomponents/pandora/MobileBottomNav.tsxdocs/roadmap.mdsupabase/migrations/20260715200029_adaptive_profile_versioning_columns.sqlsupabase/migrations/20260715200058_pandora_operator_action_center.sqlsupabase/migrations/20260715200108_pandora_operator_action_runner_statuses.sqlsupabase/migrations/20260715200130_pandora_shadow_context_pack_lab.sqlsupabase/migrations/20260715200149_pandora_shadow_pack_preflight.sqlsupabase/migrations/20260715200514_pandora_promotion_request_board.sqltests/unit/retrieval-eval.test.ts
🛑 Comments failed to post (3)
app/globals.css (1)
204-204: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the deprecated
clipproperty.Stylelint reports
clipas deprecated. Useclip-path: inset(50%)instead to retain the visually hidden behavior without a deprecated declaration.🧰 Tools
🪛 Stylelint (17.14.0)
[error] 204-204: Deprecated property "clip" (property-no-deprecated)
(property-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/globals.css` at line 204, Update the .pd-sr-only rule by removing the deprecated clip declaration and adding clip-path: inset(50%) to preserve its visually hidden behavior. Keep the remaining accessibility styles unchanged.Source: Linters/SAST tools
supabase/migrations/20260715200058_pandora_operator_action_center.sql (1)
16-16: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Missing
updated_attriggers. Both tables define anupdated_atcolumn but omit the database trigger required to automatically refresh it upon row modifications. This deviates from the robust pattern established for other tables in this PR (e.g.,pandora_shadow_context_packs) and introduces the risk of stale timestamps if the application layer fails to manually supply the new time on every update.
supabase/migrations/20260715200058_pandora_operator_action_center.sql#L16-L16: create and attach abefore updatetrigger forpublic.pandora_operator_actions.supabase/migrations/20260715200149_pandora_shadow_pack_preflight.sql#L15-L15: create and attach abefore updatetrigger forpublic.pandora_shadow_pack_preflights.📍 Affects 2 files
supabase/migrations/20260715200058_pandora_operator_action_center.sql#L16-L16(this comment)supabase/migrations/20260715200149_pandora_shadow_pack_preflight.sql#L15-L15🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260715200058_pandora_operator_action_center.sql` at line 16, Following the existing trigger pattern used by pandora_shadow_context_packs, create and attach a before-update updated_at trigger for public.pandora_operator_actions in supabase/migrations/20260715200058_pandora_operator_action_center.sql (line 16) and public.pandora_shadow_pack_preflights in supabase/migrations/20260715200149_pandora_shadow_pack_preflight.sql (line 15), using the established timestamp-refresh function.supabase/migrations/20260715200130_pandora_shadow_context_pack_lab.sql (1)
1-2: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Add missing
action_typevalues to the constraint.This migration updates the
pandora_operator_actions_action_type_checkconstraint, but omits'prepare_shadow_pack_preflight'and'prepare_promotion_request'. The features added in subsequent migrations within this PR rely on these upstreamOperatorActionTypevalues to function.If this constraint is not updated to include them, proposing either of those action types will result in a hard database constraint violation.
🛡️ Proposed fix to sync with the application contract
alter table public.pandora_operator_actions drop constraint if exists pandora_operator_actions_action_type_check; -alter table public.pandora_operator_actions add constraint pandora_operator_actions_action_type_check check (action_type in ('verify_namespace_invariants','verify_pack_supersession','check_retrieval_eval_status','refresh_dashboard_snapshot','prepare_distill_smoke_plan','prepare_shadow_context_pack')); +alter table public.pandora_operator_actions add constraint pandora_operator_actions_action_type_check check (action_type in ('verify_namespace_invariants','verify_pack_supersession','check_retrieval_eval_status','refresh_dashboard_snapshot','prepare_distill_smoke_plan','prepare_shadow_context_pack','prepare_shadow_pack_preflight','prepare_promotion_request'));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.alter table public.pandora_operator_actions drop constraint if exists pandora_operator_actions_action_type_check; alter table public.pandora_operator_actions add constraint pandora_operator_actions_action_type_check check (action_type in ('verify_namespace_invariants','verify_pack_supersession','check_retrieval_eval_status','refresh_dashboard_snapshot','prepare_distill_smoke_plan','prepare_shadow_context_pack','prepare_shadow_pack_preflight','prepare_promotion_request'));🧰 Tools
🪛 Squawk (2.59.0)
[warning] 2-2: By default new constraints require a table scan and block writes to the table while that scan occurs. Use
NOT VALIDwith a laterVALIDATE CONSTRAINTcall.(constraint-missing-not-valid)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260715200130_pandora_shadow_context_pack_lab.sql` around lines 1 - 2, Update the pandora_operator_actions_action_type_check constraint to include prepare_shadow_pack_preflight and prepare_promotion_request alongside the existing action types, so proposals using these OperatorActionType values satisfy the database contract.
2 participants
Checklist
What
20260715200029–20260715200514), matching the precedent set by the adaptive log RLS migration sync. These are the five previously-drifted migrations plus the promotion request board migration from Add Pandora Promotion Request Board v1 #138.(user_id, namespace, pack_type), archive (never delete) older packs, and never touch other users' packs..pd-sr-onlyutility), sincearia-labelon a plain<i>is not reliably exposed to assistive tech.docs/roadmap.md— the committed, evidence-backed roadmap replacing the numbered list previously referenced only in commit messages.Verification
typecheck ✅ · lint ✅ (0 errors, 2 pre-existing warnings in untouched files) · test 642/642 ✅ · build ✅ · env:policy ✅ (89 keys)
🤖 Generated with Claude Code
https://claude.ai/code/session_01KuzYogRG6c6324Lq4N4aAC
Generated by Claude Code
Summary by CodeRabbit
Accessibility
New Features
Documentation
Bug Fixes