feat: add phase 04 analyze runtime and result bundles#4
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (9)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (Custom checks)
Files:
src/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{ts,tsx,js,jsx,css}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{ts,tsx,js,jsx,css,html}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{ts,tsx,js,jsx,html}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{ts,tsx,js,jsx,html,css}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔍 Remote MCP FirecrawlRelevant findings for this PR (short)
Concrete reviewer-relevant facts (observed in the cited sources)
Suggested concrete action items for this PR (grounded in the above facts)
Sources / evidence
🔇 Additional comments (20)
WalkthroughImplements an analysis runtime and model-routing pipeline: adds the Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Pull request overview
This PR delivers Phase 04 of deps-workbench by adding a live analyze command backed by an OpenAI Agents SDK runtime, policy-based routing with bounded escalation recovery, and canonical schema-validated result bundle generation (plus matching docs/fixtures/tests).
Changes:
- Add
analyzeCLI command and runtime (intake→eligibility→routing→ model execution → optional single-hop escalation recovery → result bundle write). - Introduce/expand Zod schemas for analysis synthesis + result bundle artifacts (manifest/report/checklists/evidence map) and policy routing weights/thresholds.
- Add regression tests + fixture/doc updates reflecting the new analyze surface and blocked/recovery semantics.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/openai-analysis.test.ts | Verifies Agents SDK is configured for schema-typed output and prompt expectations. |
| test/cli.test.ts | Adds CLI parsing coverage for analyze. |
| test/analyze-run.test.ts | End-to-end-ish tests for routing, blocked runs, bounded escalation recovery, and schema-valid result bundles. |
| src/schemas/result.ts | Adds result-bundle and synthesis schemas/types (claims, evidence, routing, recovery, manifests). |
| src/schemas/policy.ts | Extends policy schema with routing thresholds/weights and validation. |
| src/schemas/enums.ts | Adds enums/types for change intent, claim buckets, recovery status, etc. |
| src/core/storage/paths.ts | Adds .local/runs/<runId>/result path helper. |
| src/core/runtime/routing.ts | Implements weighted routing score and tier selection. |
| src/core/runtime/recovery.ts | Implements gating + record builder for bounded escalation recovery. |
| src/core/runtime/intake.ts | Loads prepared bundle artifacts from runId/manifest. |
| src/core/runtime/eligibility.ts | Structural eligibility checks producing blockers/degradations and fallback claims. |
| src/core/runtime/analyze-run.ts | Orchestrates analyze flow including recovery and result bundle writing. |
| src/core/results/write-result-bundle.ts | Writes canonical result bundle artifacts + manifest and executive brief. |
| src/core/models/openai-analysis.ts | OpenAI Agents SDK model executor using outputType for typed synthesis output. |
| src/commands/analyze.ts | Implements the analyze command runner and help/summary output. |
| src/cli.ts | Wires analyze into CLI parsing/help and execution. |
| README.md | Marks Phase 04 done and documents analyze usage and behavior. |
| package.json | Adds @openai/agents dependency. |
| fixtures/schemas/minimal-result-manifest.json | Updates minimal manifest fixture to include new required fields (routing, etc.). |
| fixtures/prep/repo-basic/config/deps-workbench.config.jsonc | Adds routing thresholds/weights to fixture policy config. |
| docs/references/repo-architecture.md | Documents result bundle semantics, schema-typed output, and recovery rules. |
| docs/references/cli-reference.md | Updates command status and analyze behavior reference. |
| docs/plan/README.md | Marks Phase 04 complete in the plan ledger. |
| docs/plan/Plan-04-analysis-runtime-and-model-routing.md | Marks Phase 04 complete + records delivered items/verification. |
| config/deps-workbench.config.jsonc | Adds routing thresholds/weights to default policy config. |
| bun.lock | Locks new dependency graph including @openai/agents. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: a98c065344
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/schemas/result.ts (1)
25-297:⚠️ Potential issue | 🟠 MajorDocument every changed export in this module.
From Line 25 onward, the added/modified exported schemas and inferred type aliases are missing the required immediately preceding
/** ... */comments. This file will fail the TS doc contract until each changed export is documented, with a one-sentence summary on the first non-empty line.As per coding guidelines,
Every changed or added exported declaration must have an immediately preceding doc comment /** ... */ with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/result.ts` around lines 25 - 297, The exported schemas and inferred type aliases added/modified from confidenceSchema onward are missing the required leading JSDoc comments; add an immediately preceding /** One-sentence summary. */ (no blank line) before each exported symbol to satisfy the TS doc contract. Specifically, add doc comments above the exports: confidenceSchema, evidenceReferenceSchema, resultClaimSchema, implementationChecklistItemSchema, validationChecklistItemSchema, openQuestionSchema, decisionReportSchema, evidenceMapSchema, implementationChecklistSchema, validationChecklistSchema, openQuestionsSchema, analysisSynthesisSchema, routingScoreFactorSchema, routingDecisionSchema, recoveryRecordSchema, resultFilesSchema, resultUnverifiedItemSchema, resultManifestSchema, and the exported type aliases EvidenceReference, ResultClaim, ImplementationChecklistItem, ValidationChecklistItem, OpenQuestion, DecisionReport, EvidenceMap, ImplementationChecklist, ValidationChecklist, OpenQuestions, AnalysisSynthesis, RoutingDecision, RecoveryRecord, ResultManifest; ensure each comment is a single-sentence summary on the first non-empty line and placed immediately above the export with no blank line in between.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 77: Update the Quick start shell snippet that currently shows "bun run
dev analyze --run-id <run-id>" to use a copy-paste-safe example value (e.g.,
"bun run dev analyze --run-id run-01" or "bun run dev analyze --run-id abc123")
so users won't accidentally trigger shell redirection; change the literal
"<run-id>" in the README snippet to a concrete sample token in the shown
command.
In `@src/commands/analyze.ts`:
- Around line 6-8: Add JSDoc comments for all exported declarations in this
module: add a /** ... */ comment immediately above AnalyzeCommandOptions and
AnalyzeRunOptions describing their purpose and properties, and add full JSDoc
comments for every exported function in the file (document every parameter with
`@param` and include an `@returns` describing the return value for non-void
functions). Ensure the comments directly precede the exported declarations and
cover the symbols AnalyzeCommandOptions, AnalyzeRunOptions and each exported
function in this file.
In `@src/core/models/openai-analysis.ts`:
- Around line 11-20: Add doc comments above the exported types
AnalysisModelInput and AnalysisModelExecutor explaining their purpose and the
meaning of their fields/return value: document that AnalysisModelInput contains
prepBundle (LoadedPrepBundle), routingDecision (RoutingDecision), and
structuralAssessment (StructuralAssessment), and document that
AnalysisModelExecutor is an async function that accepts AnalysisModelInput and
returns a Promise resolving to an object with synthesis (AnalysisSynthesis) and
modelUsed (string). Keep the comments concise, follow project doc comment style
(brief description + parameter/return descriptions), and place them immediately
above the respective type declarations.
- Around line 1-10: This file imports the AI SDK symbols Agent and run from
'@openai/agents' but lacks the required doc comment; add a top-of-file JSDoc
block that includes either an `@see` pointing to the ADR/SPEC path or official
OpenAI docs URL or an `@remarks` containing an ADR- or SPEC- token (e.g., "@see
SPEC-xxx" or "@see https://platform.openai.com/docs") so the import of Agent and
run complies with the coding guideline; keep the comment directly above the
imports and ensure it clearly references the required ADR/SPEC or docs string.
- Around line 95-122: Add a JSDoc-style doc comment above the exported function
buildPrompt describing its purpose, parameters, and return value: state that
buildPrompt accepts an AnalysisModelInput (describe key fields used such as
prepBundle, structuralAssessment) and returns a single string prompt composed of
instructions, JSON example (analysisPromptExample) and a serialized payload;
include brief notes about expected format (strict JSON, no markdown fences) and
any side-effects or assumptions. Ensure the comment appears immediately above
the buildPrompt declaration and uses standard JSDoc tags like `@param`
{AnalysisModelInput} and `@returns` {string}.
- Around line 136-168: Add a JSDoc comment above the exported const
runOpenAIAnalysis describing its purpose (synthesizes dependency-upgrade prep
bundles into machine-usable implementation plans), parameters/return shape, and
any thrown errors; include an `@see` (or `@remarks`) referencing the `@openai/agents`
SDK docs and note that it uses Agent, run, analysisSynthesisSchema, and
buildPrompt for implementation details so reviewers can trace behavior to those
symbols.
- Around line 22-93: Add a doc comment above the exported constant
analysisPromptExample describing its purpose and usage: state that it is an
example instance of the analysisSynthesisSchema used for tests/docs, summarize
the key fields it contains (e.g., executiveBrief, claims,
implementationChecklist, validationChecklist, openQuestions,
recommendedNextMode), and note that it is exported for external consumption;
place the comment directly above the analysisPromptExample declaration so
linters and consumers can see its intent.
In `@src/core/results/write-result-bundle.ts`:
- Around line 21-37: Add JSDoc-style doc comments above the exported types
ResultBundleWriteInput and WrittenResultBundle describing their purpose and each
field's meaning; for ResultBundleWriteInput document that it represents the
inputs required to write a result bundle and briefly explain fields like
prepBundle, synthesis, outcomeClass, primaryAction, routingDecision, recovery,
modelUsed, generatedAt, and structuralAssessment, and for WrittenResultBundle
document that it represents the written output including resultRoot,
manifestPath, and manifest. Keep comments concise, follow existing project doc
comment style, and place them immediately above the respective type
declarations.
- Around line 80-82: Add a JSDoc comment for the exported async function
writeResultBundle describing its purpose, document the input parameter (type
ResultBundleWriteInput) with an `@param` tag, and document the return value
(Promise<WrittenResultBundle>) with an `@returns` tag; place the comment
immediately above the writeResultBundle declaration and keep the summary concise
and aligned with project comment style.
In `@src/core/runtime/analyze-run.ts`:
- Around line 22-33: Add JSDoc-style doc comments for the three exported types:
AnalyzeRunOptions, AnalyzeRunDependencies, and AnalyzeRunResult. For
AnalyzeRunOptions describe its purpose (options for running an analysis) and
document fields repoRoot (optional string path) and runId (string identifier).
For AnalyzeRunDependencies describe each injected dependency: now (optional Date
factory), loadPolicy (optional function returning a Policy), and modelExecutor
(optional AnalysisModelExecutor). For AnalyzeRunResult document that it
represents the resolved return type of writeResultBundle (the result bundle
shape). Place the comments immediately above the corresponding type declarations
(AnalyzeRunOptions, AnalyzeRunDependencies, AnalyzeRunResult) using standard /**
... */ JSDoc format.
- Around line 138-154: The fallback claim object with id 'blocked_analysis' is
missing the required package field per resultClaimSchema; update the claim (the
object with id 'blocked_analysis' in analyze-run.ts) to include a valid package
identifier (match the format used by other claims or the surrounding code, e.g.,
the same package string used for analysis/result claims) so the object conforms
to resultClaimSchema and passes validation.
- Around line 113-116: Add a JSDoc-style doc comment above the exported function
analyzePreparedRun describing its purpose (summary), and include `@param` tags for
options: AnalyzeRunOptions and dependencies?: AnalyzeRunDependencies, plus an
`@returns` tag describing the Promise<AnalyzeRunResult> result; place the comment
directly above the analyzePreparedRun declaration and ensure it follows project
doc conventions (brief summary line, blank line, then `@param/`@returns
annotations).
In `@src/core/runtime/eligibility.ts`:
- Around line 9-24: Add doc comments for the exported types StructuralFinding
and StructuralAssessment to satisfy coding guidelines: locate the type
declarations for StructuralFinding and StructuralAssessment and add concise
JSDoc-style comments describing each type's purpose and the meaning of their
fields (e.g., package, blockers, degradations for StructuralFinding;
canSynthesize, readyForImplementation, outcomeClass, primaryAction,
stopConditions, findings, fallbackClaims, topRiskSignals for
StructuralAssessment). Ensure the comments are placed immediately above each
exported type and follow the project's doc comment style/convention.
- Around line 67-70: Add a JSDoc/TSDoc comment for the exported function
evaluateStructuralEligibility describing its purpose, and include `@param` tags
for prepBundle (LoadedPrepBundle) and mode (Mode) plus an `@returns` tag
describing the StructuralAssessment result; place the comment immediately above
the evaluateStructuralEligibility declaration so it documents the exported
symbol and satisfies the project's doc-comment guideline.
- Around line 121-127: The bundle-level degradation check on
prepBundle.manifest.degradedArtifactFamilies is inside the per-package loop and
causes the same string to be appended to each package's
degradations/fallbackClaims; move the check out of the per-package loop (or add
a boolean guard) so you push the `"prep bundle degraded families: ..."` message
only once into the bundle-level findings or a single shared degradations array
instead of into each package's degradations list (adjust references to
degradations, fallbackClaims and findings[*].degradations accordingly).
In `@src/core/runtime/intake.ts`:
- Around line 28-56: Add immediately preceding JSDoc-style comments for the
exported type LoadedPrepBundle and the exported functions
loadPrepBundleFromRunId and loadPrepBundleFromManifest (no blank line between
the comment and the declaration). For each, write a brief /** ... */ summary
describing purpose and key parameters/return: describe LoadedPrepBundle as the
in-memory bundle of prep artifacts (list main fields like manifestPath,
manifest, meta, docs, releases, sourcePaths, diff, usage, signals), describe
loadPrepBundleFromRunId(repoRoot, runId) as loading a bundle by run id (mention
repoRoot and runId inputs and that it resolves manifestPath), and describe
loadPrepBundleFromManifest(manifestPath) as loading and returning a
LoadedPrepBundle from a manifest path.
- Around line 63-81: Validate and bound each manifest.artifactFiles path to the
run/prep directory before calling readJsonFile to prevent path traversal; in
src/core/runtime/intake.ts check that manifest and manifest.artifactFiles exist,
resolve each path (meta, docs, releases, sourcePaths, diff, usage, signals)
against the run/prep root and ensure the resolved path is inside that directory
(reject or normalize if not), then perform the readJsonFile calls (you can run
them in parallel) using the validated paths and the existing schema variables
(metaArtifactSchema, docsArtifactSchema, releasesArtifactSchema,
sourcePathsArtifactSchema, diffArtifactSchema, usageArtifactSchema,
signalsArtifactSchema).
- Around line 1-83: The exported items are missing required doc comments: add an
immediately preceding single-sentence JSDoc for the type LoadedPrepBundle and
for the functions loadPrepBundleFromRunId and loadPrepBundleFromManifest; for
the two functions include `@param` tags for each parameter
(loadPrepBundleFromRunId: repoRoot, runId; loadPrepBundleFromManifest:
manifestPath) and an `@returns` tag documenting the Promise<LoadedPrepBundle>
return value, ensuring the comments sit directly above the declarations.
In `@src/core/runtime/recovery.ts`:
- Around line 51-66: Add a doc comment for the exported function
buildRecoveryRecord describing its purpose and behavior, and include `@param` tags
for the input object (documenting status: RecoveryRecord['status'], trigger:
string, fromTier/toTier: RoutingDecision['selectedTier'], notes?: string[]) and
an `@returns` tag describing that it returns a RecoveryRecord with action
're_run_with_escalation'; place the comment immediately above the
buildRecoveryRecord function declaration.
- Around line 9-49: Add a doc comment for the exported function
shouldAttemptEscalationRecovery: add a brief one-line summary describing that it
decides whether to attempt automatic escalation-based recovery, then document
the input parameter (object shape: mode, routingDecision, synthesis, policy,
recoveryCount) with `@param` explaining each field's role (e.g., mode: Mode,
routingDecision.selectedTier/escalationScore, synthesis.claims/semanticOutcome,
policy.recovery.allowedActions/maxAutomaticHops/policy.modelRouting.thresholds.recoveryEscalationMin,
recoveryCount), and add an `@returns` tag describing the boolean result (true if
escalation recovery should be attempted, false otherwise); place the comment
immediately above the function declaration.
In `@src/core/runtime/routing.ts`:
- Around line 128-131: Add a TSDoc block immediately above the exported function
buildRoutingDecision describing its purpose, document the input parameter and
its fields with `@param` tags (e.g., `@param` input - object containing prepBundle:
LoadedPrepBundle and policy: Policy; then `@param` input.prepBundle -
LoadedPrepBundle used for ...; `@param` input.policy - Policy controlling ...),
and add an `@returns` tag describing the returned RoutingDecision; ensure the
comment directly precedes the function declaration and references the types
LoadedPrepBundle, Policy, and RoutingDecision.
- Around line 33-49: The majorVersionScore logic incorrectly derives majors by
splitting strings and ignores semver prefixes/ranges and the 0.x->1.x breaking
case; update majorVersionScore to robustly parse majors from
prepBundle.sourcePaths.packages using the semver library (e.g., semver.coerce or
semver.minVersion) for both entry.current?.version and entry.target?.version
(falling back to prepBundle.manifest.request.targetVersion when needed), and
change the scoring condition in majorVersionScore to treat any upgrade where
targetMajor > currentMajor — including currentMajor === 0 and targetMajor >= 1 —
as a high-risk (100) case, keeping the existing equal-major (35) and fallback
scores otherwise.
In `@src/schemas/policy.ts`:
- Around line 45-55: The superRefine validator for the policy weights uses exact
equality on totalWeight which is brittle with decimal weights; update the check
in the superRefine block that computes totalWeight (from value.weights) to use
an epsilon comparison (e.g., if (Math.abs(totalWeight - 100) > 1e-6) { ... })
instead of totalWeight !== 100, and keep the ctx.addIssue behavior the same
(adjust message if you want to note tolerance); reference the weights field,
scoringCategorySchema, totalWeight local, and the superRefine/ctx.addIssue call
when making the change.
In `@src/schemas/result.ts`:
- Around line 84-100: Extract the duplicated literal arrays into shared z.enum
schemas and replace inline literals with those constants: create and export e.g.
semanticOutcomeSchema (z.enum([...])) and routingCategorySchema (or similarly
named) and use them in decisionReportSchema and analysisSynthesisSchema (and any
routing logic that currently repeats the lists) instead of repeating the literal
arrays; update references to use the new exported symbols
(semanticOutcomeSchema, routingCategorySchema) so adding a new value only
requires changing one definition.
- Around line 1-3: Remove the file-level JSDoc that uses the forbidden
`@fileoverview` tag at the top of src/schemas/result.ts and replace it with a
plain block comment (/* ... */) or remove it entirely; do not use any disallowed
JSDoc/TSDoc tags such as `@fileoverview`—only use the allowed tags (`@remarks`,
`@param`, `@typeParam`, `@returns`, `@throws`, `@example`, `@see`, `@deprecated`) inside JSDoc
blocks attached to specific symbols if needed.
- Around line 27-34: The evidenceReferenceSchema currently allows objects with
only artifactFamily, which yields non-locatable evidence; update
evidenceReferenceSchema so it requires a locatable payload by enforcing that
either package or locator is present. Locate evidenceReferenceSchema in
src/schemas/result.ts and add a Zod-level constraint (e.g., .refine(...) or an
equivalent union) that checks at least one of the optional fields
nonEmptyStringSchema (package) or nonEmptyStringSchema (locator) is set, and
provide a clear validation message; keep the object .strict() and preserve
artifactFamilySchema and excerpt as-is.
---
Outside diff comments:
In `@src/schemas/result.ts`:
- Around line 25-297: The exported schemas and inferred type aliases
added/modified from confidenceSchema onward are missing the required leading
JSDoc comments; add an immediately preceding /** One-sentence summary. */ (no
blank line) before each exported symbol to satisfy the TS doc contract.
Specifically, add doc comments above the exports: confidenceSchema,
evidenceReferenceSchema, resultClaimSchema, implementationChecklistItemSchema,
validationChecklistItemSchema, openQuestionSchema, decisionReportSchema,
evidenceMapSchema, implementationChecklistSchema, validationChecklistSchema,
openQuestionsSchema, analysisSynthesisSchema, routingScoreFactorSchema,
routingDecisionSchema, recoveryRecordSchema, resultFilesSchema,
resultUnverifiedItemSchema, resultManifestSchema, and the exported type aliases
EvidenceReference, ResultClaim, ImplementationChecklistItem,
ValidationChecklistItem, OpenQuestion, DecisionReport, EvidenceMap,
ImplementationChecklist, ValidationChecklist, OpenQuestions, AnalysisSynthesis,
RoutingDecision, RecoveryRecord, ResultManifest; ensure each comment is a
single-sentence summary on the first non-empty line and placed immediately above
the export with no blank line in between.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 34b0fab1-e693-4ab3-8ce6-f2a1857e3e00
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
README.mdconfig/deps-workbench.config.jsoncdocs/plan/Plan-04-analysis-runtime-and-model-routing.mddocs/plan/README.mddocs/references/cli-reference.mddocs/references/repo-architecture.mdfixtures/prep/repo-basic/config/deps-workbench.config.jsoncfixtures/schemas/minimal-result-manifest.jsonpackage.jsonsrc/cli.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/core/runtime/recovery.tssrc/core/runtime/routing.tssrc/core/storage/paths.tssrc/schemas/enums.tssrc/schemas/policy.tssrc/schemas/result.tstest/analyze-run.test.tstest/cli.test.tstest/openai-analysis.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (11)
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
package.jsonfixtures/prep/repo-basic/config/deps-workbench.config.jsonctest/cli.test.tssrc/core/storage/paths.tsREADME.mddocs/plan/README.mdconfig/deps-workbench.config.jsoncsrc/schemas/policy.tstest/openai-analysis.test.tsdocs/references/cli-reference.mddocs/references/repo-architecture.mdsrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tsdocs/plan/Plan-04-analysis-runtime-and-model-routing.mdsrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tstest/analyze-run.test.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.tsfixtures/schemas/minimal-result-manifest.json
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
test/cli.test.tssrc/core/storage/paths.tssrc/schemas/policy.tstest/openai-analysis.test.tssrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tssrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tstest/analyze-run.test.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
test/cli.test.tssrc/core/storage/paths.tssrc/schemas/policy.tstest/openai-analysis.test.tssrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tssrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tstest/analyze-run.test.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
test/cli.test.tssrc/core/storage/paths.tssrc/schemas/policy.tstest/openai-analysis.test.tssrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tssrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tstest/analyze-run.test.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
test/cli.test.tssrc/core/storage/paths.tssrc/schemas/policy.tstest/openai-analysis.test.tssrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tssrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tstest/analyze-run.test.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
test/cli.test.tssrc/core/storage/paths.tssrc/schemas/policy.tstest/openai-analysis.test.tssrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tssrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tstest/analyze-run.test.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
test/cli.test.tssrc/core/storage/paths.tssrc/schemas/policy.tstest/openai-analysis.test.tssrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tssrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tstest/analyze-run.test.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/cli.test.tstest/openai-analysis.test.tstest/analyze-run.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/core/storage/paths.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/core/runtime/recovery.tssrc/commands/analyze.tssrc/schemas/enums.tssrc/core/results/write-result-bundle.tssrc/cli.tssrc/core/models/openai-analysis.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
config/deps-workbench.config.jsonc
📄 CodeRabbit inference engine (AGENTS.md)
Keep checked-in policy in
config/deps-workbench.config.jsonc
Files:
config/deps-workbench.config.jsonc
src/cli.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not blur canonical command role boundaries:
preparefor deterministic local evidence collection,analyzefor model-driven analysis,reportfor compact rendering,runfor end-to-end convenience, andresumefor inspecting and continuing prior runs
Files:
src/cli.ts
🪛 LanguageTool
docs/references/repo-architecture.md
[style] ~118-~118: This phrase is redundant. Consider writing “outcome”.
Context: ...ceeded only when the rerun upgrades the final outcome to ready_to_implement; otherwise pres...
(FINAL_END)
🔍 Remote MCP Context7, Firecrawl
Based on the research I've completed, here's the additional context gathered for reviewing this PR:
Additional Context for PR #4 Review
OpenAI Agents SDK - Schema-Typed Output Configuration
According to the OpenAI Agents SDK documentation, when outputType is provided (either as a Zod schema or JSON Schema), the SDK automatically uses structured outputs instead of plain text. If the agent has a Zod schema defined as output type, the output will automatically be parsed against this schema, making finalOutput typed as z.infer<outputType>.
The implementation in src/core/models/openai-analysis.ts follows this recommended pattern by:
- Defining
analysisSynthesisSchemaas a Zod schema - Passing it to the Agent constructor as
outputType - Validating that
result.finalOutputexists after execution
Zod is the recommended approach for JavaScript/TypeScript projects because it provides full type inference and runtime validation, detailed error messages, and more ergonomic API.
Recovery Pattern Design for AI Systems
According to AI failure design best practices, AI will make mistakes—23% of AI interactions result in unsatisfactory outputs. The difference between successful AI products and failed experiments is how gracefully they handle failures, with recovery patterns including: confidence cascades (matching AI behavior to certainty levels), graceful degradation (creating fallback levels that maintain usefulness), and learn-and-recover patterns (turning mistakes into visible improvements).
The PR's bounded escalation recovery design aligns with this by:
- Limiting automatic recovery to a single hop only (preventing unbounded retries)
- Restricting escalation to implementation mode only (preventing premature escalation in triage/research)
- Requiring escalation score to meet
recoveryEscalationMinthreshold (gating recovery based on confidence/uncertainty) - Distinguishing recovery metadata between "attempted," "succeeded," and "skipped" states (transparent about what happened)
Related PR Context
The PR builds directly on:
- PR
#2(Phase 03): Introduced the prep bundle collection andpreparecommand with fixture-backed tests - PR
#1(Phase 02): Established schema/runtime foundations with Zod validation and policy loading
The Phase 04 implementation extends these by adding the analysis phase that consumes prep bundles and produces canonical result bundles with weighted routing and bounded recovery.
Bun Test Framework Context
Bun provides a native testing framework with built-in matchers and mocking capabilities, eliminating the need for external testing libraries like Jest or Mocha.
The test files (test/analyze-run.test.ts, test/openai-analysis.test.ts) use Bun's native mocking via afterEach cleanup and dynamic module mocking (as seen in openai-analysis.test.ts mocking @openai/agents).
🔇 Additional comments (15)
src/core/storage/paths.ts (1)
114-126: Result artifact root resolution is safely constrained.Nice reuse of
resolveRunDirectory+assertPathWithinBaseto keepresult/inside.local/runs/<runId>/.test/cli.test.ts (1)
56-67: Analyze CLI parsing test coverage looks good.This cleanly verifies the new
analyzecommand shape and key flags.fixtures/schemas/minimal-result-manifest.json (1)
11-47: Fixture expansion matches result manifest contract updates.The added routing, recovery, and promotion fields are consistent with the new result schema shape.
docs/plan/README.md (1)
30-31: Documentation state and locked decision updates are consistent.These edits cleanly align the plan index with Phase 04 completion and the implementation-mode-only recovery constraint.
Also applies to: 55-56, 147-147
config/deps-workbench.config.jsonc (1)
13-26: Routing thresholds and weights look valid.The new policy block is structurally sound, thresholds are bounded, and the weights sum to 100 as required by the routing schema.
test/openai-analysis.test.ts (1)
121-155: Strong regression coverage for structured output wiring.This test correctly guards the contract that
runOpenAIAnalysisusesanalysisSynthesisSchemaasoutputTypeand returns typed synthesis.docs/references/repo-architecture.md (1)
71-73: Recovery and outcome semantics are clearly tightened.These doc updates correctly lock blocked-outcome preservation and implementation-mode-only automatic escalation behavior.
Also applies to: 100-102, 109-120
docs/plan/Plan-04-analysis-runtime-and-model-routing.md (1)
15-15: Phase completion ledger and verification updates look consistent.The task ledger, execution notes, and completion criteria now accurately reflect delivered Phase 04 runtime behavior.
Also applies to: 57-58, 112-145, 148-160, 175-185
src/commands/analyze.ts (1)
1-4: Biome CI check passed.
npx@biomejs/biomeci --error-on-warnings .executed successfully with exit code 0. Checked 53 files with no diagnostics or fixes applied.src/cli.ts (1)
1-1: LGTM!The
analyzecommand integration follows the established CLI patterns:
parseAnalyzeArgsmirrorsparsePrepareArgsstructure with proper option validation- Required
--run-idenforcement is correctly implemented- Error handling in
runClimatches the existingpreparepattern- Command role boundary is respected:
analyzehandles model-driven analysis over prep bundlesAlso applies to: 197-259, 283-285, 347-362
src/schemas/enums.ts (1)
11-27: LGTM!The new enum schemas and inferred types are well-structured:
changeIntentSchema,claimBucketSchema, andrecoveryStatusSchemafollow the established pattern- Doc comments properly describe each enum's purpose
- Type exports via
z.inferprovide type-safe access to the literal unionsAlso applies to: 108-113
src/core/results/write-result-bundle.ts (1)
39-49: Clarify the sort order fortopUnverifiedItems.The function sorts UNVERIFIED claims by descending confidence (highest first). This seems counterintuitive for "top unverified items"—operators might expect the most uncertain claims (lowest confidence) to be surfaced first since those represent the highest risk.
If the current behavior is intentional, consider adding a comment explaining why higher-confidence unverified claims are prioritized.
test/analyze-run.test.ts (1)
1-837: LGTM!Comprehensive test coverage for the analyze runtime:
- Tier selection tests verify routing logic for low-risk (nano) and high-risk (full) scenarios
analyzePreparedRuntests cover blocked synthesis, bounded escalation recovery (attempted→succeeded/skipped), and no-escalation cases- Schema validation tests ensure all result artifacts parse correctly
- Proper test isolation via temp directories with cleanup in
finallyblocks- Fixture-backed approach aligns with coding guidelines
src/core/runtime/analyze-run.ts (1)
47-56: > Likely an incorrect or invalid review comment.src/core/models/openai-analysis.ts (1)
143-155: No changes needed. TheAgentconfiguration correctly uses the@openai/agentsSDK API. ThemodelSettings.reasoning.effortandmodelSettings.text.verbosityproperties are documented and supported;mapReasoningEffort()returns valid effort levels ('medium'or'low'), andtext.verbosity: 'low'is a valid verbosity setting.
There was a problem hiding this comment.
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 `@src/core/runtime/recovery.ts`:
- Around line 53-56: Extract the hardcoded threshold `unverifiedCount >= 2` into
a named, configurable parameter (e.g., UNVERIFIED_CLAIMS_THRESHOLD) or add it to
the policy schema next to `recoveryEscalationMin`, and replace the literal with
a reference to that parameter in the condition that also checks
`input.synthesis.semanticOutcome === 'review_required'`; alternatively, if
making it configurable is undesirable, add a concise inline comment next to
`unverifiedCount >= 2` explaining the policy rationale for the value. Ensure you
update or document the default value where the new parameter is declared (and
reference `unverifiedCount` and `recoveryEscalationMin` in the schema or comment
so reviewers can find related policy settings).
In `@test/analyze-run.test.ts`:
- Around line 869-893: The test uses a null-coalescing fallback
(openQuestionsPath ?? '') when reading the file, which could produce an invalid
path; after asserting expect(openQuestionsPath).toBeDefined(), replace the
fallback by explicitly narrowing the value (e.g., assert or non-null assertion)
and then call Bun.file(openQuestionsPath!) to read/parse the file — update both
occurrences around openQuestionsSchema.parse(...) and Bun.file(...).exists() to
use the narrowed/non-null form so TypeScript and runtime both treat
openQuestionsPath as defined.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 3ad996bf-e2dc-4b8f-9965-6acaf82b0486
📒 Files selected for processing (3)
src/core/results/write-result-bundle.tssrc/core/runtime/recovery.tstest/analyze-run.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.tstest/analyze-run.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.tstest/analyze-run.test.ts
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
src/core/runtime/recovery.tssrc/core/results/write-result-bundle.tstest/analyze-run.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/analyze-run.test.ts
🔍 Remote MCP Firecrawl
Summary of additional, review-relevant findings
-
The OpenAI Agents SDK supports schema-typed outputs via an outputType (Zod or JSON schema) and run(agent, prompt) returns a structured finalOutput when configured; using Zod as outputType is a recommended pattern (docs/lesson describing outputType + run usage).
-
There is a known TypeScript/Zod compatibility issue with the OpenAI Agents JS SDK: certain Zod versions (>=3.25.68) produced type errors when passed directly as Agent outputType; the project recommended locking Zod to <=3.25.67 as a workaround and published fixes/PRs addressing this. This is relevant because the PR adds
@openai/agents^0.8.3 and the code passes Zod schemas as outputType (see src/core/models/openai-analysis.ts). -
Users have reported runtime ModelBehaviorError / "Invalid output type" when the model yields unstructured responses despite an outputType; community posts note both: (a) occasional model outputs not matching strict schema (GPT4.1 behavior), and (b) the SDK has historically thrown errors that were hard to catch (some fixes merged). This suggests the PR’s runtime should (and tests partly do) defend against both schema-validation failures and intermittent non-conforming model outputs (e.g., catching ModelBehaviorError and recording recovery/skip).
Actionable implications for the review
- Confirm the repo’s Zod dependency (or transitive Zod version) is compatible with
@openai/agentsusage. If the project does not pin Zod, consider adding a resolution/lock to ensure Zod <=3.25.67 (or verify the agents SDK version used includes the upstream fixes). - Ensure analyze runtime robustly handles ModelBehaviorError / invalid output parsing paths (the PR’s tests already mock and assert behavior—verify error handling paths around the Agent.run call and that they record recovery/skipped statuses accordingly).
- Optional: run a quick dependency audit in CI (or locally) to detect Zod version and ensure tests run with the same Zod that the SDK expects.
Sources
- OpenAI Agents SDK — Zod outputType usage / run semantics (tutorial-like lesson).
- GitHub issue: Agent outputType type error with zod@3.25.68+ (workaround: lock zod <=3.25.67; upstream fixes referenced).
- OpenAI Developer Community thread: ModelBehaviorError / invalid output type (reports of intermittent non-JSON outputs and SDK error handling).
🔇 Additional comments (13)
src/core/runtime/recovery.ts (2)
9-57: Missing doc comment on exported function.
shouldAttemptEscalationRecoveryis an exported function and requires a doc comment with summary and@param/@returnstags per coding guidelines.
59-74: Missing doc comment on exported function.
buildRecoveryRecordis an exported function and requires a doc comment with summary and@param/@returnstags per coding guidelines.src/core/results/write-result-bundle.ts (4)
21-31: Missing doc comment on exported type.
ResultBundleWriteInputis an exported type and requires a doc comment per coding guidelines.
33-37: Missing doc comment on exported type.
WrittenResultBundleis an exported type and requires a doc comment per coding guidelines.
80-82: Missing doc comment on exported function.
writeResultBundleis an exported async function and requires a doc comment with summary and@param/@returnstags per coding guidelines.
177-202: LGTM!Good use of
Promise.allfor concurrent file writes, and the conditional logic foropen_questions.json(write if present, remove with{ force: true }if absent) correctly handles both initial runs and reruns that may need to clean up stale artifacts.test/analyze-run.test.ts (7)
51-57: LGTM!Good use of temp directory with proper cleanup pattern. Using
cpwith{ recursive: true }to copy fixtures is appropriate.
340-350: LGTM!Good use of
Promise.allfor concurrent artifact writes, matching the pattern used in production code. The schema validation via.parse()ensures fixture data is always valid.
484-522: LGTM!Good test coverage for structural blocking precedence. The test correctly verifies that blocking happens before the model executor is called (
modelCalls === 0) and that thesemanticOutcomein the decision report matches the blocked state.
524-612: LGTM!Thorough test for bounded escalation recovery. Correctly verifies:
- Two model executor calls (initial + escalation)
- Recovery record transitions (
attempted→succeeded)- Final routing decision reflects escalated tier
- Outcome class transitions to
ready_to_implement
693-744: LGTM!Good test for the escalation-skipped path. Correctly verifies that when escalation doesn't resolve uncertainty, the recovery status is
skippedand includes a descriptive note about the final outcome.
746-799: LGTM!Good parametrized test pattern using
for...ofto cover bothblockedanddegraded_reference_onlysemantic outcomes. This ensures the recovery guard conditions inshouldAttemptEscalationRecoveryare properly tested.
897-905: LGTM!Good regression test to ensure the analysis prompt example stays schema-valid. This catches any schema drift that would break the model's expected output format.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57c7a96f41
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/schemas/result.ts (1)
23-303:⚠️ Potential issue | 🟠 MajorThis file still violates the exported-declaration doc contract.
Nearly every exported schema/type here is missing the required immediately preceding
/** ... */comment -- e.g.confidenceSchema,evidenceReferenceSchema,resultClaimSchema, the bundle schemas, and the inferred exported types at the bottom. The plain file header does not satisfy that rule, so this should still hard-fail the TS doc-comment check.As per coding guidelines "Every changed or added exported declaration must have an immediately preceding doc comment
/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/result.ts` around lines 23 - 303, Several exported declarations are missing the required preceding doc-comments; add an immediately preceding /** ... */ comment (with no blank line) for each exported schema and exported type such as confidenceSchema, evidenceReferenceSchema, resultClaimSchema, implementationChecklistItemSchema, validationChecklistItemSchema, openQuestionSchema, decisionReportSchema, evidenceMapSchema, implementationChecklistSchema, validationChecklistSchema, openQuestionsSchema, analysisSynthesisSchema, routingScoreFactorSchema, routingDecisionSchema, recoveryRecordSchema, resultFilesSchema, resultUnverifiedItemSchema, resultManifestSchema and the exported inferred types (EvidenceReference, ResultClaim, ImplementationChecklistItem, ValidationChecklistItem, OpenQuestion, DecisionReport, EvidenceMap, ImplementationChecklist, ValidationChecklist, OpenQuestions, AnalysisSynthesis, RoutingDecision, RecoveryRecord, ResultManifest); ensure each comment is directly above the export with no blank line and contains a short description per the doc-comment contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/models/openai-analysis.ts`:
- Around line 150-155: The exported function that "Executes OpenAI Agents
synthesis against the prepared bundle and routing decision" is missing `@throws`
JSDoc entries for the exceptions it actually throws; add `@throws` blocks directly
beneath the existing description documenting each distinct throw site (the
errors thrown around the initial input validation and the later runtime/response
failure), naming the exception type (e.g., Error), a brief description of the
condition that causes it, and any relevant message/fields returned so callers
know what to catch; update the function's JSDoc in
src/core/models/openai-analysis.ts to include one `@throws` per exception type
thrown by that function (matching the throws at the input-validation and
runtime/response failure locations).
- Around line 175-180: Wrap the call to run(agent, buildPrompt(input)) in a
try/catch that specifically handles the SDK's ModelBehaviorError (in addition to
a generic catch); when catching ModelBehaviorError from run or
agent.buildPrompt, extract and surface the structured parse failure details
(error.message / error.details) and rethrow or return a clearer Error that
includes those details so upstream recovery can act on schema-parse failures;
reference the run, agent, buildPrompt symbols and ensure the existing undefined
finalOutput check remains after the try/catch.
In `@src/core/runtime/analyze-run.ts`:
- Around line 171-179: The writeResultBundle call in this pre-model blocked
branch should not record routingDecision.selectedModel as modelUsed because no
modelExecutor ran; change this branch to either omit modelUsed (set modelUsed:
undefined) or add a new field like plannedModel: routingDecision.selectedModel
and leave modelUsed undefined, and update the writeResultBundle signature/type
and any consumers (audit/cost reporting) to accept plannedModel or optional
modelUsed so blocked runs are not treated as real model executions; locate the
change inside the blocked branch where writeResultBundle is called and adjust
callers/types accordingly (function: writeResultBundle, symbol:
routingDecision.selectedModel, field: modelUsed / plannedModel).
In `@src/core/runtime/eligibility.ts`:
- Around line 85-91: The code currently assigns bundle-scoped
degradedArtifactFamilies to prepBundle.meta.packages[0] via
bundleDegradationMessage and bundleDegradationPackage, which incorrectly
attributes bundle-wide degradation to the first package; instead, create a
bundle-level degradation evidence (e.g., bundleDegradationMessage variable kept
at module/function scope) that is emitted once as bundle-level state/fallback
rather than attached to any specific package, and when building per-package
evidence (iterate over prepBundle.meta.packages) only attach package-specific
degradation info—not bundleDegradationMessage—so remove usage of
bundleDegradationPackage/prepBundle.meta.packages[0] and ensure
degradedArtifactFamilies from prepBundle.manifest are reported separately at the
bundle level (also update the identical logic currently duplicated around the
other block that references bundleDegradationMessage/bundleDegradationPackage).
In `@src/core/runtime/intake.ts`:
- Around line 87-125: The seven independent reads (calls to readPrepArtifact for
manifest.artifactFiles.meta/docs/releases/sourcePaths/diff/usage/signals) are
being awaited inline causing serialized I/O; change intake to start all seven
promises first (e.g., create variables like metaP, docsP, releasesP,
sourcePathsP, diffP, usageP, signalsP by calling readPrepArtifact without
awaiting), then await them together with Promise.all (or Promise.allSettled if
you prefer) and use the resolved values to construct the returned object so the
reads run concurrently.
- Around line 80-85: The manifest is treated as untrusted but
loadPrepBundleFromManifest recomputes runDirectory from
manifest.repoRoot/manifest.runId, allowing a tampered manifest to redirect to
attacker-controlled files; fix by using the already-resolved canonical
manifestPath/runDirectory from the caller (the path computed in
loadPrepBundleFromRunId) instead of recomputing from manifest fields—i.e., after
readJsonFile(manifestPath, ...) ignore manifest.repoRoot and manifest.runId for
path resolution and validate artifactRoot with the known canonical runDirectory
before calling assertPathWithinBase (update loadPrepBundleFromManifest and any
use of runDirectory/manifestPath accordingly).
In `@src/schemas/policy.ts`:
- Around line 10-18: Export the existing scoringCategorySchema (the z.enum
defined as scoringCategorySchema) so it can be imported elsewhere, and update
the RoutingCategory type to be derived from it using z.infer<typeof
scoringCategorySchema> instead of a separately maintained literal union;
specifically, export scoringCategorySchema from its module and change the
RoutingCategory typedef to import scoringCategorySchema and set RoutingCategory
= z.infer<typeof scoringCategorySchema> to ensure both stay aligned.
---
Outside diff comments:
In `@src/schemas/result.ts`:
- Around line 23-303: Several exported declarations are missing the required
preceding doc-comments; add an immediately preceding /** ... */ comment (with no
blank line) for each exported schema and exported type such as confidenceSchema,
evidenceReferenceSchema, resultClaimSchema, implementationChecklistItemSchema,
validationChecklistItemSchema, openQuestionSchema, decisionReportSchema,
evidenceMapSchema, implementationChecklistSchema, validationChecklistSchema,
openQuestionsSchema, analysisSynthesisSchema, routingScoreFactorSchema,
routingDecisionSchema, recoveryRecordSchema, resultFilesSchema,
resultUnverifiedItemSchema, resultManifestSchema and the exported inferred types
(EvidenceReference, ResultClaim, ImplementationChecklistItem,
ValidationChecklistItem, OpenQuestion, DecisionReport, EvidenceMap,
ImplementationChecklist, ValidationChecklist, OpenQuestions, AnalysisSynthesis,
RoutingDecision, RecoveryRecord, ResultManifest); ensure each comment is
directly above the export with no blank line and contains a short description
per the doc-comment contract.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 423e1f60-867b-45bd-b58e-895777690498
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
README.mdpackage.jsonsrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/core/runtime/recovery.tssrc/core/runtime/routing.tssrc/schemas/policy.tssrc/schemas/result.tstest/analyze-run.test.tstest/schemas.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
package.jsontest/schemas.test.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tsREADME.mdsrc/core/runtime/eligibility.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
test/schemas.test.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tssrc/core/runtime/eligibility.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
test/schemas.test.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tssrc/core/runtime/eligibility.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
test/schemas.test.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tssrc/core/runtime/eligibility.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
test/schemas.test.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tssrc/core/runtime/eligibility.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
test/schemas.test.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tssrc/core/runtime/eligibility.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
test/schemas.test.tssrc/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tssrc/core/runtime/eligibility.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/schemas.test.tstest/analyze-run.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/schemas/policy.tssrc/core/runtime/routing.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/recovery.tssrc/core/runtime/intake.tssrc/core/runtime/eligibility.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
🔍 Remote MCP Firecrawl
Summary of additional review-relevant facts
-
OpenAI Agents SDK: outputType accepts Zod schemas and run(...) / run(agent, prompt) returns a RunResult whose finalOutput is parsed to the Zod type when outputType is a Zod schema (SDK docs describe finalOutput behavior and recommend Zod for JS/TS) — relevant to src/core/models/openai-analysis.ts using Zod outputType.
-
Known Zod compatibility bug: openai-agents-js had a TypeScript error when consumers passed Zod >= 3.25.68 as outputType; the project advised locking Zod to <= 3.25.67 as a workaround and merged fixes (issue
#187, related PRs/commits). This directly affects the PR because it adds@openai/agentsand passes Zod schemas as outputType. Consider verifying/resolving Zod version in the repo (pin or resolution) or updating agents SDK version that includes the fix. -
ModelBehaviorError / invalid output type is a real, reported runtime issue: community reports that models (GPT-4.1 etc.) sometimes produce outputs that don't match strict schema and that the Agents SDK historically surfaced ModelBehaviorError; the SDK has had fixes to ensure those exceptions can be handled. The PR’s tests and runtime should therefore robustly handle ModelBehaviorError / schema-parse failures (catch, record recovery/skipped status) — the community thread and GitHub issues document both the runtime occurrences and SDK fixes.
Sources (tool calls)
- Firecrawl search results for OpenAI Agents SDK guides and results (Zod outputType, finalOutput semantics) — Firecrawl_firecrawl_search.
- Firecrawl search results for openai-agents-js GitHub issue about Zod 3.25.68 TypeScript error and related fixes/workarounds — Firecrawl_firecrawl_search.
- Firecrawl search results for community reports of ModelBehaviorError / Invalid output type and SDK exception handling discussion — Firecrawl_firecrawl_search.
🔇 Additional comments (11)
README.md (1)
76-77: LGTM!The quick start example now uses a concrete sample value (
run_123) for the--run-idflag, addressing the previous shell-redirection concern.src/schemas/policy.ts (1)
48-61: LGTM!The epsilon-based weight-sum validation (
Math.abs(totalWeight - 100) > 1e-6) correctly handles floating-point precision for decimal weights, addressing the previous review concern.test/schemas.test.ts (1)
53-59: LGTM!The test correctly validates that
evidenceReferenceSchemarejects incomplete evidence references lacking a locator, ensuring schema integrity for the evidence/claim modeling.src/commands/analyze.ts (1)
1-60: LGTM!The module correctly implements the
analyzeCLI command with proper doc comments for all exported declarations. Error handling is appropriately delegated to the CLI dispatcher insrc/cli.ts.src/core/runtime/routing.ts (3)
35-41: LGTM!The
extractMajorfunction correctly usessemver.coerceandsemver.minVersionto handle version strings with prefixes (^,~,v) and ranges, addressing the previous parsing fragility.
50-62: LGTM!The major-version scoring now correctly treats all
target > currentcases as high-risk (100), including0.x → 1.xupgrades, per the previous review feedback.
141-146: LGTM!The exported
buildRoutingDecisionfunction now has the required doc comment with@paramand@returnstags.src/core/results/write-result-bundle.ts (1)
1-215: LGTM!The result bundle writer is well-structured with proper doc comments, concurrent artifact writes via
Promise.all, atomic manifest persistence, and correct cleanup of staleopen_questions.jsonwhen not needed.package.json (1)
33-36: No compatibility issue—@openai/agents 0.8.3 supports Zod 4 for outputType. The dependency versions in the PR are compatible as specified.src/core/models/openai-analysis.ts (1)
167-172: BothmodelSettings.reasoning.effortandmodelSettings.text.verbosityare valid, officially documented options in@openai/agents0.8.3. No changes needed; this concern can be resolved.src/core/runtime/analyze-run.ts (1)
185-249: Add try-catch around bothmodelExecutorcalls to persist recovery records and fallback synthesis.Both executor calls on lines 185 and 219 lack error handling. If
runOpenAIAnalysisthrows aModelBehaviorErrorduring Zod schema validation, the function exits before writing the result bundle. When the second executor throws (line 219), therecoveryarray is already marked withstatus: 'attempted'(line 214) but never advances to'succeeded'or'skipped', breaking the bounded-recovery contract and leaving no audit trail. Wrap eachawait modelExecutor(...)in try-catch; on error, persist a recovery record with status'failed'and synthesize a blocked-outcome fallback before callingwriteResultBundle.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/schemas/policy.ts (1)
1-76:⚠️ Potential issue | 🔴 CriticalAdd required doc comments to exported declarations.
The exported
policySchema(line 24) andPolicy(line 76) lack the required preceding doc comment/** ... */. Each exported declaration must include a doc comment with a single-sentence summary describing behavior and intent, ending with a period.For
policySchema, document what this Zod schema validates (the policy configuration structure and constraints). ForPolicy, document that it is the inferred TypeScript type from the schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/policy.ts` around lines 1 - 76, Add a single-sentence JSDoc comment (/** ... */) immediately above the exported policySchema describing that this Zod schema validates the policy configuration structure and its constraints (modes, modelRouting with tiers/thresholds/weights, recovery settings, and framework enrichments). Also add a single-sentence JSDoc comment above the exported Policy type stating that it is the TypeScript type inferred from policySchema. Ensure each comment is one sentence, ends with a period, and precedes the respective exported declaration (policySchema and Policy).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/runtime/analyze-run.ts`:
- Around line 147-187: The blocked branch writes a false `modelUsed` even though
no model ran (structuralAssessment.canSynthesize is false) — update the manifest
and callsite so blocked runs omit modelUsed: make
ResultBundleWriteInput.modelUsed optional (string | undefined) and change
resultManifestSchema.modelUsed to optional, then remove or pass undefined for
modelUsed in writeResultBundle when inside the blocked branch (refer to
structuralAssessment, writeResultBundle, routingDecision.selectedModel,
ResultBundleWriteInput.modelUsed and resultManifestSchema.modelUsed to locate
the code to change).
In `@src/schemas/policy.ts`:
- Around line 42-65: Add a cross-field guard in the existing .superRefine to
validate thresholds ordering (value.thresholds.triageNanoMax,
value.thresholds.fullEscalationMin, value.thresholds.recoveryEscalationMin) so
triageNanoMax < fullEscalationMin < recoveryEscalationMin (or <= as your policy
requires); if the ordering is violated, call ctx.addIssue with appropriate paths
(e.g. ['thresholds','triageNanoMax'] or ['thresholds','fullEscalationMin']) and
a clear message so the schema refuses policies that would collapse tier bands —
implement this check inside the same superRefine where weights are validated.
---
Outside diff comments:
In `@src/schemas/policy.ts`:
- Around line 1-76: Add a single-sentence JSDoc comment (/** ... */) immediately
above the exported policySchema describing that this Zod schema validates the
policy configuration structure and its constraints (modes, modelRouting with
tiers/thresholds/weights, recovery settings, and framework enrichments). Also
add a single-sentence JSDoc comment above the exported Policy type stating that
it is the TypeScript type inferred from policySchema. Ensure each comment is one
sentence, ends with a period, and precedes the respective exported declaration
(policySchema and Policy).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: b0233918-c694-4a12-b8f0-cfa8bdb65d64
📒 Files selected for processing (3)
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
src/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/schemas/policy.ts
🔍 Remote MCP Firecrawl
Summary of additional review-relevant facts
-
OpenAI Agents SDK: finalOutput is automatically parsed to z.infer when an Agent is created with a Zod schema as outputType; the SDK doc explains finalOutput shapes and recommends Zod for structured outputs.
-
Known Zod / TypeScript incompatibility (affects projects that add
@openai/agents+ Zod):- Zod v3.25.68+ introduced private/changed members that caused TS errors across package boundaries; OpenAI/Agents maintainers and Zod discussion show this broke consumer builds. Recommended mitigations used by the SDK and community: pin Zod to <= 3.25.67 or ensure a single deduped Zod instance (peerDependency/overrides/resolutions). See the Zod issue and the OpenAI Agents issue and related fixes/PRs.
-
Runtime model/schema mismatches (ModelBehaviorError) are real and discussed by users:
- The Agents SDK exposes ModelBehaviorError for unexpected model behavior; community threads report models (e.g., GPT-4.1 variants) sometimes return unstructured output causing ModelBehaviorError, and the SDK had fixes to ensure such exceptions can be handled. Recommend the PR’s runtime/tests explicitly handle agent schema-parse failures and ModelBehaviorError paths.
Cited searches/tools used
- Firecrawl firecrawl_search (OpenAI Agents docs, GitHub issues, community threads) — results above.
🔇 Additional comments (17)
src/core/results/write-result-bundle.ts (7)
1-19: LGTM!Imports are well-organized with clear separation between Node.js built-ins, internal schemas, and local utilities.
21-39: LGTM!Exported types have proper doc comments per guidelines. Type definitions are clear and well-structured.
41-51: LGTM!The
topUnverifiedItemshelper correctly filters, sorts by confidence descending, limits to 5, and maps to the required shape matchingresultUnverifiedItemSchema.
53-81: LGTM!Clean Markdown rendering with proper conditional recovery line inclusion.
83-96: LGTM!Doc comment follows the required format with
@paramand@returns. Directory creation withrecursive: truehandles nested paths correctly.
192-214: Independent writes are correctly parallelized.Good use of
Promise.allfor concurrent artifact writes. The conditional cleanup viarm(openQuestionsPath, { force: true })correctly handles staleopen_questions.jsonfrom previous runs when the current synthesis has no open questions.
165-189: LGTM!The manifest construction correctly:
- Falls back to
structuralAssessment.topRiskSignalswhen synthesis signals are empty (lines 177-180)- Uses schema parsing to validate the full manifest shape before writing
src/core/runtime/analyze-run.ts (10)
1-20: LGTM!Imports are well-organized and all necessary dependencies are imported.
22-36: LGTM!Exported types have proper doc comments per guidelines. Using
Awaited<ReturnType<...>>forAnalyzeRunResultensures type consistency withwriteResultBundle.
38-62: LGTM!The
blockedSynthesishelper correctly constructs a schema-valid blocked synthesis with a minimal validation checklist item guiding the user to refresh evidence.
64-77: LGTM!Clean mapping from
outcomeClasstoprimaryActionwith clear conditional logic.
79-104: LGTM!The outcome finalization correctly prioritizes structural blocking, then synthesis-level blocking, then ready/degraded states, with a safe
review_requiredfallback.
106-114: LGTM!Simple and correct check for whether escalation moved the outcome to
ready_to_implement.
116-145: LGTM!Doc comment follows required format. Dependency injection pattern with sensible defaults enables testability. The
blockedPackageNamefallback chain (findings[0]?.package→request.packages[0]→'analysis') ensures a valid package identifier is always available.
189-195: LGTM!Initial synthesis execution with the active routing decision. The
awaitensures proper error propagation.
196-255: LGTM!Escalation recovery logic is well-structured:
- Guards via
shouldAttemptEscalationRecoveryensure bounded, policy-compliant recovery- Two recovery records are pushed:
attemptedbefore re-run, thensucceeded/skippedafter- The
activeRoutingDecisionis updated to reflect the escalated tier for final manifest persistence- Outcome comparison correctly determines if escalation resolved uncertainty
257-275: LGTM!Final outcome computation and result bundle write for the non-blocked path correctly uses
synthesisResult.modelUsed(the actual model that ran) rather than the routing plan.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/references/repo-architecture.md`:
- Around line 110-113: Update the docs to explicitly document the parse-failure
behavior for schema-typed agent output: state that when the Agents SDK fails to
produce structured output (e.g., finalOutput is undefined) synthesis is blocked,
the system should map this to outcome "blocked" with action "stop_blocked", and
the runtime must perform finalOutput guard checks before proceeding; reference
the same guard-check pattern used in the OpenAI analysis implementation (the
module that performs finalOutput validation) as the example to follow.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 48c58844-562e-4363-9892-d67ea8636baf
📒 Files selected for processing (2)
docs/references/cli-reference.mddocs/references/repo-architecture.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
docs/references/cli-reference.mddocs/references/repo-architecture.md
🪛 LanguageTool
docs/references/repo-architecture.md
[style] ~128-~128: This phrase is redundant. Consider writing “outcome”.
Context: ...ceeded only when the rerun upgrades the final outcome to ready_to_implement; otherwise pres...
(FINAL_END)
🔍 Remote MCP Context7, Firecrawl
Summary of additional review-relevant facts
-
OpenAI Agents SDK: when an Agent is created with a Zod schema as outputType, the agent run will parse and expose the structured result on result.finalOutput as the inferred Zod type (z.infer) — i.e., the SDK supports schema-typed finalOutput when you supply a Zod schema.,
-
Practical/typing notes from community and SDK sources:
- Consumers have reported TypeScript errors when different Zod copies/versions are present (starting v3.25.68) because Zod introduced private/changed members that broke structural assignability across package boundaries; the OpenAI Agents SDK and community recommended mitigations include pinning Zod to <=3.25.67 or ensuring a single deduped Zod instance (peerDependency/re-export/overrides). The Agents SDK maintainers applied temporary mitigations (lock or peer-dep) in upstream repos. This is a known, active compatibility risk for projects that depend on
@openai/agents+ Zod.
- Consumers have reported TypeScript errors when different Zod copies/versions are present (starting v3.25.68) because Zod introduced private/changed members that broke structural assignability across package boundaries; the OpenAI Agents SDK and community recommended mitigations include pinning Zod to <=3.25.67 or ensuring a single deduped Zod instance (peerDependency/re-export/overrides). The Agents SDK maintainers applied temporary mitigations (lock or peer-dep) in upstream repos. This is a known, active compatibility risk for projects that depend on
-
Runtime failure mode to consider in review/tests: the Agents SDK (and community threads) note Model/Agent schema parse failures (ModelBehaviorError / finalOutput undefined / parse errors) occur in practice when models return unstructured output; tests and runtime code should explicitly handle agent run parse failures and ModelBehaviorError paths rather than assuming successful structured output. The PR’s tests mock the agent returning finalOutput, but reviewers should verify real-agent failure handling is exercised or gated.,
Sources
- OpenAI Agents docs / guides (structured outputs, finalOutput behavior) — Context7 query of openai-agents-js docs.
- OpenAI Community / CDN typings / GitHub issues demonstrating finalOutput typing and Zod/Agents interactions (community thread, CDN d.ts, openai-agents-js issues) — Firecrawl search results.
- Zod GitHub issues and downstream discussion showing zod 3.25.68+ incompatibility and recommended mitigations (pin, peerDependencies, re-export) — Firecrawl search results.
Tag
🔇 Additional comments (4)
docs/references/repo-architecture.md (2)
71-81: Good contract tightening for blocked semantics and artifact safety.Line 71 through Line 81 are consistent with the runtime/result-writer behavior shown in
src/core/runtime/analyze-run.ts,src/core/runtime/intake.ts, andsrc/core/results/write-result-bundle.ts.
107-109: Routing and bounded recovery rules are clearly and correctly constrained.The semantics at Line 107 through Line 109 and Line 119 through Line 130 match the conservative routing and escalation guards in
src/core/runtime/routing.tsandsrc/core/runtime/recovery.ts.Also applies to: 119-130
docs/references/cli-reference.md (2)
7-7: Status section update is clear and accurate.The command status wording at Line 7 and Line 20 through Line 24 is consistent with the PR objective and current shipped surface.
Also applies to: 20-24
25-45: Analyze behavior section is well-scoped and implementation-aligned.Line 25 through Line 45 correctly captures intake boundaries, routing, recovery gating, model recording semantics, and structured output consumption.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ded239459
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/schemas/policy.ts (2)
100-100:⚠️ Potential issue | 🟡 MinorMissing doc comment on exported
Policytype.The exported
Policytype alias lacks the required doc comment.📝 Proposed fix
+/** Inferred policy configuration type from the policy schema. */ export type Policy = z.infer<typeof policySchema>;As per coding guidelines, "Every changed or added exported declaration must have an immediately preceding doc comment."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/policy.ts` at line 100, The exported type alias Policy lacks the required doc comment; add an immediately preceding JSDoc-style comment describing what Policy represents and referencing that it is inferred from policySchema (e.g., "Policy represents the validated shape produced by policySchema"). Place this doc comment directly above the export statement for Policy so the exported declaration has the required comment and mentions policySchema for clarity.
24-98:⚠️ Potential issue | 🟡 MinorMissing doc comment on exported
policySchema.The exported
policySchemadeclaration lacks an immediately preceding doc comment as required by coding guidelines for exported declarations.📝 Proposed fix
+/** + * Zod schema for the deps-workbench policy configuration. + */ export const policySchema = z .object({As per coding guidelines, "Every changed or added exported declaration must have an immediately preceding doc comment
/** ... */with no blank line separating it."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/policy.ts` around lines 24 - 98, Add a preceding doc comment immediately above the exported symbol policySchema (no blank line) describing what the schema validates (e.g., "Policy configuration schema for modes, modelRouting, recovery, and frameworkEnrichments") so the exported declaration has a /** ... */ doc comment right before export const policySchema; ensure the comment is concise and follows the coding guideline format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/runtime/analyze-run.ts`:
- Around line 116-126: The JSDoc for analyzePreparedRun is missing the required
AI-SDK reference: update the comment block above the analyzePreparedRun function
(which imports ../models/openai-analysis) to include either an `@see` tag pointing
to the ADR/SPEC path or official OpenAI/agents docs URL, or add an `@remarks` tag
containing an ADR- or SPEC- identifier per guidelines; ensure the new tag is
placed in the existing JSDoc for analyzePreparedRun so the file importing
`@openai/agents` complies with the doc comment rule.
In `@src/core/runtime/intake.ts`:
- Around line 68-76: The JSDoc for the exported function
loadPrepBundleFromManifest documents manifestPath but omits the runDirectory
param; update the comment block above loadPrepBundleFromManifest to add a line
in the format "@param runDirectory - <description>" (e.g., "Path to the run
directory used to validate run-bound paths") so every parameter is documented
per guidelines.
In `@src/schemas/result.ts`:
- Around line 276-294: Add missing JSDoc comments immediately above each
exported type alias so every exported declaration has a preceding /** ... */
with no blank line; update EvidenceReference, ResultClaim,
ImplementationChecklistItem, ValidationChecklistItem, OpenQuestion,
DecisionReport, EvidenceMap, ImplementationChecklist, ValidationChecklist,
OpenQuestions, AnalysisSynthesis, RoutingDecision, and RecoveryRecord with
one-line descriptions describing what each alias represents (e.g., "Represents a
reference to evidence", "Represents a claim in a result", etc.), ensuring the
comment directly precedes the export and follows the project's doc-comment
style.
- Around line 52-205: Add missing JSDoc comments immediately above each exported
schema from resultClaimSchema through recoveryRecordSchema (resultClaimSchema,
implementationChecklistItemSchema, validationChecklistItemSchema,
openQuestionSchema, decisionReportSchema, evidenceMapSchema,
implementationChecklistSchema, validationChecklistSchema, openQuestionsSchema,
analysisSynthesisSchema, routingScoreFactorSchema, routingDecisionSchema,
recoveryRecordSchema). For each, insert a concise /** ... */ doc comment (one or
two sentences) describing the schema's role in the result bundle structure, with
no blank line between the comment and the export, and keep comments brief and
specific to the schema's purpose.
- Around line 23-33: Add missing JSDoc-style comments immediately above each
exported schema constant (confidenceSchema, semanticOutcomeSchema,
routingCategorySchema) with no blank line between the comment and the export;
describe purpose/constraints concisely (e.g., confidence range for
confidenceSchema; alias/semantic meaning for semanticOutcomeSchema;
list/description of enum categories for routingCategorySchema) so each exported
declaration has an immediately preceding /** ... */ doc comment per guidelines.
---
Outside diff comments:
In `@src/schemas/policy.ts`:
- Line 100: The exported type alias Policy lacks the required doc comment; add
an immediately preceding JSDoc-style comment describing what Policy represents
and referencing that it is inferred from policySchema (e.g., "Policy represents
the validated shape produced by policySchema"). Place this doc comment directly
above the export statement for Policy so the exported declaration has the
required comment and mentions policySchema for clarity.
- Around line 24-98: Add a preceding doc comment immediately above the exported
symbol policySchema (no blank line) describing what the schema validates (e.g.,
"Policy configuration schema for modes, modelRouting, recovery, and
frameworkEnrichments") so the exported declaration has a /** ... */ doc comment
right before export const policySchema; ensure the comment is concise and
follows the coding guideline format.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 93cb57b5-2658-4f40-9ffe-c0b47b6a0b80
📒 Files selected for processing (11)
docs/references/repo-architecture.mdsrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/core/runtime/analyze-run.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/policy.tssrc/schemas/result.tstest/analyze-run.test.tstest/schemas.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
test/schemas.test.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
test/schemas.test.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.ts
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
test/schemas.test.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tsdocs/references/repo-architecture.mdsrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
test/schemas.test.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
test/schemas.test.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
test/schemas.test.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
test/schemas.test.tssrc/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/schemas.test.tstest/analyze-run.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/commands/analyze.tssrc/core/models/openai-analysis.tssrc/core/results/write-result-bundle.tssrc/schemas/policy.tssrc/core/runtime/eligibility.tssrc/core/runtime/analyze-run.tssrc/core/runtime/intake.tssrc/schemas/result.ts
🪛 LanguageTool
docs/references/repo-architecture.md
[style] ~130-~130: This phrase is redundant. Consider writing “outcome”.
Context: ...ceeded only when the rerun upgrades the final outcome to ready_to_implement; otherwise pres...
(FINAL_END)
🔍 Remote MCP Firecrawl
Summary of additional review-relevant facts
-
OpenAI Agents + Zod compatibility risk: multiple issues show that Zod v3.25.68+ (and some v4 combinations) break TypeScript compatibility when different Zod copies/type layouts are present; the Agents SDK consumers were advised to pin to zod <= 3.25.67 or use the SDK’s documented workarounds (peer-dep/dedupe or import-from-subpath approaches). See discussion and repros in the Zod and openai-agents repos.
-
OpenAI Agents SDK action taken / guidance: the openai-agents-js repo documented the issue and applied fixes/workarounds (examples: locking/pinning Zod or making Zod a peerDependency) and closed related issues / PRs. Relevant threads and PRs document pinning or peer-dep fixes.
Why this matters for this PR
- This PR adds
@openai/agentsto package.json (dependencies). If consumers or the repo itself end up with mismatched or duplicated Zod versions, TypeScript build/type errors or runtime/typing surprises can occur when supplying Zod schemas to the Agents SDK (the PR uses Zod schemas as agent outputType and in many result schemas). The documented mitigations are: pin Zod to a compatible version (<=3.25.67), make Zod a peerDependency of packages that expose/consume Zod schemas, or import Zod via the versioned subpaths per Zod guidance.
Actionable review suggestions (concise)
- Confirm repo dependency strategy for Zod (is zod present anywhere in repo devDependencies/peerDependencies?) and either:
- Pin/document zod version compatibility (e.g., require zod <=3.25.67) in README/package.json, or
- Make packages that expose Zod schemas treat zod as a peerDependency and add devDependency for tests, or
- Import Zod via subpath (zod/v3 or zod/v4/core) per Zod library-author guidance and/or test against multiple Zod versions in CI.
(Rationale: prevents the known TS type-assignability failures when@openai/agentsis used.)
Sources / tool calls used
- Firecrawl — searched and scraped GitHub/Zod/OpenAI issues documenting Zod ↔ Agents compatibility and recommended mitigations:
- Zod issue: TS instantiation / private _cached incompatibility discussion
- OpenAI Agents issue: Agent outputType type error & pin/peer-dep fixes
- OpenAI Agents zod pin discussion / recommendations
- Zod library-authors guidance page (peerDependency / subpath recommendations)
🔇 Additional comments (27)
test/schemas.test.ts (2)
54-89: Good test coverage for threshold ordering validation.This test correctly validates that the schema rejects policies where
triageNanoMax >= recoveryEscalationMin(both set to 58), exercising thesuperRefinecross-field guard inpolicySchema. The test aligns with the routing logic insrc/core/runtime/routing.tswhich depends on proper threshold ordering.
91-97: Evidence reference locatability test is appropriate.This validates that evidence references require more than just
artifactFamilyto be locatable, matching the schema's requirement forpackageand/orlocatorfields as documented in the architecture reference.src/commands/analyze.ts (1)
1-60: LGTM!The analyze command module is well-structured with proper documentation. The doc comments for
AnalyzeCommandOptions,getAnalyzeHelp, andrunAnalyzeCommandsatisfy the coding guidelines. The privaterenderAnalyzeSummaryhelper appropriately handles the optionalmodelUsedfield with a fallback.src/schemas/policy.ts (1)
52-89: Threshold ordering and weight validation look correct.The
superRefineimplementation properly enforces:
triageNanoMax < recoveryEscalationMin(lines 53-63)recoveryEscalationMin < fullEscalationMin(lines 65-75)- Weights sum to 100 within floating-point tolerance (lines 77-88)
This ensures the routing bands don't collapse and aligns with the routing logic in
src/core/runtime/routing.tswhich depends on these threshold orderings.docs/references/repo-architecture.md (1)
71-81: LGTM!The documentation updates accurately reflect the Phase 04 implementation:
- Blocked runs preserve
semanticOutcome: blockedindecision_report.json- Schema-typed output requirement and blocked-before-synthesis behavior for parse failures
- Recovery restricted to
implementationmode withrecoveryEscalationMingating- Recovery state distinguishes "rerun happened" vs "uncertainty resolved"
These align with the runtime code in
openai-analysis.ts,recovery.ts, andwrite-result-bundle.ts.Also applies to: 107-115, 120-132
src/core/models/openai-analysis.ts (3)
177-194: Good structured error handling forModelBehaviorError.The try/catch correctly handles SDK parse failures by checking
error.name === 'ModelBehaviorError'and wrapping with contextual details (model name, error details). This addresses the past review suggestion and aligns with the documented blocked-before-synthesis behavior.
1-27: Documentation requirements satisfied.The file properly includes:
- Top-level
@seereferencing the OpenAI Agents SDK docs (line 4)- Doc comments on all exported types (
AnalysisModelInput,AnalysisModelExecutor)- The
@throwsdocumentation for error conditions
163-175: No issues found. The@openai/agentsSDK version 0.8 fully supports Zod 4 schemas asoutputType, and the agent configuration at these lines is compliant.src/core/results/write-result-bundle.ts (1)
90-223: LGTM!The
writeResultBundleimplementation is well-structured:
- Parallel artifact writes via
Promise.all(lines 193-215)- Sequential manifest write after artifacts complete (line 216)
- Conditional
open_questions.jsonhandling with cleanup viarm(..., { force: true })(lines 212-214)- Atomic JSON writes via
writeJsonFileAtomic- Schema validation on all artifacts before writing
The documentation requirements are satisfied with proper
@paramand@returnstags.src/core/runtime/intake.ts (2)
78-91: Good security boundary enforcement.The implementation properly addresses the trust boundary concern:
- Validates manifest path matches expected canonical path (lines 79-86)
- Validates
artifactRootis withinrunDirectoryviaassertPathWithinBase(lines 88-91)- Individual artifact paths are validated within
artifactRootviareadPrepArtifacthelper (line 65)This prevents a tampered manifest from redirecting reads outside the run directory.
92-129: Concurrent artifact loading is correct.The seven independent artifact reads are properly parallelized via
Promise.all, addressing the previous review feedback about I/O waterfall. Each read goes throughreadPrepArtifactwhich validates paths stay withinartifactRoot.src/core/runtime/eligibility.ts (2)
71-89: Good separation of bundle-level degradation handling.The
buildBundleFallbackClaimhelper correctly creates a bundle-scoped claim without apackagefield, usinglocator: 'meta:bundle'instead. This addresses the previous review concern about bundle-level degradation being incorrectly attributed to arbitrary packages.
98-228: LGTM!The structural eligibility evaluation is well-structured:
- Bundle-level degradation computed once (lines 105-110) and added once (lines 175-177)
- Per-package findings collected with clear blocker/degradation separation
- Mode-specific implementation gating (lines 143-156)
- Clear outcome determination logic with proper
outcomeClassandprimaryActionmappingtopRiskSignalscorrectly filters outinfoseverity signalsDocumentation requirements are satisfied.
src/core/runtime/analyze-run.ts (6)
176-186: LGTM!The blocked branch correctly omits
modelUsedfromwriteResultBundle, addressing the previous review concern about recording a planned model as used when no model execution occurred.
22-36: LGTM!The exported types now have proper doc comments addressing the previous review feedback.
38-62: LGTM!The
blockedSynthesishelper correctly constructs a schema-valid blocked synthesis with the required validation checklist item for refresh guidance.
64-104: LGTM!The
resolvePrimaryActionandfinalizeOutcomeClasshelpers correctly implement the outcome/action mapping logic with proper precedence (structural blocked > synthesis blocked > other outcomes).
256-274: LGTM!The final bundle write correctly captures the active routing decision (reflecting any escalation), includes
modelUsedfrom the synthesis result, and derives the outcome/action pair consistently.
209-227: No issue found. The schema and fixture configuration confirm thatpolicy.modelRouting.tiers.fullis a string (model name), matching the expected type forselectedModel.> Likely an incorrect or invalid review comment.test/analyze-run.test.ts (6)
932-943: LGTM!The
openQuestionsPathhandling now uses an explicit guard (if (openQuestionsPath === undefined) throw) instead of null-coalescing, addressing the previous review feedback and making the code type-safe.
1-28: LGTM!Clean imports covering the necessary test utilities and runtime modules. Using Bun-native test framework as preferred.
60-351: LGTM!The
writePrepFixturehelper is well-structured with comprehensive artifact generation and proper use ofPromise.allfor parallel I/O. The signal generation logic correctly distinguishes major vs minor version changes.
430-511: LGTM!Good coverage of routing tier selection: low-risk staying on nano, high-risk escalating to full, and major version boundary detection. Proper cleanup in
finallyblocks.
513-675: LGTM!Excellent coverage of the core runtime flows:
- Blocked runs short-circuit before model invocation
- Boundary escape protection prevents path traversal
- Recovery hop is properly bounded with correct state transitions (
attempted→succeeded)
964-972: LGTM!Good contract test ensuring the
analysisPromptExampleremains schema-valid, which is critical since it's used to guide model output shape.src/schemas/result.ts (2)
35-50: LGTM!The
evidenceReferenceSchemanow correctly enforces that at least one ofpackageorlocatormust be present, addressing the previous review feedback about non-locatable evidence references.
226-246: LGTM!The
resultManifestSchemaupdates correctly reflect the new runtime requirements:
modelUsedis now optional, properly handling blocked runs that never invoke the model.routingDecisionandrecoverycapture the routing/escalation flow.- The cross-validation between
outcomeClassandprimaryActionensures consistency.
There was a problem hiding this comment.
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 `@src/core/runtime/eligibility.ts`:
- Around line 30-44: The hasCollectedDocsOrReleases function is repeatedly
scanning prepBundle.docs.packages and prepBundle.releases.packages with .find,
causing quadratic behavior; fix by pre-indexing those arrays into Maps (e.g.,
docsByPackage: Map<string, DocsPackageEntry> and releasesByPackage: Map<string,
ReleasesPackageEntry>) once before any package loop and then replace all .find
calls in hasCollectedDocsOrReleases (and the other repeated lookups noted around
the other eligibility checks) with O(1) map.get(packageName) lookups; update
function(s) to accept or capture those maps (or create a small helper to build
and reuse them) so subsequent membership/status checks use the maps instead of
.find.
- Around line 151-163: In implementation mode the diff being "skipped" should be
treated the same as "degraded": update the checks that currently only test
diffEntry?.status === 'degraded' to also consider diffEntry?.status ===
'skipped' and push an appropriate degradation message (e.g., 'source diff is
degraded' or 'source diff skipped/unavailable'); apply this change in both
places around the existing blocks that reference mode, diffEntry, degradations
(the branch that checks mode === 'implementation' and the else branch that
checks prepBundle.manifest.request.targetVersion) so a skipped diff prevents
marking the run implementation-ready when no usable source diff exists.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6e58c4ba-894d-4d48-9401-cbeda8df3df0
📒 Files selected for processing (5)
docs/references/cli-reference.mddocs/references/repo-architecture.mdsrc/core/runtime/eligibility.tssrc/core/runtime/recovery.tstest/analyze-run.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
docs/references/cli-reference.mddocs/references/repo-architecture.mdsrc/core/runtime/recovery.tssrc/core/runtime/eligibility.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
src/core/runtime/recovery.tssrc/core/runtime/eligibility.tstest/analyze-run.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/core/runtime/recovery.tssrc/core/runtime/eligibility.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
src/core/runtime/recovery.tssrc/core/runtime/eligibility.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
src/core/runtime/recovery.tssrc/core/runtime/eligibility.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
src/core/runtime/recovery.tssrc/core/runtime/eligibility.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
src/core/runtime/recovery.tssrc/core/runtime/eligibility.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
src/core/runtime/recovery.tssrc/core/runtime/eligibility.tstest/analyze-run.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/analyze-run.test.ts
🪛 LanguageTool
docs/references/repo-architecture.md
[style] ~137-~137: This phrase is redundant. Consider writing “outcome”.
Context: ...ceeded only when the rerun upgrades the final outcome to ready_to_implement; otherwise pres...
(FINAL_END)
🔍 Remote MCP Firecrawl
Key additional facts relevant to this PR
-
Known Zod ↔
@openai/agentsTypeScript incompatibility: OpenAI Agents SDK reported TypeScript errors when consumers or the SDK use Zod v3.25.68+ (and some v4 cases). OpenAI recommended pinning to zod@3.25.67 as a workaround and published fixes/workarounds (make Zod a peerDependency / dedupe / docs updates). -
Zod maintainers and the community tracked related breaking assignability / TS instantiation issues caused by private/internal properties (e.g., _cached) introduced in zod v3.25.68+, and recommended library authors rely on peerDependencies or import-from-subpath patterns to avoid multiple Zod copies.
-
Multiple community posts and Q&A confirm practical impact: users encountering Agent outputType/type errors are resolved by downgrading to zod@3.25.67 or ensuring a single deduplicated Zod instance. (Community threads / StackOverflow evidence.)
Why this matters for this PR
- This PR adds
@openai/agentsto dependencies and uses Zod schemas directly as the agent outputType (src/core/models/openai-analysis.ts). If the repository (or downstream consumers) installs an incompatible or duplicate Zod version, TypeScript build or runtime typing errors are likely.
Actionable checks / suggestions to include in review
- Ask the author to ensure Zod will be deduplicated or constrained:
- Prefer making zod a peerDependency of packages that expose Zod schema values, or
- Pin zod in package.json (e.g., <=3.25.67) and document the compatibility caveat, or
- Import Zod from explicit subpaths per Zod guidance to avoid cross-copy assignability issues.
(These mitigations are supported by the sources above.)
Sources / searches used
- Firecrawl search results (scraped pages):
- openai-agents-js issue: "Agent outputType type error with zod@3.25.68+" — GitHub
- zod issue: "TS2589: Type instantiation is excessively deep..." — GitHub
- OpenAI community thread: "Typesafe structured output openai Agents Typescript"
- StackOverflow: "Agent outputType not working when using zodSchema"
🔇 Additional comments (1)
test/analyze-run.test.ts (1)
1015-1023: Lines 1015–1023 only validate static schema structure; the real agent integration is already tested.This test verifies that
analysisPromptExampleparses successfully againstanalysisSynthesisSchema. However, the actual@openai/agents+ Zod integration is tested separately intest/openai-analysis.test.ts(lines 122–156), which mocks the Agents SDK and verifies thatrunOpenAIAnalysisconstructs an Agent withoutputType: analysisSynthesisSchema.The Zod dependency is also correctly resolved:
zod@4.3.6satisfies the@openai/agents@0.8.3peer dependency (^4.0.0), and no duplicate instances exist in the lockfile.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/runtime/analyze-run.ts`:
- Around line 128-135: The code currently passes prepBundle.manifest.repoRoot
into loadCheckedInPolicy (via dependencies.loadPolicy), trusting a parsed
manifest; instead compute and use the caller repo root (the same value used to
call loadPrepBundleFromRunId) — e.g. const callerRepoRoot = options.repoRoot ??
process.cwd(); — and pass callerRepoRoot to (dependencies.loadPolicy ??
loadCheckedInPolicy) to avoid manifest tampering; keep
prepBundle.manifest.repoRoot only for informational use or as a fallback if
absolutely necessary.
In `@src/core/runtime/intake.ts`:
- Around line 89-92: The code currently bounds manifest.artifactRoot against
runDirectory, which allows a tampered manifest to point at non-prep artifacts;
instead, validate artifactRoot against the canonical prep subtree: replace the
assertPathWithinBase(runDirectory, manifest.artifactRoot) call with an assertion
against the prep root (e.g., assertPathWithinBase(prepRoot,
manifest.artifactRoot) or assertPathWithinBase(path.join(runDirectory, 'prep'),
manifest.artifactRoot)), ensuring artifactRoot is resolved inside the prep
subtree used by the prep loader (update any variables or imports to reference
prepRoot if necessary).
- Around line 79-87: In loadPrepBundleFromManifest, don't call
readJsonFile(manifestPath) before validating the path: first compute
resolvedManifestPath = path.resolve(manifestPath) and expectedManifestPath =
path.join(runDirectory, 'prep', 'manifest.json') (or use path.resolve on
expectedManifestPath), compare them and throw the existing mismatch Error if
they differ, and only after that call readJsonFile (using the validated
resolvedManifestPath or manifestPath) to parse the JSON; update the sequence
around readJsonFile, manifestPath, resolvedManifestPath, expectedManifestPath to
enforce the pre-check.
In `@test/analyze-run.test.ts`:
- Around line 475-479: The test currently asserts decision.escalationScore
against the hardcoded 72; change it to assert against the configured threshold
instead. Replace the literal with the active threshold from the objects you
already have (e.g. use decision.fullEscalationMin or policy.fullEscalationMin)
so the assertion reads like:
expect(decision.escalationScore).toBeGreaterThanOrEqual(<configured threshold>),
referencing buildRoutingDecision, decision, and policy to locate the check.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 01f95ff8-3943-4770-98fc-51e6ae0c49ff
📒 Files selected for processing (5)
src/core/runtime/analyze-run.tssrc/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tstest/analyze-run.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
src/core/runtime/eligibility.tssrc/core/runtime/intake.tstest/analyze-run.test.tssrc/schemas/result.tssrc/core/runtime/analyze-run.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/analyze-run.test.ts
🔍 Remote MCP Firecrawl
Summary of additional, review-relevant facts
-
Zod ↔ OpenAI Agents TypeScript incompatibility: OpenAI Agents reported TypeScript errors when consumers or the SDK use zod v3.25.68+ (and some v4 cases). OpenAI recommended locking Zod to 3.25.67 as a workaround while they addressed the issue.
-
Root cause and long-term guidance from Zod maintainers: the breakage is due to private/internal members (e.g., _cached) introduced in Zod v3.25.68+ that make Zod schema instances incompatible across multiple installed copies; maintainers advise library authors to use peerDependencies or ensure a single deduped Zod instance, or otherwise avoid exposing Zod schema values in public APIs.
-
OpenAI/Community evidence of real user impact and mitigations:
- OpenAI’s agents repo applied fixes/workarounds (making zod a peerDependency / replacing zod subpath imports) and suggested pinning to <=3.25.67 until resolved.
- Community and OpenAI forum posts document related structured-output and ModelBehaviorError issues tied to Zod version mismatches; several users resolved by downgrading/pinning Zod.
Actionable check(s) to include in PR review (based on above)
- Because this PR adds
@openai/agentsand passes Zod schemas to the agent outputType (src/core/models/openai-analysis.ts), request the author to mitigate Zod compatibility risk by one or more of:- Add guidance in package.json / contributor docs to pin or otherwise constrain Zod so consumers won’t install incompatible versions (e.g., recommend zod@3.25.67), or
- Prefer declaring Zod as a peerDependency (or add a note to upstream package consumers) so a single deduped Zod instance is used, or
- Avoid exposing Zod schema instances directly to
@openai/agents(e.g., pass JSON Schema or inferred types instead) to sidestep cross-package assignability issues.
Sources / searches used
- Firecrawl search results (scraped pages):
- openai/openai-agents-js issue: "Agent outputType type error with zod@3.25.68+" — GitHub
- colinhacks/zod issue: "TS2589: Type instantiation is excessively deep..." — GitHub
- OpenAI Developer Community thread: "Structured Outputs example broken (node.js: triggering Zod version discovered)"
🔇 Additional comments (2)
src/schemas/result.ts (1)
157-172: No compatibility issue detected. The project pinszod@^4.3.6, which satisfies@openai/agents@^0.8.3's peer dependency requirement ofzod@^4.0.0. The lockfile shows a single Zod resolution with no duplicates or conflicts.> Likely an incorrect or invalid review comment.src/core/runtime/intake.ts (1)
1-26: Biome CI passes clean.
npx@biomejs/biomeci --error-on-warnings .exits with code 0. No diagnostics at warning/error severity. ✓
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/runtime/analyze-run.ts`:
- Around line 116-123: The JSDoc for analyzePreparedRun has the `@see` tag placed
before `@param` and `@returns`; update the comment block for the function
analyzePreparedRun so that tags follow the required order: place all `@param`
entries first, then the `@returns` tag, and move the `@see` tag after `@returns` (and
before any `@deprecated/`@throws if present); ensure the description and other
tags remain unchanged and only the tag order is adjusted.
- Around line 128-133: The code currently awaits
loadPrepBundleFromRunId(repoRoot, options.runId) before loading the policy,
causing an unnecessary serial I/O waterfall; instead, start both operations
concurrently by creating the prep promise and the policy promise (using
dependencies.loadPolicy ?? loadCheckedInPolicy with repoRoot) without awaiting,
then await both (e.g., via Promise.all) and assign prepBundle and policy from
the resolved values; reference the existing symbols loadPrepBundleFromRunId,
dependencies.loadPolicy, loadCheckedInPolicy, prepBundle, and policy to locate
where to change.
In `@src/core/runtime/intake.ts`:
- Around line 68-75: The JSDoc for loadPrepBundleFromManifest is missing `@throws`
entries for the runtime errors it can throw; update the function's comment block
to add one or more `@throws` tags that name each exception type (or a clear
description of the error condition) and explain when it is thrown (e.g.,
manifest not found, invalid manifest schema, or path validation failures
originating from the runDirectory check used in loadPrepBundleFromManifest), so
the exported contract documents all throw points in the function body.
- Around line 79-90: Resolve symlinks before trusting paths: call fs.realpath
(or equivalent) on manifestPath and use that real path for the earlier
comparison (replace resolvedManifestPath with the realpath result) and before
reading the file in readJsonFile, and in assertPathWithinBase ensure you compare
realpath(prepRoot) with realpath(join(prepRoot, manifest.artifactRoot)) (or
lstat+reject if the target is a symlink you don't allow) so any symlinked
manifest or artifactRoot that points outside the run directory is detected and
rejected; update the checks that currently use path.resolve()/path.relative() to
use the real paths returned by fs.realpath for manifestPath, prepRoot and
artifactRoot, and error out if the realpath of manifestPath or artifactRoot is
not within the realpath of prepRoot.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 36a60608-9e30-4821-8a82-54ca9f89c259
📒 Files selected for processing (3)
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
test/analyze-run.test.tssrc/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
test/analyze-run.test.tssrc/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
test/analyze-run.test.tssrc/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
test/analyze-run.test.tssrc/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
test/analyze-run.test.tssrc/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
test/analyze-run.test.tssrc/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
test/analyze-run.test.tssrc/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/analyze-run.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/core/runtime/intake.tssrc/core/runtime/analyze-run.ts
🔍 Remote MCP Firecrawl
Summary — relevant live-web findings for this PR (focus: @openai/agents + Zod compatibility)
-
Problem observed by OpenAI Agents maintainers: Zod v3.25.68+ introduces a private/property change causing TypeScript assignability errors when a consumer and the SDK each have their own Zod copy; openai/agents advised pinning to zod@3.25.67 as a workaround.
-
Zod maintainer/issue thread confirms root cause: private/internal members (e.g., _cached) and multiple Zod instances cause the TS errors; guidance to library authors: use peerDependencies (so consumers & lib share a single Zod instance) or avoid exposing Zod schema instances in public APIs.
-
Related Zod issue and fixes: the _cached/private-property problem was raised in zod issues; zod maintainers changed modifiers in a later commit but the community guidance remains to dedupe Zod (peerDeps) or pin to known-compatible versions.
Actionable reviewer notes (grounded in above sources)
- Because this PR passes Zod schema values into
@openai/agents(analysis module → agent.outputType), there is a documented compatibility risk. Mitigations drawn from the cited threads:- Pin or document a safe Zod version (e.g., recommend/lock to zod@3.25.67) for consumers testing the analyze runtime.
- Prefer making Zod a peerDependency (or otherwise ensure a single deduped Zod instance across packages) so types are compatible at compile time.
- Alternatively avoid exposing Zod schema instances to
@openai/agents(e.g., pass JSON Schema or inferred TS types instead) to eliminate cross-package Zod-assignability risk.
Sources (all retrieved via Firecrawl)
- openai/openai-agents-js — "Agent outputType type error with zod@3.25.68+" (GitHub issue)
- colinhacks/zod — "TS2589: Type instantiation is excessively deep ..." (GitHub issue & discussion)
- colinhacks/zod — "_cached private property" compatibility issue (GitHub issue)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f97bad5692
ℹ️ 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".
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/core/runtime/intake.ts (2)
60-65:⚠️ Potential issue | 🔴 CriticalLexical path checks still allow symlink escapes.
path.resolve()andassertPathWithinBase()only normalize strings. Ifprep/manifest.json,manifest.artifactRoot, or any artifact file is a symlink,readJsonFile()will still follow it outside the run boundary. Resolve real paths first, or explicitly reject symlinks, before comparing and reading here.Also applies to: 80-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/runtime/intake.ts` around lines 60 - 65, readPrepArtifact currently uses assertPathWithinBase and path normalization which can be bypassed by symlinks; before calling readJsonFile you must resolve and validate real filesystem paths (or reject symlinks) to prevent escapes. Update readPrepArtifact to call fs.realpathSync or lstat+resolve for artifactRoot and artifactPath (and any similar helpers used in the other locations around lines 80-103), verify the resolved path is within the resolved base via assertPathWithinBase, and explicitly error if any path component is a symlink (or if realpath resolution moves outside the base) before passing the path to readJsonFile. Ensure the same change is applied to the other functions that read artifacts.
68-75:⚠️ Potential issue | 🟡 MinorPrefix the
@throwsentry with the exception type.This still misses the required type token because the tag starts with the condition instead of the exception. Use
@throws Error - ....📝 Proposed doc fix
- * `@throws` When the manifest path or manifest identity does not match the requested run. + * `@throws` Error - When the manifest path or manifest identity does not match the requested run.As per coding guidelines "If an exported function body contains token
throwin newly added or modified lines, at least one@throwsblock is required per exception type with non-empty description explaining when thrown; use separate@throwsblock per exception type and begin with exception name or explicit type token".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/runtime/intake.ts` around lines 68 - 75, The JSDoc `@throws` entry for the prep bundle loader must begin with an exception type token; update the JSDoc above the function that "Loads a prep bundle from an existing manifest path" (the prep-bundle loader function) to use one or more `@throws` tags that start with the exception type (e.g., "@throws Error - manifest path does not exist or cannot be read" and "@throws Error - manifest identity does not match the requested run"), matching each distinct throw site in the function; ensure each `@throws` has a non-empty description explaining when it is thrown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/core/runtime/intake.ts`:
- Around line 60-65: readPrepArtifact currently uses assertPathWithinBase and
path normalization which can be bypassed by symlinks; before calling
readJsonFile you must resolve and validate real filesystem paths (or reject
symlinks) to prevent escapes. Update readPrepArtifact to call fs.realpathSync or
lstat+resolve for artifactRoot and artifactPath (and any similar helpers used in
the other locations around lines 80-103), verify the resolved path is within the
resolved base via assertPathWithinBase, and explicitly error if any path
component is a symlink (or if realpath resolution moves outside the base) before
passing the path to readJsonFile. Ensure the same change is applied to the other
functions that read artifacts.
- Around line 68-75: The JSDoc `@throws` entry for the prep bundle loader must
begin with an exception type token; update the JSDoc above the function that
"Loads a prep bundle from an existing manifest path" (the prep-bundle loader
function) to use one or more `@throws` tags that start with the exception type
(e.g., "@throws Error - manifest path does not exist or cannot be read" and
"@throws Error - manifest identity does not match the requested run"), matching
each distinct throw site in the function; ensure each `@throws` has a non-empty
description explaining when it is thrown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b7d234d7-f86c-4053-99b7-86915edf2ac9
📒 Files selected for processing (3)
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx}: Runnpx@biomejs/biomeci --error-on-warnings .from repo root and attach full output to PR review; hard fail if command exits non-zero
Ensurebiome ci --error-on-warningsexits clean with zero diagnostics at warning/error severity; hard fail if any diagnostics exist
Hard fail if Biome output contains any rule codes starting withlint/correctness/orlint/complexity/, even if configured as warning
Do not accept broad rule suppressions viabiome-ignore; only allow when narrowly scoped to one line/block, includes a reason, and links to a tracking issue or ADR/SPEC
No promise-valued statement may be left unhandled; must beawaited,returned, terminated with.catch(...), terminated with.then(onFulfilled, onRejected)with rejection handler, or explicitly ignored withvoidmeeting additional requirements
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), fire-and-forget viavoidis forbidden unless it has an inline comment explaining why detaching is safe AND errors are surfaced via.catch(...)or dedicated error sink
For files importing AI SDK/Vercel AI modules (ai,@ai-sdk/*,@vercel/ai,openai,@openai/*), doc comment must include either an@seeline referencing ADR/SPEC path or official docs URL, OR an@remarksline containingADR-orSPEC-; hard fail if missing
Deferawaituntil the branch that truly needs it; use early returns to skip expensive work and eliminate waterfalls
Start independent async work immediately, then await later (const p = fn(); ... await p); in Route Handlers/Server Actions avoid auth→config→data chains
UsePromise.all()for independent operations; do not serialize unrelated fetches
For dependency graphs with some tasks depending on others, maximize overlap using strategic promise composition orbetter-allfor DAG-style orchestration
Use strategic<Suspense>boundaries to stream non-critical regions; fetch in leaf async ...
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (Custom checks)
Hard fail if PR introduces any test-only helpers, shims, mocks, or conditional branches intended only for tests in production source files; all test scaffolding must live in test suites and configuration
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx}: Doc comment formatting must satisfy Biome, including violations ofuseSingleJsDocAsterisk; hard fail if Biome reports any JSDoc/TSDoc formatting issues
Every changed or added exported declaration must have an immediately preceding doc comment/** ... */with no blank line separating it, including functions, async functions, consts, classes, interfaces, types, enums, Next.js route handlers (GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD), and re-exports
Pure presentational React components exported only for JSX composition may omit doc comments only if they have no non-obvious behavior (no IO, side effects, caching semantics, security checks, data fetching, tool calls)
For every required doc comment, the first non-empty line must be a single-sentence summary ending with a period that describes behavior and intent, not just the symbol name
Only these JSDoc/TSDoc tags are allowed in TS/TSX files:@remarks,@param,@typeParam,@returns,@throws,@example,@see,@deprecated; hard fail if any other tag appears
Tags in doc comments must appear in this exact order (skip unused):@remarks, all@typeParam, all@param,@returns, all@throws, all@example, all@see,@deprecated; hard fail if out of order
@remarksmust be separated from summary by a blank line and have non-empty content; hard fail if empty or not separated
For every exported function including route handlers, document every parameter with@paramin format@param <name> - <description>where name matches actual parameter exactly, separator-is present, and description is non-empty and explains semantics and constraints; JSDoc type braces forbidden in TS/TSX
If an exported function/type/class introduces non-trivial generic type parameters, require@typeParamin format@typeParam <name> - <description>; hard fail if required generics are undocumented or format is incorrect
Every exported function whose return type is notvoidorPromise<void>must have...
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
**/*
📄 CodeRabbit inference engine (Custom checks)
Reject Unicode EM DASH (code point U+2014) in comments, string literals, and docs; use
--instead; hard fail if any newly introduced content includes U+2014
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css}: Honorprefers-reduced-motionby providing reduced variant or disabling animations
Animate only compositor-friendly props (transform,opacity); never animate layout props (top,left,width,height)
Never usetransition: all; list properties explicitly
Animate only to clarify cause/effect or add deliberate delight; choose easing to match the change (size/distance/trigger)
Ensure animations are interruptible and input-driven; never autoplay animations
Set correcttransform-originso motion starts where it 'physically' should; usetransform-box: fill-boxfor SVG transforms on<g>wrapper
Animate SVG wrapper around SVG for transform animations; avoid animating the<svg>element directly when smoothness matters
Usecontent-visibility:autopluscontain-intrinsic-sizefor long lists to skip offscreen layout/paint
Batch DOM writes, then do layout reads; prefer toggling CSS classes over inline styles to avoid forced reflows
Set-webkit-tap-highlight-colorto match design when setting custom tap highlight
Useoverscroll-behavior: containin modals/drawers to prevent scroll chaining
During drag, disable text selection and setinerton dragged elements
Use optical alignment; adjust ±1px when perception beats geometry
Maintain deliberate alignment to grid/baseline/edges; avoid accidental placement
Balance icon/text lockups for weight/size/spacing/color consistency
Verify design on mobile, laptop, and ultra-wide (simulate ultra-wide at 50% zoom)
Avoid unwanted scrollbars; fix overflows to prevent layout shifts
Prefer Flex/Grid over JS measurement for layout
Usefont-variant-numeric: tabular-numsfor number comparisons
Text containers must handle long content viatruncate,line-clamp-*, orbreak-words
Flex children needmin-w-0to allow truncation
Test iOS Low Power Mode and macOS Safari for performance impacts
For native<select>, explicitly setbackground-colorandcolor(Windows fix)
Use layered shadow...
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,css,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,css,html}: Provide full keyboard support per WAI-ARIA APG patterns for all interactive components
Ensure visible focus rings using:focus-visibleand:focus-within; never useoutline: nonewithout visible focus replacement
Manage focus (trap, move, return) per WAI-ARIA APG patterns for modals, dialogs, and complex interactions
Hit target must be ≥24px (mobile ≥44px); if visual <24px, expand hit area; for mobile<input>font-size ≥16px to prevent iOS zoom
Never disable browser zoom viauser-scalable=noormaximum-scale=1; usetouch-action: manipulationto prevent double-tap zoom
Touch and drag interactions must have generous targets and clear affordances; avoid finicky interactions
If it looks clickable, it must be clickable; affordance must match interaction
Respect safe areas usingenv(safe-area-inset-*)
Use curly quotes (" "); avoid widows/orphans viatext-wrap: balance
Provide redundant status cues, not color-only; icon-only buttons must have text labels oraria-label
Use…character (not...) for ellipsis in UI text, labels, and CSS content
Use non-breaking spaces in UI:10 MB,⌘ K, brand names
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (Custom checks)
**/*.{ts,tsx,js,jsx,html}: Never block paste in<input>/<textarea>
Enter key submits focused<input>; in<textarea>, ⌘/Ctrl+Enter submits
Useautocompleteattribute with meaningfulname; use correcttypeandinputmode
Disable spellcheck for emails/codes/usernames viaspellcheck="false"
Placeholders should end with…and show example pattern
For checkboxes/radios, ensure no dead zones; label+control must share one hit target
Links must use<a>/<Link>for navigation to support Cmd/Ctrl/middle-click; never use<div onClick>for navigation
Use politearia-livefor toasts/inline validation feedback
Page<title>must match current context
Accessible names must exist even when visuals omit labels for form fields and icon buttons
Addscroll-margin-topon headings; include 'Skip to content' link; use hierarchical<h1>–<h6>
Provide accuratearia-labelfor icon-only buttons; mark decorative elements witharia-hidden
Prefer native semantics (button,a,label,table) before ARIA
Preload above-fold images; lazy-load the rest vialoading="lazy"
Use<link rel="preconnect">for CDN domains to reduce connection latency
Critical fonts: use<link rel="preload" as="font">withfont-display: swap
For dark themes, setcolor-scheme: darkon<html>
Use<meta name="theme-color">matching page background for browser chrome theming
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (Custom checks)
Prevent Cumulative Layout Shift (CLS) by setting explicit image dimensions
Files:
src/core/runtime/analyze-run.tssrc/core/runtime/intake.tstest/analyze-run.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Keep repo buildable with Bun-only commands and prefer fixture-backed tests over live network gates
Files:
test/analyze-run.test.ts
🔍 Remote MCP Firecrawl
Summary of additional review-relevant findings
-
There is a known TypeScript compatibility issue between Zod v3.25.68+ (and some v4 variants) and consumers that ship their own Zod copies: Zod added/changed a private/internal field (e.g., _cached) which breaks structural assignability across package boundaries and can produce TS errors like "Types have separate declarations of a private property '_cached'" and "TS2589: Type instantiation is excessively deep…". See colinhacks/zod issues summarizing the root cause and fixes: colinhacks/zod#4903 and colinhacks/zod#4984.
-
The OpenAI Agents JS SDK reported that this exact issue caused errors when consumers pass Zod schemas into agent APIs (agent.outputType). The Agents repo advised the workaround of pinning to zod@3.25.67 and later updated to avoid duplicate Zod installs (making zod a peerDependency / other changes). See openai/openai-agents-js issue and related PRs that recommended locking/pinning or making Zod a peerDependency: openai/openai-agents-js#187.
-
Community/consumer guidance and common mitigations found across the discussion:
- Pin Zod to 3.25.67 (or override/resolve to that version) to avoid the TS errors until the ecosystem is stable.
- Make Zod a peerDependency in libraries so consumers and the library share a single Zod instance (dedupe). OpenAI's Agents team implemented this approach in their repo.
- Avoid exposing Zod schema instances in public APIs consumed across package boundaries (instead pass JSON Schema, raw typed shapes, or inferred TS types) to eliminate cross-package Zod assignability surface.
Concrete implications for this PR (deps-workbench)
- This PR passes Zod schema objects into
@openai/agents(analysisSynthesisSchema → agent.outputType) and adds@openai/agentsas a new dependency (package.json). That creates the same compatibility risk described above: consumers with mismatched/deduped Zod versions may get TS errors or runtime type incompatibilities. Evidence: openai/openai-agents-js issue#187and zod issues#4903/#4984.
Recommended reviewer actions (based on cited sources)
- Recommend/require one or more mitigations in this PR:
- Pin or document a safe Zod version for the repo (e.g., add an overrides/resolutions entry or lock to zod@3.25.67) so local development and tests avoid the issue.
- Prefer making Zod a peerDependency for packages that surface Zod schemas to other packages, or ensure dependency dedupe in the repo's lockfile.
- Consider avoiding passing raw Zod schema instances into external SDKs (e.g., pass JSON Schema or inferred TS types to
@openai/agents) to remove cross-package schema instance assignability concerns.
Sources (retrieved via Firecrawl)
- openai/openai-agents-js — "Agent outputType type error with zod@3.25.68+" (GitHub issue)
- colinhacks/zod — "Using 3.25.68 and newer versions fails to compile: Types have separate declarations of a private property '_cached'
#4903" and "TS2589: Type instantiation is excessively deep…#4984" (GitHub issues)
🔇 Additional comments (1)
src/core/runtime/analyze-run.ts (1)
9-11: No action needed.@openai/agents0.8.3 fully supports Zod 4.3.6 schemas without duplicate-Zod or private-field type errors. The import and default executor flow are sound.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a5f96586c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 402960ae80
ℹ️ 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".
| 'JSON shape:', | ||
| JSON.stringify(analysisPromptExample, null, 2), | ||
| 'Prep bundle summary:', | ||
| JSON.stringify(payload, null, 2), |
There was a problem hiding this comment.
buildPrompt currently stringifies the full prep payload, which includes unbounded nested fields (notably docs.packages[].raw from collectDocsArtifact and usage.packages[].auditJson). On larger runs this can push the prompt past model context limits and cause runOpenAIAnalysis to fail before any structured synthesis is produced, turning otherwise analyzable runs into blocked failures; the prompt should be reduced to a bounded summary instead of raw artifact dumps.
Useful? React with 👍 / 👎.
| resolveRepoRoot(manifest.repoRoot) !== expectedRepoRoot || | ||
| manifest.runId !== runId |
There was a problem hiding this comment.
The manifest identity guard compares resolveRepoRoot(manifest.repoRoot) against expectedRepoRoot, which only normalizes path strings and does not account for symlink aliases. If prepare and analyze are run from different aliases of the same repository path, this throws a false prep manifest identity mismatch and blocks valid runs; the comparison should use realpath-normalized roots to validate filesystem identity without rejecting legitimate aliases.
Useful? React with 👍 / 👎.
Summary
This PR delivers Phase 04 of
deps-workbench: the newanalyzecommand, the OpenAI-backed analysis runtime, conservative routing and bounded recovery, canonical result bundle generation, and the matching plan/reference documentation updates.Commits
feat(cli): add analyze runtime and result bundlesanalyzeCLI commanddocs(plan): record phase 04 analyze runtime deliveryanalyzesurface and corrected recovery/blocked semanticsVerification
bun run checkNotes
recoveryEscalationMinresult_manifest.jsonanddecision_report.json