Add admin pet generation pilot - #41
Conversation
Add the admin-only generation state machine, private YDB artifact storage, leased worker, deterministic v2 package assembly, review UI, moderation linkage, migration, tests, and rollout documentation. Generation remains disabled by default and OpenAI credentials stay worker-only.
There was a problem hiding this comment.
Pull request overview
Adds an admin-only “pet generation” pilot across the web app and a separate worker process, backed by new YDB tables and a run state machine, with artifacts persisted privately and final approval tied into the existing moderation flow.
Changes:
- Add YDB schema + migration for generation runs, stage attempts, and chunked artifacts.
- Implement generation-run persistence/state transitions, submission→pending-pet creation (deterministic IDs), and moderation linkage.
- Add admin API routes and UI components for creating/controlling runs and viewing moderated artifacts; add a single-concurrency worker script + OpenAI provider pipeline.
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/schema.yql | Adds generation run/attempt/artifact tables to the canonical schema. |
| ydb/migrations/20260808_001_add_pet_generation_runs.mjs | Adds migration to create new generation tables when missing. |
| src/lib/ydb/schema.ts | Registers new generation table names in app constants. |
| src/lib/pets/repository.ts | Adds deterministic pending-pet IDs for replay-safe generation submissions. |
| src/lib/pets/mock-data.ts | Allows deterministic mock pet IDs to support replay/idempotency tests. |
| src/lib/pets/generation/types.ts | Defines generation run/stage/attempt/artifact types and enums. |
| src/lib/pets/generation/submission.ts | Implements final approval submission: validate artifacts/package and create deterministic pending pet. |
| src/lib/pets/generation/submission.test.ts | Tests replay-safe submission and moderation linkage using mock datasource. |
| src/lib/pets/generation/state-machine.ts | Adds run status transition rules and conflict error type. |
| src/lib/pets/generation/retry-policy.ts | TS re-export wrapper for runtime retry policy. |
| src/lib/pets/generation/retry-policy-runtime.mjs | Implements bounded retry decisions and call budget enforcement. |
| src/lib/pets/generation/retry-policy-runtime.d.mts | Adds types for retry policy runtime module. |
| src/lib/pets/generation/repository.ts | Adds YDB-backed generation run + artifact persistence and admin mutations. |
| src/lib/pets/generation/repository-idempotency.test.ts | Tests run creation idempotency and conflict behavior (mock). |
| src/lib/pets/generation/input.ts | TS re-export wrapper for reference-image normalization runtime. |
| src/lib/pets/generation/input-runtime.mjs | Validates/normalizes reference images into safe PNG inputs. |
| src/lib/pets/generation/input-runtime.d.mts | Types for normalized reference image helper. |
| src/lib/pets/generation/final-metadata.ts | Validates final moderator-owned metadata (id/name/desc/kind/tags). |
| src/lib/pets/generation/contracts.test.ts | Contract tests for config defaults, transitions, chunking, retry policy, and metadata/ref normalization. |
| src/lib/pets/generation/config.ts | TS re-export wrapper for generation config runtime. |
| src/lib/pets/generation/config-runtime.mjs | Defines env-driven, bounded generation config with disabled-by-default defaults. |
| src/lib/pets/generation/config-runtime.d.mts | Types for generation config runtime module. |
| src/lib/pets/generation/artifact-route.test.ts | Tests admin-only artifact route behavior and header hardening. |
| src/lib/pets/generation/artifact-chunks.ts | Implements chunking + SHA verification for stored artifacts. |
| src/lib/pets/generation/admin-api.ts | Adds admin gate + idempotency key parsing + mutation response helper. |
| src/lib/pets/generation/admin-api.test.ts | Tests admin gate ordering, disabled-flag behavior, and idempotency key validation. |
| src/lib/pets/generation-requests-repository.ts | Exposes getGenerationRequestById and adds request “reopen” helper. |
| src/components/GenerationRequestsTable/GenerationRequestsTable.tsx | Displays latest run card per request and disables manual actions while automation active. |
| src/components/AdminGenerationRunCard/AdminGenerationRunCard.tsx | Adds admin UI for run progress, artifact previews, and run actions/approval form. |
| src/components/AdminGenerationRunCard/AdminGenerationRunCard.scss | Styles the new run card and previews. |
| src/components/AdminGenerationRequestActions/AdminGenerationRequestActions.tsx | Adds automationActive mode to route admins to run actions. |
| src/app/api/admin/submissions/[id]/reject/route.ts | Links rejection to generated-run reopening logic. |
| src/app/api/admin/submissions/[id]/delete/route.ts | Links deletion to generated-run reopening logic. |
| src/app/api/admin/submissions/[id]/approve/route.ts | Links approval to generation run completion. |
| src/app/api/admin/generation-runs/[id]/retry/route.ts | Adds admin endpoint to retry failed runs. |
| src/app/api/admin/generation-runs/[id]/regenerate-base/route.ts | Adds admin endpoint to regenerate the base image. |
| src/app/api/admin/generation-runs/[id]/cancel/route.ts | Adds admin endpoint to cancel a run. |
| src/app/api/admin/generation-runs/[id]/artifacts/[key]/route.ts | Adds private admin artifact download/preview endpoint. |
| src/app/api/admin/generation-runs/[id]/approve-final/route.ts | Adds admin endpoint to submit final metadata and create pending pet. |
| src/app/api/admin/generation-runs/[id]/approve-base/route.ts | Adds admin endpoint to approve the base image and queue hatch. |
| src/app/api/admin/generation-requests/[id]/runs/route.ts | Adds admin endpoint to create a run with Idempotency-Key support. |
| src/app/admin/requests/page.tsx | Loads latest runs and passes generation-enabled flag into admin requests UI. |
| scripts/lib/pet-generation-worker-ydb.mjs | Adds YDB client bootstrap for the standalone worker container. |
| scripts/lib/pet-generation-worker-runtime.test.ts | Tests worker orchestration, leasing, retries, moderation, and budgeting with an in-memory repository. |
| scripts/lib/pet-generation-worker-runtime.mjs | Implements concurrency-one worker runtime with call ledger, retries, and artifact writing. |
| scripts/lib/pet-generation-worker-runtime.d.mts | Declares worker runtime types for TS consumers/tests. |
| scripts/lib/pet-generation-worker-repository.mjs | Implements YDB-backed worker repository: leasing, attempts ledger, artifact chunk persistence, cleanup. |
| scripts/lib/pet-generation-provider.test.ts | Tests OpenAI provider request shapes and ambiguous-loss behavior. |
| scripts/lib/pet-generation-provider.mjs | Implements OpenAI image/moderation/review provider with strict validation and sizing limits. |
| scripts/lib/pet-generation-provider.d.mts | Type declarations for the OpenAI provider module. |
| scripts/lib/pet-generation-pipeline.test.ts | Tests deterministic v2 pipeline assembly and artifact outputs. |
| scripts/lib/pet-generation-pipeline.mjs | Implements deterministic v2 sprite pipeline (grid processing, atlas assembly, QA artifacts). |
| scripts/lib/pet-generation-pipeline.d.mts | Type declarations for pipeline module. |
| scripts/generation-worker.mjs | Adds runnable worker entrypoint wiring config, YDB repo, provider, and polling loop. |
| package.json | Adds generation:worker script entry. |
| DEPLOYMENT.md | Documents rollout boundary, worker container setup, and additive rollout/rollback steps. |
| deploy/generation-worker.env.runtime.example | Adds example env file for the worker container (includes key mount). |
| deploy/app-session.env.runtime.example | Adds web-container env notes for generation pilot (explicitly no OpenAI key). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return new Response(new Uint8Array(artifact.buffer), { headers: { | ||
| "Cache-Control": "private, no-store", | ||
| "Content-Type": artifact.metadata.contentType, | ||
| "Content-Disposition": `inline; filename="${safeName(artifact.metadata.fileName)}"`, | ||
| "X-Content-Type-Options": "nosniff", | ||
| } }); |
| export async function listLatestGenerationRunsByRequestIds(ids: readonly string[]) { | ||
| const wanted = new Set(ids); | ||
| if (!wanted.size) return new Map<string, PetGenerationRun>(); | ||
| let runs: PetGenerationRun[]; | ||
| if (isMockPetsDataSource()) runs = Array.from(mockRuns.values()); | ||
| else if (!isYdbConfigured()) runs = []; | ||
| else runs = rowsFromResult(await withSession((session) => | ||
| session.executeQuery(`SELECT ${RUN_COLUMNS} FROM ${TABLES.generationRuns} ORDER BY created_at DESC LIMIT 500;`), | ||
| )).map(parseRun); | ||
| const result = new Map<string, PetGenerationRun>(); | ||
| for (const run of runs.filter((item) => wanted.has(item.requestId)).sort((a, b) => b.createdAt.localeCompare(a.createdAt))) { | ||
| if (!result.has(run.requestId)) result.set(run.requestId, run); | ||
| } | ||
| return result; |
| <div className="admin-generation-run-card__progress" aria-label={`${progress}% complete`}> | ||
| <span style={{ width: `${progress}%` }} /> | ||
| </div> |
| if (input.deterministicPetId) { | ||
| const existing = await getPetById(input.deterministicPetId); | ||
| if (existing) return toPublicPet(existing, EMPTY_METRICS, await getOwnerProfileByRow(existing)); | ||
| } |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const transitioned = await transitionGenerationRun({ | ||
| runId: run.id, | ||
| status: "awaiting_moderation", | ||
| finalMetadata: input.metadata, | ||
| finalPetId: pet.id, | ||
| finalPetSlug: pet.slug, | ||
| approvedBy: input.approvedBy, | ||
| }); |
There was a problem hiding this comment.
createPendingPet persists a moderation-visible pet before the separate transition records finalPetId and moves the run to awaiting_moderation. If execution stops or that transition fails, an administrator can approve the persisted pet, but completeGeneratedPetModeration cannot find an awaiting_moderation run by its ID and returns without completing the run or fulfilling its request. Make pet creation and run linkage atomic, or keep the pet unavailable for moderation until linkage succeeds and recover already-created pending pets.
Artifacts
Focused executable harness for linked and unlinked generation moderation states
- A Vitest harness was authored and run against mock persistence to compare moderation after a linked versus unlinked pending-pet creation, showing the unlinked approval leaves both records incomplete.
Control run log for an already linked awaiting-moderation generation
- The focused control command passed after approving a pet linked to an awaiting-moderation run, showing the expected completed run and fulfilled request path.
Race reproduction log for an unlinked pet while the run is submitting
- The focused race command passed its assertions after approving a pet created while its run remained submitting, showing the incomplete-state consequence is reproducible.
Combined focused generation linkage test log
- The full focused harness passed both the linked control and unlinked race assertions in one execution, confirming the behavioral contrast.
Typecheck log for the focused validation harness
- The repository TypeScript check completed successfully after adding only the artifact-contained validation harness, confirming it compiles cleanly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/pets/generation/submission.ts
Line: 97-104
Comment:
**Pending pet can be approved before generation linkage**
`createPendingPet` persists a moderation-visible pet before the separate transition records `finalPetId` and moves the run to `awaiting_moderation`. If execution stops or that transition fails, an administrator can approve the persisted pet, but `completeGeneratedPetModeration` cannot find an `awaiting_moderation` run by its ID and returns without completing the run or fulfilling its request. Make pet creation and run linkage atomic, or keep the pet unavailable for moderation until linkage succeeds and recover already-created pending pets.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d770f43da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| automationActive={Boolean( | ||
| row.generationRun && | ||
| !["cancelled", "submission_rejected"].includes( | ||
| row.generationRun.status, | ||
| ), |
There was a problem hiding this comment.
When the pilot is rolled back with PET_GENERATION_ENABLED=false, a request with any existing run other than cancelled or submission_rejected still sets automationActive, so the table hides the manual attach/reject controls. The remaining run actions all pass through requireGenerationAdmin() and return 503 while disabled, leaving queued, failed, or review-stage requests with no usable admin workflow despite the rollback procedure in DEPLOYMENT.md stating that manual flows continue.
Useful? React with 👍 / 👎.
| const changed = await transitionGenerationRun({ | ||
| runId: run.id, | ||
| status: "submitting", | ||
| lastStage: "submission", | ||
| finalMetadata: input.metadata, |
There was a problem hiding this comment.
If any subsequent artifact read, package validation, asset write, or pet write fails after this transition, the run remains submitting. The worker does not process that status, and after a refresh the run card no longer renders the approval form, retry button, or cancel button, so the admin cannot recover the request through the UI; transition the run to a recoverable state on failure or keep a submission retry action available.
Useful? React with 👍 / 👎.
| if (run.targetedRetryCount > 0) return run.lastStage === stage | ||
| ? { key: `work-${alias}-t${run.targetedRetryCount}`, alias } | ||
| : { key: alias, alias }; |
There was a problem hiding this comment.
When mechanical QA attributes a failure to cardinal, a targeted retry gives only that stage a revision key while look-row-9 and look-row-10 reuse their aliases. Those two images were generated with the old cardinal image as an explicit reference in hatchV2Pet, so the retry can combine a new cardinal guide with stale dependent direction rows and approve an inconsistent atlas; cardinal retries need to invalidate and regenerate both look rows as well.
Useful? React with 👍 / 👎.
| SELECT run_id,artifact_key FROM ${TABLES.artifacts} | ||
| WHERE retained=$retained AND expires_at<$expires_at LIMIT 100;`, |
There was a problem hiding this comment.
The query limits expired artifacts to 100 before the loop excludes nonterminal runs. Once 100 expired artifacts belong to failed, review-stage, or moderation-stage runs, each hourly pass can repeatedly select and skip that same set, preventing expired artifacts from terminal runs from ever being reached; this is realistic because a single hatch stores dozens of artifacts. Filter or paginate by terminal eligibility before imposing the batch limit so retention cleanup cannot starve.
Useful? React with 👍 / 👎.
| await requireRunUpdate(repository, claimed.run.id, { | ||
| status: "validating", lastStage: "assembly", expectedStatuses: ["generating"], | ||
| }); |
There was a problem hiding this comment.
If the worker dies after moving a hatch run to validating but before all artifacts are stored, findRunnableRun selects it and claimRun deliberately preserves validating; the resumed execution then reaches this update with expectedStatuses: ["generating"], treats the mismatch as cancellation, and leaves the run in validating. Every subsequent poll repeats the same path, so this crash window permanently loops instead of completing or failing the run.
Useful? React with 👍 / 👎.
| const confirmed = await getGenerationRunById(id); | ||
| return confirmed?.updatedAt === next.updatedAt ? { ok: true, run: confirmed } : conflict("Run changed concurrently."); |
There was a problem hiding this comment.
Under concurrent run actions, two transitions can compute the same millisecond-resolution updatedAt; one conditional update can lose because the status changed, yet this check still reports success when the winner stored that same timestamp, even if its status is different from the requested one. This can make an approve, cancel, or retry response claim success for an action that never occurred, so confirmation must also verify the intended status or use a monotonic version returned by the conditional write.
Useful? React with 👍 / 👎.
| const [request, spritesheet] = await Promise.all([ | ||
| getGenerationRequestById(run.requestId), | ||
| readGenerationArtifact({ runId: run.id, key: "spritesheet" }), | ||
| ]); | ||
| if (!request || !spritesheet) { | ||
| return { ok: false, error: "invalid_package", message: "Required final artifacts are missing." }; |
There was a problem hiding this comment.
If an admin rejects or deletes the generation request while its run is active, these existing mutation routes do not cancel the run and getGenerationRequestById still returns the closed record, so this check accepts it and creates a pending pet anyway. A rejected request can later be changed back to fulfilled when that pet is approved, while a deleted request leaves an approved pet with a run stuck in awaiting_moderation; require the request to remain pending or in_progress, and coordinate rejection/deletion with run cancellation.
AGENTS.md reference: src/AGENTS.md:L70-L70
Useful? React with 👍 / 👎.
| if (run.status !== "awaiting_final_review" && run.status !== "submitting") { | ||
| return { ok: false, error: "conflict", message: "Run is not awaiting final review." }; |
There was a problem hiding this comment.
Once one approval has changed the run to submitting but is still assembling or storing the package, a second request is admitted here and may supply different metadata. Both requests can pass the deterministic-ID existence check before either pet write; if their metadata IDs produce different slugs, codex_pets accepts both because its primary key is the slug, leaving two rows with the same generated pet ID and a shared asset ID whose contents are overwritten by the last writer. Serialize this stage or make replays use the persisted final metadata under an idempotent transaction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e815d64c9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| enabled: env.PET_GENERATION_ENABLED?.trim().toLowerCase() === "true", | ||
| model: env.PET_GENERATION_MODEL?.trim() || PET_GENERATION_DEFAULT_MODEL, | ||
| reviewModel: env.PET_GENERATION_REVIEW_MODEL?.trim() || PET_GENERATION_DEFAULT_REVIEW_MODEL, | ||
| maxImageCalls: bounded(env.PET_GENERATION_MAX_IMAGE_CALLS, 15, 1, 15), |
There was a problem hiding this comment.
With the default—and maximum permitted—budget of 15, a cardinal QA retry cannot complete: the initial base plus 12 hatch images consumes 13 calls, and retrying cardinal now correctly regenerates cardinal and both dependent look rows, requiring calls 14–16. The third regenerated image therefore receives image_call_budget_exhausted, while the run has already consumed its sole targeted retry, so this failure is unrecoverable without starting over.
Useful? React with 👍 / 👎.
| if (!request) { | ||
| return NextResponse.json({ error: "not_found" }, { status: 404 }); | ||
| } | ||
| if (guard.runId) await cancelGenerationRun(guard.runId); |
There was a problem hiding this comment.
In the reject route, if the worker changes an awaiting_final_review run to submitting after the guard returns but before rejectGenerationRequest completes, this cancellation is rejected because submitting -> cancelled is not allowed, and its result is ignored; submission then observes the closed request and returns with the run permanently stuck in submitting. The fulfill and delete routes use the same ordering. Fresh evidence in this revision is that the newly added guard and cancellation remain separated by the request write and do not check cancellation failure, so the previously identified closure race is still possible.
AGENTS.md reference: src/AGENTS.md:L70-L70
Useful? React with 👍 / 👎.
| if (!input.allowExpired) { | ||
| const run = await readRun(execute, input.runId); | ||
| if (!run) return { kind: "missing" }; | ||
| if (TERMINAL.has(run.status)) return { kind: "cancelled" }; |
There was a problem hiding this comment.
If both heartbeat updates fail long enough for the orchestration and provider leases to expire, a replacement worker marks the provider attempt ambiguous and moves the run to failed, but the original provider call can still return and continue because provider acquisition rejects only terminal statuses, not failed or a lost orchestration lease. Its token-guarded finishAttempt silently becomes a no-op, yet it can dispatch the remaining expensive image calls and overwrite aliases, potentially concurrently with an admin-triggered targeted retry; require the run to remain in its claimed generating state and verify lease ownership before permitting another provider attempt.
Useful? React with 👍 / 👎.
| const pet = await createPendingPet({ | ||
| deterministicPetId: deterministicPetId(run.id), | ||
| petJson: validation.value.petJson, | ||
| ownerId: request.requesterUserId ?? "", |
There was a problem hiding this comment.
For a logged-in requester, assigning the generated pending pet to requesterUserId lets the existing owner-delete endpoint soft-delete it while the run is awaiting_moderation. Only the admin reject/delete routes call reopenGeneratedPetRequest; the owner route does not, so the run remains active, the request remains in_progress, and the deleted pet can no longer be moderated or recovered through the generation UI. Route owner deletion through the same generation reconciliation before exposing the pending pet to its owner.
AGENTS.md reference: src/AGENTS.md:L70-L70
Useful? React with 👍 / 👎.
| if (normalizeContentType(input.declaredContentType) !== detected) { | ||
| throw new Error("Reference image content type does not match its bytes."); | ||
| } |
There was a problem hiding this comment.
A multipart request can declare image/png while supplying a readable JPEG (or another allowed format), because the public request route checks the declared MIME and calls only sharp(...).metadata(). The request is accepted and shown as generation-ready, but this later byte/MIME equality check deterministically fails before any generation call; retrying the failed base repeats the same failure and consumes the run's sole targeted retry. Perform the same signature normalization at upload time, or persist the detected type instead of trusting the declaration.
AGENTS.md reference: src/AGENTS.md:L58-L58
Useful? React with 👍 / 👎.
Summary
Verification
Rollout boundary
This PR does not apply the YDB migration, deploy the worker, mount an OpenAI key, enable the feature, or run the five-request pilot.
Greptile Summary
This change adds an administrative pet-generation workflow with provider execution, generated artifacts, moderation, and generation-request tracking. A pending pet can become available for approval before its generation run is durably linked to it; approval in that window publishes the pet but leaves the run and its request incomplete.
Confidence Score: 4/5
Not safe to merge until pending-pet creation and generation-run linkage cannot be separated, or the workflow can recover the linkage before moderation approval.
A focused executable test exercised the normal linked approval path and the unlinked interruption path. The normal path completed the generation run and request, while the interrupted path left both records incomplete after the pet was approved.
Files Needing Attention: src/lib/pets/generation/submission.ts needs atomicity or recovery for pending-pet linkage. src/lib/pets/generation/repository.ts and src/app/api/admin/submissions/[id]/approve/route.ts need to ensure approval cannot silently bypass completion of an unlinked generation run.
What T-Rex did
Comments Outside Diff (1)
General comment
submitGenerationRuncreates a moderation-visible pending pet, then performs a separate conditional run transition that writesfinalPetId/finalPetSlug. If that second operation fails or the process stops, the pet remains approvable. The approve route approves it and callscompleteGeneratedPetModeration, but that helper cannot find a run by the pet ID and returns immediately. Executed evidence confirms the run remainssubmittingand unlinked and the request remainsin_progressand unlinked.finalPetIdand inawaiting_moderation.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Add admin pet generation pilot" | Re-trigger Greptile