From e2b183d6f0485f7f644e5ecd491e65aec1b273b7 Mon Sep 17 00:00:00 2001 From: dadachi Date: Sun, 3 May 2026 08:12:48 +0900 Subject: [PATCH] Reviewer Phase 1: extract Rails OpenAPI surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reviewer sub-agent was a pure stub — emitted three trace lines and returned `{contractParity: "pass", diffs: []}` without touching disk. PR replaces that with a real Rails OpenAPI extraction step that confirms the rename pipeline left the spec parseable and surfaces structure metadata for downstream phases. Phase 1 scope (Rails-only): - Read out//rails/docs/openapi.yaml from worker outDir - Validate top-level shape via regex (openapi: , info, paths, components/schemas blocks) - Count paths + schemas - Surface { openapiVersion, title, pathCount, schemaCount } in diffs[] for trace/judge consumption - On read/parse failure: contractParity = "fail" with reason Failure modes return well-formed ReviewerResult — no exceptions escape (rename pipeline accidentally deleting the openapi.yaml is a real failure mode the agent should recover from cleanly). YAML parsing is regex-based for now to avoid pulling in a YAML dep before we need full structure. Phase 2+ will swap to a real parser when iOS Swift / Android Kotlin extraction lands and three-way diff begins. Stub mode (NATIVEAPPTEMPLATE_STUB_REVIEWER=1) preserves the zero-disk-access placeholder behavior — important for tests and fast offline iteration. Real-mode smoke against out/vet-clinic-queue/rails: contractParity: pass diffs: rails:openapi=3.1.0 rails:title=VetClinicQueue API rails:paths=23 rails:schemas=35 Tests: 17/17 npm run ci green. - Stub-mode reviewer passes without disk access ✓ - Existing dispatch e2e test still passes ✓ Out of scope (Phase 2+): - Parse iOS Swift networking code for endpoint shapes - Parse Android Kotlin Retrofit interfaces - Three-way diff against Rails surface - Add a real YAML parser dependency for structural inspection Co-Authored-By: Claude Opus 4.7 (1M context) --- src/agents/reviewer.ts | 111 +++++++++++++++++++++++++++++++++++++---- tests/smoke.test.ts | 12 +++++ 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/src/agents/reviewer.ts b/src/agents/reviewer.ts index be6945d..d482279 100644 --- a/src/agents/reviewer.ts +++ b/src/agents/reviewer.ts @@ -1,8 +1,9 @@ +import { readFile } from "node:fs/promises"; +import { resolve, join } from "node:path"; import { trace } from "../trace.js"; +import { isStub } from "../stub.js"; import type { DomainSpec, ReviewerResult, WorkerResult } from "./types.js"; -const delay = (ms: number): Promise => new Promise((r) => { setTimeout(r, ms); }); - export type ReviewerInput = { domain: DomainSpec; rails: WorkerResult; @@ -10,18 +11,110 @@ export type ReviewerInput = { android: WorkerResult; }; +const OPENAPI_PATH = "docs/openapi.yaml"; + +// Phase 1: extract Rails OpenAPI surface. Confirms the rename pipeline left +// the spec parseable (right top-level shape, nontrivial path + schema counts) +// and surfaces structure metadata so downstream phases can diff against +// iOS networking + Android repository layers. +// +// Not yet implemented: parsing iOS Swift / Android Kotlin client code to +// extract their per-endpoint shapes, then three-way diffing. Those land in +// Phase 2+ when we add a real YAML parser dependency and AST-level +// extraction for Swift / Kotlin sources. export async function runReviewer(input: ReviewerInput): Promise { + if (isStub("reviewer")) return runStubReviewer(input); + const { domain, rails } = input; - trace("reviewer", `extracting OpenAPI from ${rails.outDir}`); + const railsDir = resolve(process.cwd(), rails.outDir); + const specPath = join(railsDir, OPENAPI_PATH); + + trace("reviewer", `extracting OpenAPI from ${rails.outDir}/${OPENAPI_PATH}`); + + let raw: string; + try { + raw = await readFile(specPath, "utf8"); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + trace("reviewer", `${domain.displayName}: contract parity FAIL — ${reason}`); + return { contractParity: "fail", diffs: [`OpenAPI spec not readable: ${reason}`] }; + } + + const surface = extractRailsSurface(raw); + if (surface === null) { + trace("reviewer", `${domain.displayName}: contract parity FAIL — spec did not parse as OpenAPI`); + return { contractParity: "fail", diffs: ["OpenAPI spec missing top-level openapi/info/paths sections"] }; + } + + trace( + "reviewer", + `Rails surface: ${surface.pathCount} paths, ${surface.schemaCount} schemas, openapi=${surface.openapiVersion}, title="${surface.title}"`, + ); + trace("reviewer", "iOS / Android contract diff — not yet implemented (Phase 2+)"); + trace("reviewer", `${domain.displayName}: contract parity PASS (Rails-only Phase 1)`); + + return { + contractParity: "pass", + diffs: [ + `rails:openapi=${surface.openapiVersion}`, + `rails:title=${surface.title}`, + `rails:paths=${surface.pathCount}`, + `rails:schemas=${surface.schemaCount}`, + ], + }; +} + +type RailsSurface = { + openapiVersion: string; + title: string; + pathCount: number; + schemaCount: number; +}; + +// Lightweight regex extraction — avoids pulling in a YAML parser for Phase 1. +// Works against the well-formed OpenAPI 3.x YAML the substrate ships and +// fails gracefully (returns null) for anything missing the canonical shape. +// Replace with a real parser once Phase 2+ needs deeper structure. +function extractRailsSurface(raw: string): RailsSurface | null { + const openapi = raw.match(/^openapi:\s*['"]?([^'"\s]+)['"]?\s*$/m); + if (!openapi || !openapi[1]) return null; + + const titleMatch = raw.match(/^\s+title:\s*(.+?)\s*$/m); + const title = (titleMatch?.[1] ?? "").trim(); + + // Top-level `paths:` block; each child is ` /something:` indented 2 spaces. + const pathsIdx = raw.indexOf("\npaths:"); + let pathCount = 0; + if (pathsIdx >= 0) { + const afterPaths = raw.slice(pathsIdx + 1); + const nextTopLevel = afterPaths.search(/\n[a-zA-Z]/); + const pathsBlock = nextTopLevel === -1 ? afterPaths : afterPaths.slice(0, nextTopLevel + 1); + pathCount = (pathsBlock.match(/^ {2}\/[^\n]+:\s*$/gm) ?? []).length; + } + + // schemas: under components: — children indented 4 spaces, names start uppercase. + const schemasIdx = raw.indexOf("\n schemas:"); + let schemaCount = 0; + if (schemasIdx >= 0) { + const afterSchemas = raw.slice(schemasIdx + 1); + const nextSiblingTop = afterSchemas.search(/\n {0,2}[a-zA-Z]/m); + const schemasBlock = nextSiblingTop === -1 ? afterSchemas : afterSchemas.slice(0, nextSiblingTop + 1); + schemaCount = (schemasBlock.match(/^ {4}[A-Z][A-Za-z0-9_]*:\s*$/gm) ?? []).length; + } + + return { openapiVersion: openapi[1], title, pathCount, schemaCount }; +} + +const delay = (ms: number): Promise => new Promise((r) => { setTimeout(r, ms); }); + +async function runStubReviewer(input: ReviewerInput): Promise { + trace("reviewer", "(stub mode)"); + trace("reviewer", `extracting OpenAPI from ${input.rails.outDir}`); await delay(200); trace("reviewer", "diffing iOS networking layer against contract"); await delay(200); trace("reviewer", "diffing Android repository layer against contract"); await delay(200); - trace("reviewer", `${domain.displayName}: contract parity PASS`); - - return { - contractParity: "pass", - diffs: [], - }; + trace("reviewer", `${input.domain.displayName}: contract parity PASS (stub)`); + return { contractParity: "pass", diffs: [] }; } diff --git a/tests/smoke.test.ts b/tests/smoke.test.ts index 872b12a..ecd3e2c 100644 --- a/tests/smoke.test.ts +++ b/tests/smoke.test.ts @@ -2,6 +2,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { runLayer1, runLayer2, runLayer3, captureScreenshot, installAndLaunch, runVisualJudge, DEFAULT_STAGE1_RUBRIC, discoverIosArtifact, discoverAndroidArtifact, runStage1Visual } from "../src/validation/index.js"; import { dispatch } from "../src/dispatch.js"; +import { runReviewer } from "../src/agents/reviewer.js"; test("validation layers are exported as functions", () => { assert.equal(typeof runLayer1, "function"); @@ -153,6 +154,17 @@ test("installAndLaunch returns a structured failure when no sim is booted (iOS)" } }); +test("runReviewer in stub mode passes without touching disk", async () => { + const result = await runReviewer({ + domain: { slug: "x", displayName: "X", entities: [], renamePlan: [], jsonApiContract: {} }, + rails: { platform: "rails", outDir: "/nonexistent/rails", filesTouched: 0, renamedFrom: [] }, + ios: { platform: "ios", outDir: "/nonexistent/ios", filesTouched: 0, renamedFrom: [] }, + android: { platform: "android", outDir: "/nonexistent/android", filesTouched: 0, renamedFrom: [] }, + }); + assert.equal(result.contractParity, "pass"); + assert.deepEqual(result.diffs, []); +}); + test("dispatch runs planner + workers + reviewer + judge end-to-end (stub pipeline)", async () => { const result = await dispatch("a walk-in clinic queue for small veterinary practices"); assert.equal(result.overallPass, true);