Repository navigation
feat: ingest and query organization datasets in Recoup - #1240
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Confidence score: 4/5
processIngestionOperation.tsis already 175 lines, and adding catalog reads makes the dispatcher harder to maintain. Extract the catalog logic into a focused helper.
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="lib/ingestion/processIngestionOperation.ts">
<violation number="1" location="lib/ingestion/processIngestionOperation.ts:22">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
`processIngestionOperation.ts` is now 175 lines, and this dispatch adds catalog reads to the already over-limit operation processor. Extract catalog-query handling or other operation flows into focused modules to bring the file under the 100-line cap.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| if ((await deps.authorize(actor, args.organization_id)) !== scope) | ||
| throw new Error("Scope changed"); | ||
| }; | ||
| if (args.action === "datasets" || args.action === "imports" || args.action === "records") { |
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
processIngestionOperation.ts is now 175 lines, and this dispatch adds catalog reads to the already over-limit operation processor. Extract catalog-query handling or other operation flows into focused modules to bring the file under the 100-line cap.
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 lib/ingestion/processIngestionOperation.ts, line 22:
<comment>`processIngestionOperation.ts` is now 175 lines, and this dispatch adds catalog reads to the already over-limit operation processor. Extract catalog-query handling or other operation flows into focused modules to bring the file under the 100-line cap.</comment>
<file context>
@@ -19,6 +19,12 @@ export async function processIngestionOperation(
if ((await deps.authorize(actor, args.organization_id)) !== scope)
throw new Error("Scope changed");
};
+ if (args.action === "datasets" || args.action === "imports" || args.action === "records") {
+ const { action, organization_id: _organization, ...query } = args;
+ const result = await deps.catalog(actor, scope.split("/")[1], action, query);
</file context>
There was a problem hiding this comment.
1 existing issue remains and no new issues found across 15 files (changes from recent commits).
Confidence score: 4/5
lib/ingestion/processIngestionOperation.tsis 180 lines andprocessIngestionOperationstill handles several distinct flows, making it harder to maintain. Split the flows into focused helpers or modules and keep the file below the 100-line limit.
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
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 6 unresolved issues from previous reviews.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
View guided diff | Turn on auto-fix | Re-trigger cubic
Organization imports can now land in Recoup's existing database: original and processing files remain private, while validated gold becomes queryable as organization-owned dataset/import records. External client database delivery stays optional.
dataset_id; passing that ID on later creates adds an immutable import version.datasets,importsand paginatedrecordsreads with organization scope, explicit import versions and per-operation limits. GET remains read-only.Depends on Database PR #133. Apply that migration to the existing Recoup project before deploying/enabling this consumer.
INGESTION_ENABLEDremains off pending hosted qualification.Validation on
98fba08d: every CI check passes, including the full API regression suite, production/contract/workflow compiler checks, lint, formatting and OAuth compatibility. The ingestion job passes 107 tests on each of PostgreSQL 15 and 17; the optional joined test is skipped in that ordinary CI job and passes separately against the real migrations in the local database harness. The database harness passes twelve SQL cases plus that joined API lifecycle on PostgreSQL 17, covering exact values, acknowledgement-loss recovery, versioned imports, cross-org denial, revocation and bounded pagination. Hosted PR-preview metadata confirms the table grants, guarded RPC and page-budget correction. Vercel preview builds successfully; preview and production HTTP checks return the expected disabled 404 with private/no-store headers.Review: missing/corrupt saved gold, invalid UTF-8, malformed completion metadata, exact redaction, denied unit membership and explicit membership fixture columns are covered. Addressed review threads have been resolved. The remaining operation-processor refactoring suggestion is retained for human review. Inline record payloads have a 2 MiB budget; oversized records remain stored and require a future large-object export path.
No production migration, intake activation or new client database was performed. See
lib/ingestion/CATALOG.mdfor operations, shared ownership, limits, validation boundaries and rollout order.