Prototype dataset builder harness#8
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a dataset-builder agent harness MVP that converts user requests into executable dataset build plans. The implementation spans type definitions, deterministic planning with optional LLM refinement via OpenRouter, agent harness stage orchestration for TinyFish, a new HTTP API endpoint, database schema for persistence, and supporting infrastructure. The deterministic planner infers dataset name, schema columns, clarifying questions, and source strategy from request keywords and provided context; optional OpenRouter refinement enhances the draft plan when an API key is available. Agent harness generation produces ordered task stages and generates TinyFish agent goals/output schemas. A CLI prototype and HTTP route enable local testing and API-driven planning. Sequence Diagram(s)sequenceDiagram
participant User
participant API as POST /api/dataset-builder/plan
participant Planner as createDatasetBuildPlan
participant Deterministic as Deterministic Planner
participant OpenRouter as OpenRouter Client
participant Harness as Agent Harness
participant DB as Database
User->>API: Send dataset build request
API->>API: Authenticate user session
API->>API: Validate & parse request body
API->>Planner: Create plan (with optional OpenRouter client)
Planner->>Deterministic: Generate draft plan
Deterministic->>Deterministic: Infer schema, columns, questions, strategy
Deterministic-->>Planner: Return draft plan
alt planningMode = "openrouter" AND client provided
Planner->>OpenRouter: Refine draft plan via LLM
OpenRouter-->>Planner: Return refined plan or throw
else Refinement fails or unavailable
Planner-->>Planner: Append warning, use draft plan
end
Planner-->>API: Resolved plan
API->>Harness: Generate TinyFish goal & output schema
Harness-->>API: Goal string & JSON schema
API->>DB: (Future) Persist plan
API-->>User: Return planId, plan, goal, schema
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Ticket flow is complete locally and CAR marked TICKET-001 done. Build/security checks passed. PR is blocked only on repo rule requiring one write-access approval; requested @pranavjana for review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
backend/src/schema.ts (1)
86-87: ⚡ Quick winAdd DB defaults for audit timestamps
Line 86/87 and Line 111/112 are non-null audit fields without defaults, which makes inserts brittle for upcoming persistence code paths.
Suggested change
- createdAt: timestamp("created_at").notNull(), - updatedAt: timestamp("updated_at").notNull(), + createdAt: timestamp("created_at").notNull().defaultNow(), + updatedAt: timestamp("updated_at").notNull().defaultNow(), @@ - createdAt: timestamp("created_at").notNull(), - updatedAt: timestamp("updated_at").notNull(), + createdAt: timestamp("created_at").notNull().defaultNow(), + updatedAt: timestamp("updated_at").notNull().defaultNow(),Also applies to: 111-112
🤖 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 `@backend/src/schema.ts` around lines 86 - 87, The createdAt and updatedAt timestamp columns (the timestamp("created_at") and timestamp("updated_at") fields) are non-null but lack DB defaults; update their column definitions to supply a database default of the current time (e.g., DEFAULT CURRENT_TIMESTAMP / use the ORM's defaultNow helper) for both createdAt and updatedAt, and for the later repeated audit fields (the other createdAt/updatedAt declarations) do the same; for updatedAt also add an automatic update behavior (ON UPDATE CURRENT_TIMESTAMP or the ORM equivalent) or ensure the application updates it on write.
🤖 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 `@backend/src/dataset-builder/agent-harness.ts`:
- Around line 96-108: The JSON schema for rows only requires
plan.schema.identityColumnName and ignores other columns marked as isRequired;
update the construction of the rows schema in agent-harness.ts so the required
array includes plan.schema.identityColumnName plus every column name from
plan.schema.columns (or equivalent) where isRequired === true (deduplicate if
necessary) before assigning it to the rows.required field; ensure you keep
additionalProperties: false and the existing rowProperties mapping intact so
validation enforces those required columns (e.g., update the code that sets
required: [plan.schema.identityColumnName] to compute and use a merged
requiredFields array).
In `@backend/src/dataset-builder/run-prototype.ts`:
- Around line 27-31: The code silently yields an undefined openRouterClient when
shouldUseOpenRouter is true but process.env.OPENROUTER_API_KEY is missing;
update the logic around shouldUseOpenRouter and createOpenRouterPlannerClient to
explicitly validate the API key: if shouldUseOpenRouter is true and
process.env.OPENROUTER_API_KEY is falsy, throw or exit with a clear error
message (e.g., "OPENROUTER_API_KEY required when --use-openrouter is set"),
otherwise pass the API key into createOpenRouterPlannerClient and assign to
openRouterClient; reference shouldUseOpenRouter, process.env.OPENROUTER_API_KEY,
createOpenRouterPlannerClient and openRouterClient when making the change.
In `@backend/src/dataset-builder/tinyfish-cli.ts`:
- Around line 53-60: The runTinyFishJson function should guard against hung
external calls and provide stderr when JSON parsing fails: when calling
execFileAsync in runTinyFishJson, include a 30_000 ms timeout option (e.g., add
timeout: 30_000 alongside env and maxBuffer) and destructure stderr from the
result; then wrap JSON.parse(stdout) in a try-catch and, on failure, throw a new
Error that includes the parse error message plus the captured stderr (and stdout
context if helpful) so callers get diagnostic details; keep the function
signature and return type (JSON parsed as unknown).
In `@backend/src/dataset-builder/types.ts`:
- Around line 71-77: The DatasetRunCell.value type currently excludes
objects/arrays causing a mismatch with DatasetColumnKind "json"; update the
DatasetRunCell interface (specifically the value property) to accept JSON values
(e.g., union with unknown | Record<string, unknown> | unknown[] or a JSON type
alias) so object and array cells are allowed, and ensure downstream places that
consume DatasetRunCell.value are adjusted to handle the broader JSON type (look
for usages of DatasetRunCell.value to update any strict string/number/boolean
assumptions).
In `@backend/src/routes/dataset-builder.ts`:
- Around line 92-100: The code currently defaults planningMode to "openrouter",
causing userRequest and providedInputs to be sent to a third party by default;
change the return so planningMode is not defaulted to "openrouter" (use
planningMode as-is or undefined) and only enable the openrouter behavior when
planningMode === "openrouter" is explicitly provided by the caller; update the
return object in the function that builds the response (the block returning {
ok: true, value: { userRequest, updateCadence, planningMode: ... ,
providedInputs: parseStringRecord(...), preferredColumns: ... } }) to stop
applying the nullish coalescing default for planningMode and ensure any
downstream code only sends userRequest/providedInputs when planningMode is
explicitly "openrouter".
- Around line 76-90: The current guards use truthy checks on body.updateCadence
and body.planningMode which let empty strings slip past validation; update the
checks around parseUpdateCadence and parsePlanningMode to explicitly detect
presence (e.g., check body.updateCadence !== undefined && body.updateCadence !==
null and if it's a string ensure length > 0) before calling the parser and
returning the error when the parser returns falsy; reference the symbols
updateCadence, parseUpdateCadence, planningMode, parsePlanningMode,
body.updateCadence, and body.planningMode when making the change.
In `@backend/src/schema.ts`:
- Around line 77-83: The status and updateCadence columns currently use
TypeScript-only unions (.$type<...>()) so invalid strings can be persisted;
replace those with pgEnum-backed types by defining pgEnum enums for
DatasetUpdateCadence and the run/dataset status union, then change the column
definitions (updateCadence and status) to use pgEnum(...) instead of text(...).
Ensure you create the enum declarations (e.g., DatasetUpdateCadence enum and
DatasetStatus/RunStatus enum) and reference them in the schema column
definitions so Postgres enforces allowed values at the DB layer.
In `@notion-docs.md`:
- Around line 1-131: This document (# BigSet Technical Specs & Goals ⛓ /
headings Timeline, Specifics, Flow A/B/C) contains internal business strategy,
team member PII, partnership details, private Notion and Discord links, and
other non-public info — remove all sensitive/internal content from
notion-docs.md and replace with a sanitized public README or a short
public-facing project summary; move the removed content (team names/locations,
marketing goals, co-marketing/convex notes, private Notion/Discord links,
internal references like "UXLabs"/"Alguna") into a private Notion/wiki or
private repo and update the public file to reference that private doc only (or
omit links), and ensure the commit does not expose secrets or internal docs
(consider rewriting the commit history if this was already pushed).
---
Nitpick comments:
In `@backend/src/schema.ts`:
- Around line 86-87: The createdAt and updatedAt timestamp columns (the
timestamp("created_at") and timestamp("updated_at") fields) are non-null but
lack DB defaults; update their column definitions to supply a database default
of the current time (e.g., DEFAULT CURRENT_TIMESTAMP / use the ORM's defaultNow
helper) for both createdAt and updatedAt, and for the later repeated audit
fields (the other createdAt/updatedAt declarations) do the same; for updatedAt
also add an automatic update behavior (ON UPDATE CURRENT_TIMESTAMP or the ORM
equivalent) or ensure the application updates it on write.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0df79e24-cb25-4442-8404-d9e1d9dac28e
⛔ Files ignored due to path filters (2)
image-1.pngis excluded by!**/*.pngimage.pngis excluded by!**/*.png
📒 Files selected for processing (16)
.gitignorebackend/.env.examplebackend/drizzle.config.tsbackend/package.jsonbackend/src/dataset-builder/agent-harness.tsbackend/src/dataset-builder/openrouter.tsbackend/src/dataset-builder/planner.tsbackend/src/dataset-builder/run-prototype.tsbackend/src/dataset-builder/tinyfish-cli.tsbackend/src/dataset-builder/types.tsbackend/src/env.tsbackend/src/index.tsbackend/src/routes/dataset-builder.tsbackend/src/schema.tsdocs/dataset-builder-agent-harness.mdnotion-docs.md
| return { | ||
| type: "object", | ||
| additionalProperties: false, | ||
| required: ["rows", "sourceUrls", "validationIssues"], | ||
| properties: { | ||
| rows: { | ||
| type: "array", | ||
| items: { | ||
| type: "object", | ||
| additionalProperties: false, | ||
| required: [plan.schema.identityColumnName], | ||
| properties: rowProperties, | ||
| }, |
There was a problem hiding this comment.
Output schema ignores isRequired columns.
Rows currently require only the identity field, so required columns (for example source_url) can be omitted and still pass schema validation.
Proposed fix
export function createTinyFishAgentOutputSchema(plan: DatasetBuildPlan) {
const rowProperties = Object.fromEntries(
plan.schema.columns.map((column) => [column.name, jsonSchemaForColumn(column)])
);
+ const requiredRowColumns = Array.from(
+ new Set([
+ plan.schema.identityColumnName,
+ ...plan.schema.columns
+ .filter((column) => column.isRequired)
+ .map((column) => column.name),
+ ])
+ );
return {
@@
rows: {
type: "array",
items: {
type: "object",
additionalProperties: false,
- required: [plan.schema.identityColumnName],
+ required: requiredRowColumns,
properties: rowProperties,
},
},📝 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.
| return { | |
| type: "object", | |
| additionalProperties: false, | |
| required: ["rows", "sourceUrls", "validationIssues"], | |
| properties: { | |
| rows: { | |
| type: "array", | |
| items: { | |
| type: "object", | |
| additionalProperties: false, | |
| required: [plan.schema.identityColumnName], | |
| properties: rowProperties, | |
| }, | |
| export function createTinyFishAgentOutputSchema(plan: DatasetBuildPlan) { | |
| const rowProperties = Object.fromEntries( | |
| plan.schema.columns.map((column) => [column.name, jsonSchemaForColumn(column)]) | |
| ); | |
| const requiredRowColumns = Array.from( | |
| new Set([ | |
| plan.schema.identityColumnName, | |
| ...plan.schema.columns | |
| .filter((column) => column.isRequired) | |
| .map((column) => column.name), | |
| ]) | |
| ); | |
| return { | |
| type: "object", | |
| additionalProperties: false, | |
| required: ["rows", "sourceUrls", "validationIssues"], | |
| properties: { | |
| rows: { | |
| type: "array", | |
| items: { | |
| type: "object", | |
| additionalProperties: false, | |
| required: requiredRowColumns, | |
| properties: rowProperties, | |
| }, |
🤖 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 `@backend/src/dataset-builder/agent-harness.ts` around lines 96 - 108, The JSON
schema for rows only requires plan.schema.identityColumnName and ignores other
columns marked as isRequired; update the construction of the rows schema in
agent-harness.ts so the required array includes plan.schema.identityColumnName
plus every column name from plan.schema.columns (or equivalent) where isRequired
=== true (deduplicate if necessary) before assigning it to the rows.required
field; ensure you keep additionalProperties: false and the existing
rowProperties mapping intact so validation enforces those required columns
(e.g., update the code that sets required: [plan.schema.identityColumnName] to
compute and use a merged requiredFields array).
| openRouterClient: shouldUseOpenRouter | ||
| ? createOpenRouterPlannerClient({ | ||
| apiKey: process.env.OPENROUTER_API_KEY, | ||
| model: process.env.OPENROUTER_MODEL || "openai/gpt-4.1-mini", | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether missing apiKey is already handled in client construction
rg -n -C3 'createOpenRouterPlannerClient|apiKey|OPENROUTER_API_KEY' \
backend/src/dataset-builder/openrouter.ts \
backend/src/dataset-builder/run-prototype.tsRepository: tinyfish-io/bigset
Length of output: 3929
Add explicit validation when --use-openrouter flag is used without API key
When --use-openrouter is set without OPENROUTER_API_KEY, the OpenRouter client silently returns undefined instead of alerting the user. This creates a confusing situation where an explicit flag request fails silently.
{
openRouterClient: shouldUseOpenRouter
- ? createOpenRouterPlannerClient({
- apiKey: process.env.OPENROUTER_API_KEY,
+ ? createOpenRouterPlannerClient({
+ apiKey: (() => {
+ const key = process.env.OPENROUTER_API_KEY;
+ if (!key) {
+ throw new Error(
+ "OPENROUTER_API_KEY is required when using --use-openrouter"
+ );
+ }
+ return key;
+ })(),
model: process.env.OPENROUTER_MODEL || "openai/gpt-4.1-mini",
})
: undefined,
}📝 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.
| openRouterClient: shouldUseOpenRouter | |
| ? createOpenRouterPlannerClient({ | |
| apiKey: process.env.OPENROUTER_API_KEY, | |
| model: process.env.OPENROUTER_MODEL || "openai/gpt-4.1-mini", | |
| }) | |
| openRouterClient: shouldUseOpenRouter | |
| ? createOpenRouterPlannerClient({ | |
| apiKey: (() => { | |
| const key = process.env.OPENROUTER_API_KEY; | |
| if (!key) { | |
| throw new Error( | |
| "OPENROUTER_API_KEY is required when using --use-openrouter" | |
| ); | |
| } | |
| return key; | |
| })(), | |
| model: process.env.OPENROUTER_MODEL || "openai/gpt-4.1-mini", | |
| }) | |
| : undefined, |
🤖 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 `@backend/src/dataset-builder/run-prototype.ts` around lines 27 - 31, The code
silently yields an undefined openRouterClient when shouldUseOpenRouter is true
but process.env.OPENROUTER_API_KEY is missing; update the logic around
shouldUseOpenRouter and createOpenRouterPlannerClient to explicitly validate the
API key: if shouldUseOpenRouter is true and process.env.OPENROUTER_API_KEY is
falsy, throw or exit with a clear error message (e.g., "OPENROUTER_API_KEY
required when --use-openrouter is set"), otherwise pass the API key into
createOpenRouterPlannerClient and assign to openRouterClient; reference
shouldUseOpenRouter, process.env.OPENROUTER_API_KEY,
createOpenRouterPlannerClient and openRouterClient when making the change.
| async function runTinyFishJson(args: string[]) { | ||
| const { stdout } = await execFileAsync("tinyfish", args, { | ||
| env: process.env, | ||
| maxBuffer: 1024 * 1024 * 10, | ||
| }); | ||
|
|
||
| return JSON.parse(stdout) as unknown; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current invocation safeguards around tinyfish process calls
rg -n -C3 'execFileAsync\(|maxBuffer|timeout|JSON.parse\(' backend/src/dataset-builder/tinyfish-cli.tsRepository: tinyfish-io/bigset
Length of output: 309
🏁 Script executed:
# Check execFileAsync import and file context
head -n 60 backend/src/dataset-builder/tinyfish-cli.tsRepository: tinyfish-io/bigset
Length of output: 1511
🏁 Script executed:
# Search for timeout usage patterns in similar execFile calls
rg -n 'timeout.*[0-9]' backend/src/ --type ts | head -20Repository: tinyfish-io/bigset
Length of output: 178
🏁 Script executed:
# Check if execFileAsync is a Node.js built-in or custom function
rg -n 'execFileAsync|execFile' backend/src/dataset-builder/tinyfish-cli.ts -B5 -A1Repository: tinyfish-io/bigset
Length of output: 395
🏁 Script executed:
# Verify coding guideline compliance for ESM imports in this file
head -n 30 backend/src/dataset-builder/tinyfish-cli.ts | grep -E '^import|^from'Repository: tinyfish-io/bigset
Length of output: 148
Harden TinyFish process execution with timeout and error diagnostics
Line 54 executes an external binary without a timeout, allowing a hung CLI run to block indefinitely. Additionally, line 59 discards stderr context when JSON parsing fails, hampering incident triage.
Add a 30-second timeout to the execFileAsync call and wrap JSON.parse in a try-catch that includes stderr output in the error message.
async function runTinyFishJson(args: string[]) {
- const { stdout } = await execFileAsync("tinyfish", args, {
+ const { stdout, stderr } = await execFileAsync("tinyfish", args, {
env: process.env,
maxBuffer: 1024 * 1024 * 10,
+ timeout: 30_000,
});
- return JSON.parse(stdout) as unknown;
+ try {
+ return JSON.parse(stdout) as unknown;
+ } catch (error) {
+ throw new Error(
+ `tinyfish returned non-JSON output. stderr=${stderr?.slice(0, 500) ?? ""}`,
+ { cause: error }
+ );
+ }
}🤖 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 `@backend/src/dataset-builder/tinyfish-cli.ts` around lines 53 - 60, The
runTinyFishJson function should guard against hung external calls and provide
stderr when JSON parsing fails: when calling execFileAsync in runTinyFishJson,
include a 30_000 ms timeout option (e.g., add timeout: 30_000 alongside env and
maxBuffer) and destructure stderr from the result; then wrap JSON.parse(stdout)
in a try-catch and, on failure, throw a new Error that includes the parse error
message plus the captured stderr (and stdout context if helpful) so callers get
diagnostic details; keep the function signature and return type (JSON parsed as
unknown).
| export interface DatasetRunCell { | ||
| columnName: string; | ||
| value: string | number | boolean | null; | ||
| sourceUrl?: string; | ||
| confidenceScore: number; | ||
| validationStatus: "valid" | "missing" | "needs_review"; | ||
| } |
There was a problem hiding this comment.
Allow JSON-valued cells in DatasetRunCell.
DatasetColumnKind supports "json", and the harness output schema allows object/array values, but DatasetRunCell.value currently rejects them. This creates a contract mismatch for valid JSON columns.
Proposed fix
+export type DatasetCellValue =
+ | string
+ | number
+ | boolean
+ | Record<string, unknown>
+ | unknown[]
+ | null;
+
export interface DatasetRunCell {
columnName: string;
- value: string | number | boolean | null;
+ value: DatasetCellValue;
sourceUrl?: string;
confidenceScore: number;
validationStatus: "valid" | "missing" | "needs_review";
}📝 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.
| export interface DatasetRunCell { | |
| columnName: string; | |
| value: string | number | boolean | null; | |
| sourceUrl?: string; | |
| confidenceScore: number; | |
| validationStatus: "valid" | "missing" | "needs_review"; | |
| } | |
| export type DatasetCellValue = | |
| | string | |
| | number | |
| | boolean | |
| | Record<string, unknown> | |
| | unknown[] | |
| | null; | |
| export interface DatasetRunCell { | |
| columnName: string; | |
| value: DatasetCellValue; | |
| sourceUrl?: string; | |
| confidenceScore: number; | |
| validationStatus: "valid" | "missing" | "needs_review"; | |
| } |
🤖 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 `@backend/src/dataset-builder/types.ts` around lines 71 - 77, The
DatasetRunCell.value type currently excludes objects/arrays causing a mismatch
with DatasetColumnKind "json"; update the DatasetRunCell interface (specifically
the value property) to accept JSON values (e.g., union with unknown |
Record<string, unknown> | unknown[] or a JSON type alias) so object and array
cells are allowed, and ensure downstream places that consume
DatasetRunCell.value are adjusted to handle the broader JSON type (look for
usages of DatasetRunCell.value to update any strict string/number/boolean
assumptions).
| const updateCadence = parseUpdateCadence(body.updateCadence); | ||
| if (body.updateCadence && !updateCadence) { | ||
| return { | ||
| ok: false, | ||
| error: "`updateCadence` must be manual, hourly, daily, or weekly.", | ||
| }; | ||
| } | ||
|
|
||
| const planningMode = parsePlanningMode(body.planningMode); | ||
| if (body.planningMode && !planningMode) { | ||
| return { | ||
| ok: false, | ||
| error: "`planningMode` must be deterministic or openrouter.", | ||
| }; | ||
| } |
There was a problem hiding this comment.
Tighten enum validation checks for optional fields.
Using truthy guards allows invalid empty-string values to bypass validation and fall through silently.
Proposed fix
const updateCadence = parseUpdateCadence(body.updateCadence);
- if (body.updateCadence && !updateCadence) {
+ if (body.updateCadence !== undefined && !updateCadence) {
@@
const planningMode = parsePlanningMode(body.planningMode);
- if (body.planningMode && !planningMode) {
+ if (body.planningMode !== undefined && !planningMode) {📝 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.
| const updateCadence = parseUpdateCadence(body.updateCadence); | |
| if (body.updateCadence && !updateCadence) { | |
| return { | |
| ok: false, | |
| error: "`updateCadence` must be manual, hourly, daily, or weekly.", | |
| }; | |
| } | |
| const planningMode = parsePlanningMode(body.planningMode); | |
| if (body.planningMode && !planningMode) { | |
| return { | |
| ok: false, | |
| error: "`planningMode` must be deterministic or openrouter.", | |
| }; | |
| } | |
| const updateCadence = parseUpdateCadence(body.updateCadence); | |
| if (body.updateCadence !== undefined && !updateCadence) { | |
| return { | |
| ok: false, | |
| error: "`updateCadence` must be manual, hourly, daily, or weekly.", | |
| }; | |
| } | |
| const planningMode = parsePlanningMode(body.planningMode); | |
| if (body.planningMode !== undefined && !planningMode) { | |
| return { | |
| ok: false, | |
| error: "`planningMode` must be deterministic or openrouter.", | |
| }; | |
| } |
🤖 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 `@backend/src/routes/dataset-builder.ts` around lines 76 - 90, The current
guards use truthy checks on body.updateCadence and body.planningMode which let
empty strings slip past validation; update the checks around parseUpdateCadence
and parsePlanningMode to explicitly detect presence (e.g., check
body.updateCadence !== undefined && body.updateCadence !== null and if it's a
string ensure length > 0) before calling the parser and returning the error when
the parser returns falsy; reference the symbols updateCadence,
parseUpdateCadence, planningMode, parsePlanningMode, body.updateCadence, and
body.planningMode when making the change.
| return { | ||
| ok: true, | ||
| value: { | ||
| userRequest, | ||
| updateCadence, | ||
| planningMode: planningMode ?? "openrouter", | ||
| providedInputs: parseStringRecord(body.providedInputs), | ||
| preferredColumns: parseStringArray(body.preferredColumns), | ||
| }, |
There was a problem hiding this comment.
Make OpenRouter refinement opt-in, not default.
Defaulting planningMode to "openrouter" means userRequest and providedInputs are sent to a third party whenever an API key is configured, even if the caller didn’t explicitly ask for that path.
Proposed fix
value: {
userRequest,
updateCadence,
- planningMode: planningMode ?? "openrouter",
+ planningMode: planningMode ?? "deterministic",
providedInputs: parseStringRecord(body.providedInputs),
preferredColumns: parseStringArray(body.preferredColumns),
},🤖 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 `@backend/src/routes/dataset-builder.ts` around lines 92 - 100, The code
currently defaults planningMode to "openrouter", causing userRequest and
providedInputs to be sent to a third party by default; change the return so
planningMode is not defaulted to "openrouter" (use planningMode as-is or
undefined) and only enable the openrouter behavior when planningMode ===
"openrouter" is explicitly provided by the caller; update the return object in
the function that builds the response (the block returning { ok: true, value: {
userRequest, updateCadence, planningMode: ... , providedInputs:
parseStringRecord(...), preferredColumns: ... } }) to stop applying the nullish
coalescing default for planningMode and ensure any downstream code only sends
userRequest/providedInputs when planningMode is explicitly "openrouter".
| updateCadence: text("update_cadence") | ||
| .$type<DatasetUpdateCadence>() | ||
| .notNull(), | ||
| status: text("status") | ||
| .$type<"draft" | "needs_input" | "ready" | "running" | "failed">() | ||
| .notNull() | ||
| .default("draft"), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Drizzle ORM v0.45.x, does text(...).$type<...>() enforce runtime/database constraints, or is it TypeScript-only?
💡 Result:
In Drizzle ORM, the.$type<...> method is strictly for TypeScript-only type inference and does not enforce any runtime or database-level constraints [1][2]. It is used to override or specify the TypeScript type for a column, which is particularly useful for handling complex types (like JSON objects), branded types, or custom data structures that Drizzle's default inference might not capture accurately [1][2]. Because it is a compile-time feature, it does not perform any validation on the data being inserted into or retrieved from the database [1][2]. If you require runtime validation or database-level constraints, you should use appropriate mechanisms such as: - Database-level constraints: Use Drizzle's schema definition features for PRIMARY KEY, UNIQUE, NOT NULL, and CHECK constraints, which are enforced by the database engine itself [3][4]. - Runtime validation: For validating data before it reaches the database, you can use schema validation libraries (e.g., Zod, Valibot) or Drizzle's $validator method (where available) to integrate with Standard Schema-compliant validators [5].
Citations:
- 1: https://orm.drizzle.team/docs/column-types/pg
- 2: https://orm.drizzle.team/docs/column-types/sqlite
- 3: https://drizzle-team-drizzle-orm.mintlify.app/schema/constraints
- 4: https://orm.drizzle.team/docs/indexes-constraints
- 5: feat(drizzle-orm): Add $validator() for Standard Schema integration on columns drizzle-team/drizzle-orm#5167
🏁 Script executed:
# Check the backend package.json for Drizzle ORM version
head -50 backend/package.json | grep -A 5 -B 5 "drizzle"Repository: tinyfish-io/bigset
Length of output: 623
🏁 Script executed:
# Read the schema.ts file to verify the current state of lines 77-83 and 102-105
cat -n backend/src/schema.ts | head -115 | tail -50Repository: tinyfish-io/bigset
Length of output: 2094
🏁 Script executed:
# Check what's imported from drizzle-orm/pg-core in the current schema.ts
cat -n backend/src/schema.ts | head -20Repository: tinyfish-io/bigset
Length of output: 550
🏁 Script executed:
# Find DatasetUpdateCadence type definition
rg "DatasetUpdateCadence" backend/src --type ts -A 5Repository: tinyfish-io/bigset
Length of output: 4019
🏁 Script executed:
# Check if pgEnum is available in drizzle-orm/pg-core by searching docs or examples
cd backend && npm list drizzle-orm 2>/dev/null | head -5Repository: tinyfish-io/bigset
Length of output: 123
🏁 Script executed:
# Get the actual type definition for DatasetUpdateCadence
cat -n backend/src/dataset-builder/types.ts | grep -A 2 "DatasetUpdateCadence"Repository: tinyfish-io/bigset
Length of output: 464
🏁 Script executed:
# Search for all text fields with $type to see if any others need enum enforcement
rg '\$type<' backend/src/schema.tsRepository: tinyfish-io/bigset
Length of output: 445
Enforce status and cadence values at the database layer
Lines 77 and 102 currently rely on TypeScript-only unions via .$type<...>(). Postgres will accept arbitrary strings, which can leak invalid states into persisted datasets and runs. Use pgEnum to enforce values at the database level.
Suggested change
import {
boolean,
index,
integer,
jsonb,
+ pgEnum,
pgTable,
text,
timestamp,
} from "drizzle-orm/pg-core";
+const datasetCadenceEnum = pgEnum("dataset_cadence", ["manual", "hourly", "daily", "weekly"]);
+const datasetStatusEnum = pgEnum("dataset_status", ["draft", "needs_input", "ready", "running", "failed"]);
+const datasetRunStatusEnum = pgEnum("dataset_run_status", ["queued", "running", "needs_input", "succeeded", "failed"]);
+
export const dataset = pgTable(
"dataset",
{
@@
- updateCadence: text("update_cadence")
- .$type<DatasetUpdateCadence>()
- .notNull(),
- status: text("status")
- .$type<"draft" | "needs_input" | "ready" | "running" | "failed">()
- .notNull()
- .default("draft"),
+ updateCadence: datasetCadenceEnum("update_cadence").notNull(),
+ status: datasetStatusEnum("status").notNull().default("draft"),
@@
export const datasetRun = pgTable(
"dataset_run",
{
@@
- status: text("status")
- .$type<"queued" | "running" | "needs_input" | "succeeded" | "failed">()
- .notNull()
- .default("queued"),
+ status: datasetRunStatusEnum("status").notNull().default("queued"),Also applies to: 102-105
🤖 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 `@backend/src/schema.ts` around lines 77 - 83, The status and updateCadence
columns currently use TypeScript-only unions (.$type<...>()) so invalid strings
can be persisted; replace those with pgEnum-backed types by defining pgEnum
enums for DatasetUpdateCadence and the run/dataset status union, then change the
column definitions (updateCadence and status) to use pgEnum(...) instead of
text(...). Ensure you create the enum declarations (e.g., DatasetUpdateCadence
enum and DatasetStatus/RunStatus enum) and reference them in the schema column
definitions so Postgres enforces allowed values at the DB layer.
giaphutran12
force-pushed
the
codex/bigset-agent-harness-autopilot
branch
from
288c710 to
c71028d
Compare
1 participant
Closes #4
Summary
Verification