feat(models): model registry reconciliation + persona seeds + readiness check

[POWERFULMOVES]

Summary

• Anthropic provider + Claude models added to model registry — personas now have valid model_preference targets (claude-sonnet-4-5, claude-opus-4-5, claude-haiku-4-5)
• 8 standard persona seeds integrated into active Supabase initdb pipeline (Developer, Researcher, Creator, Analyst, Archivist, Coordinator, Tester, Security)
• GPU models YAML reconciled — 10 models added to gpu-models.yaml for GPU Orchestrator VRAM scheduling
• Service-model mappings expanded from 4 to 15+ services (hirag, archon, coding, orchestrator, vl_sentinel, tts, extract_worker, etc.)
• Persona-model resolution view — SQL view joining persona→model→provider for runtime agent identity lookup
• Readiness check tool — make model-readiness validates registry, personas, Ollama, TensorZero at startup

TAC Tree Branches

Branch	Status	Scope
B: Model Registry Reconciliation	Done	Anthropic provider, 5 missing Ollama models, 6 TTS models, VRAM fixes
A: Persona Seed Integration	Done	17_persona_seed.sql in initdb pipeline
D: GPU Models YAML Sync	Done	10 models added to gpu-models.yaml
C: Service-Model Mapping Expansion	Done	25+ new service-model mappings
F: Persona-Model Resolution View	Done	persona_model_resolution + active_persona_summary views
E: Startup Readiness Check	Done	model_readiness_check.py + Make target

Key Changes

Model Registry (12_model_registry_seed.sql)

• New providers: anthropic_primary, tts_local (10 total, was 8)
• New models: 3 Anthropic + 5 Ollama + 6 TTS = 14 new models (40 total, was ~28)
• VRAM fixes: qwen3:8b 8000→6144, nomic-embed-text 1000→512 (gpu-models.yaml is truth)
• Service mappings: 41 total (was ~16), covering hirag, archon, coding, orchestrator, vl_sentinel, agent_zero_subordinate, tts, extract_worker, research_coordinator, knowledge_manager

Persona Seeds (17_persona_seed.sql)

• 8 personas from pmoves/db/v5_14_seed_standard_personas.sql now in active initdb pipeline
• Sequenced as 17_* (after model registry at 12_*)
• All use ON CONFLICT (name, version) DO UPDATE SET for idempotency

GPU Models (gpu-models.yaml)

• 10 models added: qwen2.5:32b/14b, qwen2-vl:7b, qwen3-reranker:4b, nemotron-mini, llama3.1, embedding models
• GPU Orchestrator can now track VRAM for all local models

Verification

make -C pmoves model-readiness          # All checks pass
# Supabase: SELECT count(*) FROM pmoves_core.personas → ≥8
# Supabase: SELECT count(*) FROM pmoves_core.models → ≥35
# Supabase: SELECT * FROM pmoves_core.persona_model_resolution → all resolved
# SELECT count(*) FROM pmoves_core.service_model_mappings → ≥15

Test plan

• make -C pmoves model-readiness passes with running stack
• SELECT count(*) FROM pmoves_core.personas returns ≥8 after initdb
• SELECT count(*) FROM pmoves_core.models returns ≥35
• SELECT * FROM pmoves_core.persona_model_resolution shows all personas with resolved providers
• SELECT count(*) FROM pmoves_core.service_model_mappings returns ≥15
• gpu-models.yaml parseable by GPU Orchestrator
• Ollama models match between YAML and SQL seed

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Production-ready model & persona infra: two new runtime views, expanded GPU/local model catalog, and eight seeded production personas with verification queries.
  • Startup readiness checker to validate model/provider/persona health and local model availability.

• Documentation

  • Added production-readiness guide, TAC model/persona overlays, handoff block templates, protocol updates, and new witness voice guidance.

• Chores

  • Integrated model/persona readiness into the verify pipeline and CI targets; tightened studio board access and RLS.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds production-readiness artifacts: large model and persona DB seeds, two new persona→model resolution views, a Python model-readiness checker wired into Make, GPU model config additions, RLS/grant tightening, and documentation/TAC overlays for model/persona readiness.

Changes

Cohort / File(s)	Summary
Documentation & TAC docs/AGENT_TRAIL.md, pmoves/docs/AGENTS/AGNOTE4482.md, pmoves/docs/AGENTS/AGNOTE4482PHI.t1.md, pmoves/docs/AGENTS/AI_GRAPHITI_PROTOCOL.md, pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md	Added TAC/model-persona readiness overlay and execution overlay references, introduced witness voice and a TAC Tree Handoff Block template, updated Graphiti protocol and agent ACK entries.
Makefile & Readiness Tool pmoves/Makefile, pmoves/tools/model_readiness_check.py	Added exported model-readiness Make target and integrated it into verify-all; new CLI ReadinessChecker that queries Supabase (providers/personas/views), Ollama, and TensorZero and emits PASS/WARN/FAIL with an exit code.
GPU Model Config pmoves/config/gpu-models.yaml	Inserted many new Ollama-backed GPU model entries (Qwen2.5/Qwen3 variants, Llama3.1, nemotron-mini, embeddings) with VRAM, context lengths, and quantization metadata.
Model Registry Seed pmoves/supabase/initdb/12_model_registry_seed.sql	Expanded model/provider seed data (Anthropic, TTS, local/cloud variants), adjusted VRAM/context values, extended service model mappings, and bumped seed version/audit fields.
Persona Seed pmoves/supabase/initdb/17_persona_seed.sql	New idempotent persona seed inserting eight production personas with full configurations, indexes, verification queries, and observability scaffolding.
Migrations / Views pmoves/supabase/migrations/20260301002000_persona_model_resolution.sql	Added pmoves_core.persona_model_resolution and pmoves_core.active_persona_summary views (with comments, GRANT SELECT to common roles, SECURITY INVOKER).
RLS / Grants pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql	Hardened studio_board privileges: removed anon/auth grants, granted schema/table/sequence usage to service_role, and replaced open policies with a service_role-scoped policy.
Supabase SQL Bulk pmoves/supabase/initdb/*, pmoves/supabase/migrations/*	Large SQL additions/updates across seeds and migrations (models, embeddings, service mappings, persona indexes); significant data seeding and access-control adjustments.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI: `make model-readiness`
    participant Checker as ReadinessChecker
    participant Supabase as Supabase<br/>(model_providers/personas/views)
    participant Ollama as Ollama API
    participant TensorZero as TensorZero<br/>(gateway)

    CLI->>Checker: instantiate & run()
    activate Checker

    Checker->>Supabase: GET /rest/v1/model_providers (check_supabase_providers)
    Supabase-->>Checker: providers list

    Checker->>Supabase: GET /rest/v1/personas (check_supabase_personas)
    Supabase-->>Checker: personas list

    Checker->>Ollama: GET /api/tags (check_ollama)
    Ollama-->>Checker: local models/tags

    Checker->>TensorZero: GET /health (check_tensorzero)
    TensorZero-->>Checker: health status

    Checker->>Supabase: GET /rest/v1/persona_model_resolution (check_persona_resolution)
    Supabase-->>Checker: resolved rows

    Checker->>Checker: summarize results & set exit code
    deactivate Checker
    Checker-->>CLI: exit 0 / exit 1

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• feat(service-discovery): add NATS-based service discovery infrastructure and personas-first architecture #493: Overlaps model-registry/persona seeds and readiness tooling — touches the same DB seeds, mappings, and readiness checks.

Poem

🐰 I hopped through seeds and schema rows,

I pinged Ollama where the soft light glows,
I tapped TensorZero at the gateway gate,
Eight personas lined up, all set for fate,
A rabbit's wiggle for readiness — celebrate!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description covers summary, key changes, TAC branches, verification steps, and a test plan. However, it lacks a Testing section with actual commands executed and does not include the required checklist items from the template.	Add a Testing section documenting executed commands/scripts with outputs, and complete the required checklist items (CHIT Contract Check, contract/schema updates, documentation updates, review coordination).

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately summarizes the main changes: model registry reconciliation, persona seeds integration, and readiness check tool addition.
Docstring Coverage	✅ Passed	Docstring coverage is 92.86% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/model-registry-persona-readiness

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pmoves/supabase/initdb/12_model_registry_seed.sql (1)

1062-1076: ⚠️ Potential issue | 🟠 Major

Model lookups with LIMIT 1 are nondeterministic for mapping seeds.

Selecting model IDs by model_id plus LIMIT 1 can bind mappings to arbitrary rows when duplicates exist across providers. That can silently route services to the wrong backend.

Suggested fix pattern

 DECLARE
+  v_ollama_local_provider_id UUID;
   v_qwen3_8b_id UUID;
 BEGIN
+  SELECT id INTO v_ollama_local_provider_id
+  FROM pmoves_core.model_providers
+  WHERE name = 'ollama_local';
+
-  SELECT id INTO v_qwen3_8b_id FROM pmoves_core.models WHERE model_id = 'qwen3:8b' LIMIT 1;
+  SELECT id INTO v_qwen3_8b_id
+  FROM pmoves_core.models
+  WHERE provider_id = v_ollama_local_provider_id
+    AND model_id = 'qwen3:8b';
+
+  IF v_qwen3_8b_id IS NULL THEN
+    RAISE EXCEPTION 'Missing model mapping source: ollama_local/qwen3:8b';
+  END IF;

Also applies to: 1227-1250

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/supabase/initdb/12_model_registry_seed.sql` around lines 1062 - 1076,
The SELECTs that populate variables like v_qwen3_8b_id, v_qwen2_5_32b_id,
v_nemotron_id, v_qwen3_emb_4b_id, v_mistral_edge_id, v_phi3_edge_id, v_zai_id,
v_openai_id, v_venice_id, v_groq_id and v_openrouter_id use "LIMIT 1" and are
nondeterministic when duplicate model rows exist; change these lookups to
deterministically identify a single row (e.g., add explicit provider/name
qualifiers or an ORDER BY that prefers the intended provider, or use an exact
name match instead of LIKE), and fail loudly if multiple matches remain (raise
an exception or use a SELECT ... INTO with a check on FOUND_ROWS/COUNT and abort
when count<>1) so seeds cannot silently map to the wrong backend; apply the same
change pattern to the similar queries around lines 1227-1250.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md`:
- Line 60: Update the migration filename reference in the doc
TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md: replace the incorrect short name
`20260301_persona_model_resolution.sql` with the actual migration filename
`20260301002000_persona_model_resolution.sql` (and search the document for any
other occurrences of the short name to correct them) so operators follow the
correct file in the execution plan.
- Around line 17-20: Update the "Current State Snapshot" section to reflect the
actual branch contents: remove or change the bullets that claim the
persona-model resolution view migration and the model readiness script + Make
target wiring are only local working-tree artifacts, and state that both are
included in this PR (and add a note about the deterministic verification
evidence bundle location or attach link/reference to the PR comment trail). Edit
the lines mentioning "persona-model resolution view migration", "model readiness
script + Make target wiring", and "deterministic verification evidence bundle"
so the status accurately matches the PR evidence.

In `@pmoves/Makefile`:
- Around line 1113-1114: The verify-all Makefile currently suppresses failures
for the model-readiness step by invoking "@$(MAKE) --no-print-directory
model-readiness" with a leading '-' prefix; remove the leading '-' so the
invocation is "@$(MAKE) --no-print-directory model-readiness" (no
failure-suppression) so that a failing model-readiness target properly fails
verify-all; locate the invocation under the verify-all/verify pipeline in the
Makefile to update the line invoking model-readiness.

In `@pmoves/supabase/initdb/12_model_registry_seed.sql`:
- Line 178: The provider metadata for tts_local declares 7 engines but only 6
TTS model rows exist; fix by either updating the metadata JSON '{"network":
"internal", "location": "local", "engines": 7}'::jsonb to engines: 6 or adding
the missing seventh model row to the seed so the tts_local provider entry and
the seeded models table are consistent (look for the tts_local provider metadata
and the TTS model INSERTs in this seed).
- Around line 1405-1415: The seed record inserted into
pmoves_core.model_providers (name '_seed_audit') should not be marked active;
change the INSERT so the active column is false (VALUES ... false ...) and
ensure the ON CONFLICT DO UPDATE does not flip it back to active — either stop
updating active in the DO UPDATE clause or explicitly set active = false there
(alongside updating metadata and updated_at) so the pseudo-provider is never
treated as an active, routable provider.

In `@pmoves/supabase/initdb/17_persona_seed.sql`:
- Around line 161-166: The upsert conflict handler currently only updates
description, system_prompt_template, tools_access, and behavior_weights, leaving
runtime config fields stale; update the DO UPDATE SET clause for the persona
upsert blocks (the INSERT ... ON CONFLICT (name, version) DO UPDATE SET section)
to also assign EXCLUDED.thread_type, EXCLUDED.model_preference,
EXCLUDED.temperature, EXCLUDED.max_tokens, EXCLUDED.is_active,
EXCLUDED.is_default, and any routing fields (e.g., EXCLUDED.routing_enabled,
EXCLUDED.routing_preference, EXCLUDED.routing_metadata) so that reseeds fully
reconcile persona runtime configuration, keeping updated_at = NOW().

In `@pmoves/supabase/migrations/20260301002000_persona_model_resolution.sql`:
- Around line 13-39: The view pmoves_core.persona_model_resolution is created
without explicit per-invoker security, so alter its creation to include WITH
(security_invoker = true) so the view runs with the caller's RLS context; update
the CREATE OR REPLACE VIEW statement for persona_model_resolution to add the
security_invoker property and do the same change for the other view that grants
postgrest_anon read access to ensure both views execute under the caller's RLS
rather than the view owner's.

In `@pmoves/tools/model_readiness_check.py`:
- Around line 130-131: The readiness checks currently call self._warn (e.g.,
"Ollama reachable" with ollama_url) which allows startup to return success;
change these to fail the readiness gate by replacing those self._warn calls with
a failing action — use self._error(...) if that helper exists (preserving the
same message and context) or raise SystemExit(1) / RuntimeError to abort startup
when Ollama (ollama_url), TensorZero, or persona_model_resolution checks fail;
apply the same change for the other occurrences referenced (around the checks
for TensorZero and persona_model_resolution).
- Around line 133-144: The current check uses substring matching (any(model in
name for name in pulled)) which allows false positives; instead normalize the
pulled model names to their base form and perform exact equality checks: when
building pulled (the set comprehension that currently does
m.get("name","").split(":")[0]), further normalize each entry by taking the
leading base token (e.g., split on "-" and take [0] or otherwise extract the
canonical base name) into a new set (e.g., pulled_base) and then change the loop
over critical models to use found = model in pulled_base and keep the same
_warn/_check calls.
- Around line 30-37: The http_get function (and the similar HTTP helper around
lines 40–51, e.g., http_post) currently calls urllib.request.urlopen on
user-controlled URLs; add a URL-scheme allowlist to reject any non-http/https
schemes before calling urlopen to prevent file://, ftp:// or custom handlers
from being processed. Use urllib.parse.urlparse(url).scheme to validate the
scheme and return None (or raise a clear error) when scheme not in {"http",
"https"} so urlopen is only invoked for safe schemes; apply the same validation
logic to the corresponding http_post helper to keep behavior consistent.

---

Outside diff comments:
In `@pmoves/supabase/initdb/12_model_registry_seed.sql`:
- Around line 1062-1076: The SELECTs that populate variables like v_qwen3_8b_id,
v_qwen2_5_32b_id, v_nemotron_id, v_qwen3_emb_4b_id, v_mistral_edge_id,
v_phi3_edge_id, v_zai_id, v_openai_id, v_venice_id, v_groq_id and
v_openrouter_id use "LIMIT 1" and are nondeterministic when duplicate model rows
exist; change these lookups to deterministically identify a single row (e.g.,
add explicit provider/name qualifiers or an ORDER BY that prefers the intended
provider, or use an exact name match instead of LIKE), and fail loudly if
multiple matches remain (raise an exception or use a SELECT ... INTO with a
check on FOUND_ROWS/COUNT and abort when count<>1) so seeds cannot silently map
to the wrong backend; apply the same change pattern to the similar queries
around lines 1227-1250.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e69ed0 and 044dc9e.

📒 Files selected for processing (11)

• docs/AGENT_TRAIL.md
• pmoves/Makefile
• pmoves/config/gpu-models.yaml
• pmoves/docs/AGENTS/AGNOTE4482.md
• pmoves/docs/AGENTS/AGNOTE4482PHI.t1.md
• pmoves/docs/AGENTS/AI_GRAPHITI_PROTOCOL.md
• pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md
• pmoves/supabase/initdb/12_model_registry_seed.sql
• pmoves/supabase/initdb/17_persona_seed.sql
• pmoves/supabase/migrations/20260301002000_persona_model_resolution.sql
• pmoves/tools/model_readiness_check.py

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

pmoves/supabase/initdb/12_model_registry_seed.sql (1)

19-33: ⚠️ Potential issue | 🟠 Major

Provider upserts are only partially reconciliatory and can preserve stale active/metadata state.

Current ON CONFLICT clauses update only selected columns, so previous incorrect provider metadata or active flags may survive reseeds.

🔧 Proposed fix pattern (apply to each provider upsert)

 ON CONFLICT (name) DO UPDATE SET
+  type = EXCLUDED.type,
   api_base = EXCLUDED.api_base,
+  api_key_env_var = EXCLUDED.api_key_env_var,
   description = EXCLUDED.description,
+  active = EXCLUDED.active,
+  metadata = EXCLUDED.metadata,
   updated_at = NOW();

Also applies to: 35-49, 51-66, 68-83, 85-100, 102-117, 119-134, 136-151, 153-168, 170-183

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/supabase/initdb/12_model_registry_seed.sql` around lines 19 - 33, The
upsert for pmoves_core.model_providers only updates a subset of columns so stale
active/metadata/type/api_key_env_var values can persist; update the ON CONFLICT
clause for the pmoves_core.model_providers INSERTs (e.g., the 'ollama_local'
upsert and the other provider upserts) to DO UPDATE SET all mutable columns — at
minimum type, api_base, api_key_env_var, description, active, metadata — and set
updated_at = NOW() so each reseed fully reconciles provider state.

pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql (1)

40-55: ⚠️ Potential issue | 🟠 Major

Recreate studio_board_service_role_all during migration so hardening applies on upgraded databases.

The NOT EXISTS guard means existing environments keep the old policy definition, so this migration may not apply the new predicate everywhere.

🔧 Proposed fix

-  IF EXISTS (
-    SELECT 1 FROM information_schema.tables
-    WHERE table_schema = 'public' AND table_name = 'studio_board'
-  ) AND NOT EXISTS (
-    SELECT 1 FROM pg_policies
-    WHERE schemaname = 'public'
-      AND tablename = 'studio_board'
-      AND policyname = 'studio_board_service_role_all'
-  ) THEN
+  IF EXISTS (
+    SELECT 1 FROM information_schema.tables
+    WHERE table_schema = 'public' AND table_name = 'studio_board'
+  ) THEN
+    DROP POLICY IF EXISTS studio_board_service_role_all ON public.studio_board;
     CREATE POLICY studio_board_service_role_all
       ON public.studio_board
       FOR ALL
       TO service_role
       USING (auth.role() = 'service_role')
       WITH CHECK (auth.role() = 'service_role');
   END IF;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql`
around lines 40 - 55, The migration currently skips creating the updated policy
when an older policy named studio_board_service_role_all already exists; change
it to ensure the new hardening applies by removing the NOT EXISTS guard and
instead dropping any existing policy then creating the new one: inside the same
IF EXISTS (...) check for the public.studio_board table, add a statement DROP
POLICY IF EXISTS studio_board_service_role_all ON public.studio_board;
immediately followed by the CREATE POLICY studio_board_service_role_all ... FOR
ALL TO service_role USING (auth.role() = 'service_role') WITH CHECK (auth.role()
= 'service_role'); so the migration always replaces the policy definition on
upgrade.

🧹 Nitpick comments (2)

pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md (1)

92-94: Clarify whether verify-all already includes model-readiness to avoid workflow drift.

If verify-all now runs the readiness gate, listing both as mandatory steps can create conflicting operator guidance.

📝 Proposed wording

 1. `make -C pmoves supabase-bootstrap`
-2. `make -C pmoves model-readiness`
-3. `make -C pmoves verify-all`
+2. `make -C pmoves verify-all`  <!-- includes model-readiness gate -->

As per coding guidelines, "Check docs for operational accuracy: Keep status claims aligned with evidence in runbooks and smokes. Flag command drift across Windows/WSL/Linux instructions."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md` around lines 92 -
94, Clarify whether the Make target verify-all invokes the model-readiness
target and update the three-step sequence accordingly: inspect the Makefile
targets (verify-all, model-readiness, supabase-bootstrap) to determine if
verify-all depends on or runs model-readiness, then edit the doc lines listing
the commands so they accurately reflect reality—either remove model-readiness if
verify-all already runs it or annotate the steps to state that verify-all
includes (or does not include) the model-readiness gate to avoid operator
confusion.

pmoves/tools/model_readiness_check.py (1)

64-93: Add explicit return type hints to __init__, _check, _warn, and main() for consistency.

These methods are missing return type annotations while other methods in the class already have them. Per coding guidelines, Python code should use type hints.

🔧 Proposed fix

 class ReadinessChecker:
-    def __init__(self, supabase_url: str, supabase_key: str,
-                 ollama_url: str, tensorzero_url: str):
+    def __init__(self, supabase_url: str, supabase_key: str,
+                 ollama_url: str, tensorzero_url: str) -> None:
@@
-    def _check(self, name: str, ok: bool, detail: str = ""):
+    def _check(self, name: str, ok: bool, detail: str = "") -> None:
@@
-    def _warn(self, name: str, detail: str = ""):
+    def _warn(self, name: str, detail: str = "") -> None:
@@
-def main():
+def main() -> None:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/tools/model_readiness_check.py` around lines 64 - 93, Add explicit
return type annotations to the listed methods for consistency: annotate
ReadinessChecker.__init__ with -> None, ReadinessChecker._check with -> None,
ReadinessChecker._warn with -> None, and the module-level main() function with
-> int (or -> None if main currently doesn't return an exit code); update their
signatures accordingly and ensure imports/types still resolve. This keeps type
hints consistent with other methods in the class and the project's typing
guidelines.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/supabase/migrations/20260301002000_persona_model_resolution.sql`:
- Around line 48-53: The section comment "RLS Policy (service_role read access)"
is misleading because the GRANT targets postgrest_anon and postgrest_auth_user;
update the comment or the grants so they match: either change the section header
to reflect postgrest read access (e.g., "RLS Policy (postgrest read access)") or
replace the GRANT target(s) with the intended service role(s); locate the
comment "RLS Policy (service_role read access)" and the GRANT SELECT ON
pmoves_core.persona_model_resolution TO postgrest_anon, postgrest_auth_user and
make them consistent.

---

Outside diff comments:
In `@pmoves/supabase/initdb/12_model_registry_seed.sql`:
- Around line 19-33: The upsert for pmoves_core.model_providers only updates a
subset of columns so stale active/metadata/type/api_key_env_var values can
persist; update the ON CONFLICT clause for the pmoves_core.model_providers
INSERTs (e.g., the 'ollama_local' upsert and the other provider upserts) to DO
UPDATE SET all mutable columns — at minimum type, api_base, api_key_env_var,
description, active, metadata — and set updated_at = NOW() so each reseed fully
reconciles provider state.

In `@pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql`:
- Around line 40-55: The migration currently skips creating the updated policy
when an older policy named studio_board_service_role_all already exists; change
it to ensure the new hardening applies by removing the NOT EXISTS guard and
instead dropping any existing policy then creating the new one: inside the same
IF EXISTS (...) check for the public.studio_board table, add a statement DROP
POLICY IF EXISTS studio_board_service_role_all ON public.studio_board;
immediately followed by the CREATE POLICY studio_board_service_role_all ... FOR
ALL TO service_role USING (auth.role() = 'service_role') WITH CHECK (auth.role()
= 'service_role'); so the migration always replaces the policy definition on
upgrade.

---

Nitpick comments:
In `@pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md`:
- Around line 92-94: Clarify whether the Make target verify-all invokes the
model-readiness target and update the three-step sequence accordingly: inspect
the Makefile targets (verify-all, model-readiness, supabase-bootstrap) to
determine if verify-all depends on or runs model-readiness, then edit the doc
lines listing the commands so they accurately reflect reality—either remove
model-readiness if verify-all already runs it or annotate the steps to state
that verify-all includes (or does not include) the model-readiness gate to avoid
operator confusion.

In `@pmoves/tools/model_readiness_check.py`:
- Around line 64-93: Add explicit return type annotations to the listed methods
for consistency: annotate ReadinessChecker.__init__ with -> None,
ReadinessChecker._check with -> None, ReadinessChecker._warn with -> None, and
the module-level main() function with -> int (or -> None if main currently
doesn't return an exit code); update their signatures accordingly and ensure
imports/types still resolve. This keeps type hints consistent with other methods
in the class and the project's typing guidelines.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 044dc9e and 605b05b.

📒 Files selected for processing (7)

• pmoves/Makefile
• pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md
• pmoves/supabase/initdb/12_model_registry_seed.sql
• pmoves/supabase/initdb/17_persona_seed.sql
• pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql
• pmoves/supabase/migrations/20260301002000_persona_model_resolution.sql
• pmoves/tools/model_readiness_check.py

🚧 Files skipped from review as they are similar to previous changes (1)

• pmoves/Makefile

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql`:
- Line 10: Add explicit REVOKE statements to remove direct privileges previously
granted to anon and authenticated so only service_role retains access: revoke
SELECT, INSERT, UPDATE, DELETE ON TABLE studio_board FROM anon, authenticated;
revoke USAGE, SELECT ON SEQUENCE studio_board_id_seq FROM anon, authenticated;
and if schema-level grants exist revoke USAGE ON SCHEMA public FROM anon,
authenticated; keep the GRANT USAGE ON SCHEMA public TO service_role and ensure
these REVOKE statements are applied in the migration alongside the RLS policy
changes (referencing studio_board, studio_board_id_seq, anon, authenticated,
service_role, and the existing GRANT USAGE ON SCHEMA public line).

In `@pmoves/tools/model_readiness_check.py`:
- Around line 170-180: The reachability check is failing because http_get
swallows non-JSON successful responses (catching json.JSONDecodeError and
returning None); update the logic so reachability is judged from HTTP success,
not JSON parsing: either change http_get to return the raw response/text (or a
(status_code, text) tuple) on 2xx responses even if JSON parsing fails, or add a
new helper (e.g., http_get_raw) that returns response.text for any successful
status; then update check_tensorzero (the code calling http_get in the shown
block) to use the raw-response helper (or check status_code) for reachability
and only attempt JSON parsing when needed, leaving the existing
_check("TensorZero health"/"TensorZero reachable") calls intact.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 605b05b and e9404e9.

📒 Files selected for processing (5)

• pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md
• pmoves/supabase/initdb/12_model_registry_seed.sql
• pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql
• pmoves/supabase/migrations/20260301002000_persona_model_resolution.sql
• pmoves/tools/model_readiness_check.py

✅ Files skipped from review due to trivial changes (1)

• pmoves/docs/TAC/TAC_MODEL_INFRA_PERSONA_PROD_READINESS.md

🚧 Files skipped from review as they are similar to previous changes (1)

• pmoves/supabase/migrations/20260301002000_persona_model_resolution.sql

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql (1)

14-27: Consolidate duplicate studio_board existence checks into one block.

The two identical IF EXISTS guards can be merged to reduce duplication and keep privilege/RLS changes together.

♻️ Suggested simplification

-  IF EXISTS (
-    SELECT 1 FROM information_schema.tables
-    WHERE table_schema = 'public' AND table_name = 'studio_board'
-  ) THEN
-    EXECUTE 'REVOKE SELECT, INSERT, UPDATE, DELETE ON TABLE public.studio_board FROM anon, authenticated';
-  END IF;
-
-  IF EXISTS (
-    SELECT 1 FROM information_schema.tables
-    WHERE table_schema = 'public' AND table_name = 'studio_board'
-  ) THEN
+  IF EXISTS (
+    SELECT 1 FROM information_schema.tables
+    WHERE table_schema = 'public' AND table_name = 'studio_board'
+  ) THEN
+    EXECUTE 'REVOKE SELECT, INSERT, UPDATE, DELETE ON TABLE public.studio_board FROM anon, authenticated';
     EXECUTE 'GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE public.studio_board TO service_role';
     EXECUTE 'ALTER TABLE public.studio_board ENABLE ROW LEVEL SECURITY';
   END IF;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql`
around lines 14 - 27, The migration repeats the same IF EXISTS check for the
table studio_board; consolidate them by keeping a single IF EXISTS (checking
information_schema for table_schema = 'public' AND table_name = 'studio_board')
and place the privilege and RLS statements inside it in the correct
order—execute the REVOKE from anon and authenticated, then EXECUTE the GRANT to
service_role, and then EXECUTE the ALTER TABLE public.studio_board ENABLE ROW
LEVEL SECURITY—so all privilege/RLS changes for public.studio_board are
performed under one existence guard.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/tools/model_readiness_check.py`:
- Around line 115-136: The check_supabase_providers flow is missing enforcement
of the minimum registry seeding thresholds for models and
service_model_mappings; add two additional HTTP GET queries using
http_get_supabase to fetch "/models" and "/service_model_mappings" (reuse
self.supabase_url and self.supabase_key), compute counts (len(list) or 0 if
None), and call self._check to assert models count >= 35 with a descriptive
message and service_model_mappings count >= 15 with a descriptive message; use
the same pattern as the existing provider checks and the same helper self._check
so failures report comparable contextual strings (refer to
check_supabase_providers, http_get_supabase, and self._check).
- Around line 206-218: The current block gated by if isinstance(data, list):
silently skips checks when persona_model_resolution returns JSON that is not a
list; add an else branch after that if to explicitly fail the check using
self._check (e.g., self._check("persona_model_resolution payload shape", False,
f"expected list but got {type(data).__name__}")) so the method records a failing
readiness check for unexpected payload shapes; keep using the same method
context (the code that calls self._check and self._warn inside the existing
block) and return or exit that branch after logging the failure.

---

Nitpick comments:
In `@pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql`:
- Around line 14-27: The migration repeats the same IF EXISTS check for the
table studio_board; consolidate them by keeping a single IF EXISTS (checking
information_schema for table_schema = 'public' AND table_name = 'studio_board')
and place the privilege and RLS statements inside it in the correct
order—execute the REVOKE from anon and authenticated, then EXECUTE the GRANT to
service_role, and then EXECUTE the ALTER TABLE public.studio_board ENABLE ROW
LEVEL SECURITY—so all privilege/RLS changes for public.studio_board are
performed under one existence guard.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9404e9 and a597643.

📒 Files selected for processing (2)

• pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql
• pmoves/tools/model_readiness_check.py

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql (1)

14-27: Consider consolidating duplicate IF EXISTS checks.

Lines 14-19 and 21-27 both check for the same condition (table_name = 'studio_board'). These could be merged into a single IF block for clarity.

♻️ Suggested consolidation

   IF EXISTS (
     SELECT 1 FROM information_schema.tables
     WHERE table_schema = 'public' AND table_name = 'studio_board'
   ) THEN
     EXECUTE 'REVOKE ALL PRIVILEGES ON TABLE public.studio_board FROM anon, authenticated';
-  END IF;
-
-  IF EXISTS (
-    SELECT 1 FROM information_schema.tables
-    WHERE table_schema = 'public' AND table_name = 'studio_board'
-  ) THEN
     EXECUTE 'GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE public.studio_board TO service_role';
     EXECUTE 'ALTER TABLE public.studio_board ENABLE ROW LEVEL SECURITY';
   END IF;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql`
around lines 14 - 27, Combine the two duplicate IF EXISTS checks that both test
for table_name = 'studio_board' into a single IF block: check the
information_schema.tables once and inside that block execute the REVOKE ALL
PRIVILEGES ... FROM anon, authenticated, then execute the GRANT SELECT, INSERT,
UPDATE, DELETE ... TO service_role and the ALTER TABLE public.studio_board
ENABLE ROW LEVEL SECURITY statements; update the block surrounding these EXECUTE
calls (referencing the IF EXISTS, the EXECUTE 'REVOKE ALL PRIVILEGES ON TABLE
public.studio_board FROM anon, authenticated', EXECUTE 'GRANT ... TO
service_role', and EXECUTE 'ALTER TABLE public.studio_board ENABLE ROW LEVEL
SECURITY') so all three operations run under the single conditional.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/tools/model_readiness_check.py`:
- Around line 188-190: The code assumes the HTTP response `data` is a dict and
directly calls data.get("models", []), which can raise AttributeError if
`http_get()` returns non-dict JSON; update the logic in the function where
`models` and `pulled_base` are computed to first verify `data` is a mapping
(e.g., isinstance(data, dict)); if not, set `models = []`, log or capture the
invalid payload, and call self._check("Ollama responding", False, "invalid/
unexpected payload") so the readiness check fails gracefully instead of raising;
ensure the subsequent usage of `pulled_base` and the existing self._check call
handle the empty list case.

---

Nitpick comments:
In `@pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql`:
- Around line 14-27: Combine the two duplicate IF EXISTS checks that both test
for table_name = 'studio_board' into a single IF block: check the
information_schema.tables once and inside that block execute the REVOKE ALL
PRIVILEGES ... FROM anon, authenticated, then execute the GRANT SELECT, INSERT,
UPDATE, DELETE ... TO service_role and the ALTER TABLE public.studio_board
ENABLE ROW LEVEL SECURITY statements; update the block surrounding these EXECUTE
calls (referencing the IF EXISTS, the EXECUTE 'REVOKE ALL PRIVILEGES ON TABLE
public.studio_board FROM anon, authenticated', EXECUTE 'GRANT ... TO
service_role', and EXECUTE 'ALTER TABLE public.studio_board ENABLE ROW LEVEL
SECURITY') so all three operations run under the single conditional.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a597643 and 0c28b0e.

📒 Files selected for processing (2)

• pmoves/supabase/migrations/20260301001000_studio_board_rls_service_role.sql
• pmoves/tools/model_readiness_check.py

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Guard Ollama payload shape before dereferencing.

At Line 188, data.get("models", []) assumes http_get() returned a dict. If /api/tags returns valid non-object JSON, this can crash readiness with AttributeError instead of recording a failed check.

Proposed hardening

     def check_ollama(self) -> None:
         """Check Ollama has expected local models pulled."""
         print("\n[3] Ollama local models")
         data = http_get(f"{self.ollama_url}/api/tags")
         if data is None:
             self._check("Ollama reachable", False, f"cannot reach {self.ollama_url}")
             return
+        if not isinstance(data, dict):
+            self._check("Ollama payload format", False,
+                        f"expected JSON object, got {type(data).__name__}")
+            return
 
-        models = data.get("models", [])
-        pulled_base = {m.get("name", "").split(":")[0].strip().lower() for m in models if m.get("name")}
+        models = data.get("models")
+        if not isinstance(models, list):
+            self._check("Ollama payload format", False, "missing models array")
+            return
+        pulled_base = {
+            m.get("name", "").split(":")[0].strip().lower()
+            for m in models
+            if isinstance(m, dict) and m.get("name")
+        }
         self._check("Ollama responding", True, f"{len(models)} models loaded")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pmoves/tools/model_readiness_check.py` around lines 188 - 190, The code
assumes the HTTP response `data` is a dict and directly calls data.get("models",
[]), which can raise AttributeError if `http_get()` returns non-dict JSON;
update the logic in the function where `models` and `pulled_base` are computed
to first verify `data` is a mapping (e.g., isinstance(data, dict)); if not, set
`models = []`, log or capture the invalid payload, and call self._check("Ollama
responding", False, "invalid/ unexpected payload") so the readiness check fails
gracefully instead of raising; ensure the subsequent usage of `pulled_base` and
the existing self._check call handle the empty list case.