Repository navigation
feat: shared organization ingestion datasets and import history - #133
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
|
Updates to Preview Branch (codex/org-ingestion) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20261110010000_ingestion_catalog.sql">
<violation number="1" location="supabase/migrations/20261110010000_ingestion_catalog.sql:2">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
This 196-line migration exceeds the rule’s under-100-line limit. Split it into focused, ordered migrations, keeping each migration transactional.</violation>
<violation number="2" location="supabase/migrations/20261110010000_ingestion_catalog.sql:62">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
`ingestion_catalog_command` combines registration, completion, and three distinct paginated reads with their validation and persistence logic. Extract action-specific handlers into private helpers while keeping this as the only granted gateway.</violation>
<violation number="3" location="supabase/migrations/20261110010000_ingestion_catalog.sql:137">
P2: Hash the complete validated completion input, or reject unknown top-level keys; the current fingerprint ignores extra `p_input` fields, so retries do not enforce the documented whole-payload consistency.</violation>
<violation number="4" location="supabase/migrations/20261110010000_ingestion_catalog.sql:185">
P3: This raise has no SQLSTATE, so callers see the default P0001. Give it an explicit code so the gateway can map it consistently with other controlled errors, for example 55000 for an object-not-in-required-state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| RAISE EXCEPTION 'Invalid ingestion gold' USING ERRCODE='22023'; | ||
| END IF; | ||
| END IF; | ||
| v_fingerprint:=pg_catalog.encode(pg_catalog.sha256(pg_catalog.convert_to(jsonb_build_object('result',v_result,'gold',v_gold)::text,'UTF8')),'hex'); |
There was a problem hiding this comment.
P2: Hash the complete validated completion input, or reject unknown top-level keys; the current fingerprint ignores extra p_input fields, so retries do not enforce the documented whole-payload consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At supabase/migrations/20261110010000_ingestion_catalog.sql, line 137:
<comment>Hash the complete validated completion input, or reject unknown top-level keys; the current fingerprint ignores extra `p_input` fields, so retries do not enforce the documented whole-payload consistency.</comment>
<file context>
@@ -0,0 +1,196 @@
+ RAISE EXCEPTION 'Invalid ingestion gold' USING ERRCODE='22023';
+ END IF;
+ END IF;
+ v_fingerprint:=pg_catalog.encode(pg_catalog.sha256(pg_catalog.convert_to(jsonb_build_object('result',v_result,'gold',v_gold)::text,'UTF8')),'hex');
+ IF v_import.status<>'queued' THEN
+ IF v_import.completion_sha256<>v_fingerprint THEN RAISE EXCEPTION 'Ingestion catalog conflict'; END IF;
</file context>
Suggested change
| v_fingerprint:=pg_catalog.encode(pg_catalog.sha256(pg_catalog.convert_to(jsonb_build_object('result',v_result,'gold',v_gold)::text,'UTF8')),'hex'); | |
| v_fingerprint:=pg_catalog.encode(pg_catalog.sha256(pg_catalog.convert_to(p_input::text,'UTF8')),'hex'); |
| @@ -0,0 +1,196 @@ | |||
| -- Shared organization datasets. Originals and processing artifacts remain in private Storage. | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
This 196-line migration exceeds the rule’s under-100-line limit. Split it into focused, ordered migrations, keeping each migration transactional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At supabase/migrations/20261110010000_ingestion_catalog.sql, line 2:
<comment>This 196-line migration exceeds the rule’s under-100-line limit. Split it into focused, ordered migrations, keeping each migration transactional.</comment>
<file context>
@@ -0,0 +1,196 @@
+-- Shared organization datasets. Originals and processing artifacts remain in private Storage.
+BEGIN;
+CREATE TABLE public.ingestion_datasets (
+ id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
</file context>
| END $$; | ||
| REVOKE ALL ON FUNCTION public.authorize_ingestion_organization(uuid,uuid) FROM PUBLIC,anon,authenticated,service_role; | ||
|
|
||
| CREATE FUNCTION public.ingestion_catalog_command(p_actor uuid,p_org uuid,p_action text,p_input jsonb DEFAULT '{}') |
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
ingestion_catalog_command combines registration, completion, and three distinct paginated reads with their validation and persistence logic. Extract action-specific handlers into private helpers while keeping this as the only granted gateway.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At supabase/migrations/20261110010000_ingestion_catalog.sql, line 62:
<comment>`ingestion_catalog_command` combines registration, completion, and three distinct paginated reads with their validation and persistence logic. Extract action-specific handlers into private helpers while keeping this as the only granted gateway.</comment>
<file context>
@@ -0,0 +1,196 @@
+END $$;
+REVOKE ALL ON FUNCTION public.authorize_ingestion_organization(uuid,uuid) FROM PUBLIC,anon,authenticated,service_role;
+
+CREATE FUNCTION public.ingestion_catalog_command(p_actor uuid,p_org uuid,p_action text,p_input jsonb DEFAULT '{}')
+RETURNS jsonb LANGUAGE plpgsql SECURITY DEFINER SET search_path='' AS $$
+DECLARE
</file context>
| IF v_after NOT BETWEEN 0 AND 10000 THEN RAISE EXCEPTION 'Invalid ingestion page' USING ERRCODE='22023'; END IF; | ||
| SELECT * INTO v_import FROM public.ingestion_imports WHERE organization_id=p_org AND dataset_id=v_dataset AND job_id=v_job; | ||
| IF NOT FOUND THEN RAISE EXCEPTION 'Ingestion access denied' USING ERRCODE='42501'; END IF; | ||
| IF v_import.status<>'ready' THEN RAISE EXCEPTION 'Artifact not ready'; END IF; |
There was a problem hiding this comment.
P3: This raise has no SQLSTATE, so callers see the default P0001. Give it an explicit code so the gateway can map it consistently with other controlled errors, for example 55000 for an object-not-in-required-state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At supabase/migrations/20261110010000_ingestion_catalog.sql, line 185:
<comment>This raise has no SQLSTATE, so callers see the default P0001. Give it an explicit code so the gateway can map it consistently with other controlled errors, for example 55000 for an object-not-in-required-state.</comment>
<file context>
@@ -0,0 +1,196 @@
+ IF v_after NOT BETWEEN 0 AND 10000 THEN RAISE EXCEPTION 'Invalid ingestion page' USING ERRCODE='22023'; END IF;
+ SELECT * INTO v_import FROM public.ingestion_imports WHERE organization_id=p_org AND dataset_id=v_dataset AND job_id=v_job;
+ IF NOT FOUND THEN RAISE EXCEPTION 'Ingestion access denied' USING ERRCODE='42501'; END IF;
+ IF v_import.status<>'ready' THEN RAISE EXCEPTION 'Artifact not ready'; END IF;
+ SELECT coalesce(jsonb_agg(jsonb_build_object('row_number',row_number,'record',record) ORDER BY row_number),'[]'::jsonb) INTO v_items FROM (
+ SELECT row_number,record FROM public.ingestion_records WHERE organization_id=p_org AND job_id=v_job AND row_number>v_after ORDER BY row_number LIMIT v_limit+1
</file context>
There was a problem hiding this comment.
1 existing issue remains and 2 new issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20261110010100_ingestion_catalog_page_budget.sql">
<violation number="1" location="supabase/migrations/20261110010100_ingestion_catalog_page_budget.sql:83">
P2: Gold rows are committed without a per-row size limit, but the records page cannot return any row above the 2 MiB budget. A single oversized row therefore blocks every page that reaches it, so later rows in that import become unreadable. Reject any gold record whose `octet_length(record::text)+64` exceeds 2097152 in `complete`, before the INSERT, so the write path matches the read limit.</violation>
<violation number="2" location="supabase/migrations/20261110010100_ingestion_catalog_page_budget.sql:128">
P2: `page_bytes` bounds only the records, but the returned object also repeats the full `field_types` metadata and adds other fields. Long accepted field names can therefore pass this filter while returning a response larger than 2 MiB; measure the complete candidate response, or reserve all response metadata, before selecting the page prefix.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
| IF NOT FOUND THEN RAISE EXCEPTION 'Ingestion access denied' USING ERRCODE='42501'; END IF; | ||
| IF v_import.status<>'ready' THEN RAISE EXCEPTION 'Artifact not ready'; END IF; | ||
| SELECT coalesce(jsonb_agg(jsonb_build_object('row_number',row_number,'record',record) ORDER BY row_number),'[]'::jsonb) INTO v_items FROM ( | ||
| SELECT row_number,record,sum(octet_length(record::text)+64) OVER (ORDER BY row_number) page_bytes FROM ( |
There was a problem hiding this comment.
P2: page_bytes bounds only the records, but the returned object also repeats the full field_types metadata and adds other fields. Long accepted field names can therefore pass this filter while returning a response larger than 2 MiB; measure the complete candidate response, or reserve all response metadata, before selecting the page prefix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At supabase/migrations/20261110010100_ingestion_catalog_page_budget.sql, line 128:
<comment>`page_bytes` bounds only the records, but the returned object also repeats the full `field_types` metadata and adds other fields. Long accepted field names can therefore pass this filter while returning a response larger than 2 MiB; measure the complete candidate response, or reserve all response metadata, before selecting the page prefix.</comment>
<file context>
@@ -0,0 +1,141 @@
+ IF NOT FOUND THEN RAISE EXCEPTION 'Ingestion access denied' USING ERRCODE='42501'; END IF;
+ IF v_import.status<>'ready' THEN RAISE EXCEPTION 'Artifact not ready'; END IF;
+ SELECT coalesce(jsonb_agg(jsonb_build_object('row_number',row_number,'record',record) ORDER BY row_number),'[]'::jsonb) INTO v_items FROM (
+ SELECT row_number,record,sum(octet_length(record::text)+64) OVER (ORDER BY row_number) page_bytes FROM (
+ SELECT row_number,record FROM public.ingestion_records WHERE organization_id=p_org AND job_id=v_job AND row_number>v_after ORDER BY row_number LIMIT v_limit
+ ) bounded
</file context>
| IF v_import.completion_sha256<>v_fingerprint THEN RAISE EXCEPTION 'Ingestion catalog conflict'; END IF; | ||
| RETURN jsonb_build_object('dataset_id',v_import.dataset_id,'job_id',v_job,'status',v_import.status,'rows',v_count); | ||
| END IF; | ||
| INSERT INTO public.ingestion_records(organization_id,job_id,dataset_id,row_number,record) |
There was a problem hiding this comment.
P2: Gold rows are committed without a per-row size limit, but the records page cannot return any row above the 2 MiB budget. A single oversized row therefore blocks every page that reaches it, so later rows in that import become unreadable. Reject any gold record whose octet_length(record::text)+64 exceeds 2097152 in complete, before the INSERT, so the write path matches the read limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At supabase/migrations/20261110010100_ingestion_catalog_page_budget.sql, line 83:
<comment>Gold rows are committed without a per-row size limit, but the records page cannot return any row above the 2 MiB budget. A single oversized row therefore blocks every page that reaches it, so later rows in that import become unreadable. Reject any gold record whose `octet_length(record::text)+64` exceeds 2097152 in `complete`, before the INSERT, so the write path matches the read limit.</comment>
<file context>
@@ -0,0 +1,141 @@
+ IF v_import.completion_sha256<>v_fingerprint THEN RAISE EXCEPTION 'Ingestion catalog conflict'; END IF;
+ RETURN jsonb_build_object('dataset_id',v_import.dataset_id,'job_id',v_job,'status',v_import.status,'rows',v_count);
+ END IF;
+ INSERT INTO public.ingestion_records(organization_id,job_id,dataset_id,row_number,record)
+ SELECT p_org,v_job,v_import.dataset_id,n::integer,r FROM jsonb_array_elements(v_gold->'records') WITH ORDINALITY AS rows(r,n);
+ UPDATE public.ingestion_imports SET status=v_status,result=v_result,field_types=v_gold->'field_types',completion_sha256=v_fingerprint,completed_at=clock_timestamp()
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Organization data imports need a queryable home in Recoup's existing database. This adds shared organization-owned datasets, immutable import versions and validated gold rows, with no database or schema per client.
The service-only database gateway rechecks and locks organization membership on every transaction. Browser and service roles have no direct table access; all three tables have RLS enabled. Gold rows and completion receipts commit atomically, retries must match the original payload, and list/record reads require explicit organization and import scope. Originals remain in private Storage.
Validation: twelve database tests pass on disposable PostgreSQL 17, plus a joined test running the matching API parser and processing entry point against the real migration. Coverage includes exact decimal/identifier strings, concurrent retries, immutable versions, denied cross-org access, membership revocation, private table/function grants, pagination and rollback. Record pages also have a 2 MiB payload budget, with controlled errors for oversized individual rows. The joined test recovers a committed completion with a lost acknowledgement without duplicate rows. CI exercises PostgreSQL 15 and 17.
Rollout: apply both additive migrations to the existing Recoup project before the dependent API consumer. Keep ingestion disabled pending hosted qualification. No hosted migration, new client database or production intake is activated. See
tests/INGESTION-CATALOG.mdfor the dependency boundary and rollback order.API consumer: API PR #1240. Read-only inspection of the Supabase PR preview confirms RLS on all three tables, no direct browser/service table reads, and service-only execution of the catalog gateway. The worker verifies the private artifact byte digest; SQL separately fingerprints the received result and gold objects for idempotency. The granted SQL gateway keeps organization authorization and write/receipt atomicity explicit; already-applied migrations are preserved with forward-only corrections.
Inline-read boundary: the 2 MiB budget covers record payloads, with bounded field-type metadata added to the response. Oversized individual gold rows remain retained and return a controlled size error on inline read; large-object export is a documented future extension. Remaining reviewer suggestions to reject those rows at storage time or further split the gateway are left for human disposition, rather than changing the documented retention/read-capacity contract in this PR.
Final status on
d0262c7: every CI check passes, including the twelve catalog SQL tests on PostgreSQL 15/17, existing database regression jobs and Supabase PR preview. The read-only preview audit also confirms the forward page-budget correction is installed and catalog execution is denied to anonymous/authenticated roles. Production ingestion remains disabled; no production migration was manually applied. Merge/release order is Database #133, then API #1240, followed by hosted API/Storage/Workflow qualification before activation.