feat(core): proposal code materialization + build gate (G5 Phase 2+3)#495
Conversation
Generates and build-validates the irreducibly-code parts of a proposal — action / event / flow logic bodies a declarative ChangeDefinition cannot express. - P3 materializer: materializeProposalChanges() generates TS source for code targets via a CodeGenerationProvider, attaching it to the new optional ProposalChange.generatedSource. Generate → quality-gate → retry-with-feedback (default 3). Declarative targets + deletes are skipped. Returns a COPY; never mutates input, writes files, runs code, approves, or graduates. - P2 build gate: checkSourceSyntax() / createSyntaxQualityGate() validate generated source SYNTACTICALLY via Bun's transpiler (no project-aware type resolution — that would false-positive on project symbol refs; left to the graduation PR CI). validatePhase2() runs it and is wired into validateProposal (was a skipped stub). Warn-only by default; ValidationContext.strictGeneratedBuild escalates to blocking. All-declarative proposals still see Phase 2 "skipped". - New @linchkit/core/server exports + barrel-surface test updated. SAFETY: candidate source only — flows through validation (Phase 2) and double human review (draft + graduation PR) before it can land. Not yet wired into a live HTTP/draft path (a thin follow-up). "AI never modifies production directly." Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r to action Address codex review on the G5 P2+P3 engine: - ProposalFileWriter now writes `change.generatedSource` verbatim when present (preferred over deterministic codegen). Without this, a materialized proposal graduated to a PR containing only a declarative stub — the AI-generated handler was silently dropped. Added a writer test asserting verbatim output. - Narrow MATERIALIZABLE_TARGETS to `action` only. `event` is declarative (EventDefinition = name + payload, not logic), `defineFlow` does not exist yet, and EventHandler is not a ProposalChangeTarget — so action's handler is the one irreducibly-code target today. The set stays extensible. Dropped the defineFlow/defineEventHandler prompt guidance that referenced non-applicable APIs. Updated docs + tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e a blank file Address codex review (round 3) on the G5 P2+P3 engine: - validatePhase2 now includes every change with a string `generatedSource` (even empty/whitespace) so checkSourceSyntax flags it — previously the `.length > 0` filter skipped empty source, bypassing validation. - ProposalFileWriter treats an empty/whitespace generatedSource as absent and falls back to deterministic codegen, so a malformed materialization can never graduate a blank file. Tests for both paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address codex review (round 4): re-materializing a proposal that already carried `generatedSource` kept the old source on the shallow copy; if every new attempt failed the gate, the outcome was `failed` but the returned proposal still held stale code. Clear it up front so it is set again only on success; test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 48 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces G5 Phase 2+3: AI code materialization for proposals with syntactic validation. It adds a ChangesG5 Phase 2+3: Code Materialization and Validation
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request implements G5 Phase 2 and 3, introducing AI-proposal code materialization and syntactic build-gating. It adds a proposal materializer to generate TypeScript source for code-based changes (such as action handlers) and a code quality gate that leverages Bun's transpiler to syntactically validate the generated code. This validation is integrated into the validation engine as Phase 2, with support for a strict blocking mode. The review feedback suggests making the code fence stripping function more robust against conversational LLM output and non-string inputs, and ensuring that retry configuration parsing safely handles NaN values.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Address gemini-code-assist review on #495: - stripCodeFence now extracts a fenced block embedded in conversational prose and returns "" for non-string input (was: returned whole prose / could throw). - materializeProposalChanges treats a non-finite maxRetries (NaN/Infinity) as the default instead of silently skipping every attempt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/engine/validation-engine.ts (1)
1-705: ⚖️ Poor tradeoffConsider splitting validation-engine.ts to stay within the 500-line guideline.
This file is now ~706 lines, exceeding the repository's 500-line guideline. While this PR only added ~8 lines to an already-large file, consider splitting it into focused modules (e.g.,
validation-phase1.ts,validation-helpers.ts,validation-rules.ts) similar to the planned split forproposal.test.ts(tracked in#388).Based on learnings: In the linchkit repository,
packages/core/__tests__/proposal.test.tshas grown to 1315 lines (well over the 500-line guideline). Splitting this file into focused test modules is tracked in issue#388. The planned split includes:proposal-create.test.ts,proposal-submit.test.ts,proposal-approve-reject.test.ts,proposal-commit-deploy.test.ts,proposal-lifecycle.test.ts,proposal-update-list.test.ts,proposal-validation-rules.test.ts, andproposal-version.test.ts, with shared fixtures extracted to__tests__/helpers/proposal-fixtures.ts. As per coding guidelines: Files must not exceed 500 lines — split files when approaching the limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/engine/validation-engine.ts` around lines 1 - 705, The file validation-engine.ts has grown past the 500-line guideline; split it into smaller modules to keep files under the limit. Move the Phase 1 orchestration (validatePhase1 and its top-level helpers like schemaExists/actionExists/eventExists and the proposedSets logic) into validation-phase1.ts; extract low-level validators (validateEntity, validateAction, validateRule, validateStateDef, validateEventDef, validateViewDef) into validation-validators.ts (or validation-rules.ts) and put resolveStateMachine and any shared helper utilities into validation-helpers.ts; then update validateProposal to import validatePhase1 and the helpers and keep the phase orchestration here (or in validation-proposal.ts) so symbols like validatePhase1, validateEntity, validateAction, validateRule, validateStateDef, resolveStateMachine, and validateProposal are all exported/imported accordingly to preserve behavior and tests.Sources: Coding guidelines, Learnings
packages/core/__tests__/proposal-file-writer.test.ts (1)
1-702: ⚡ Quick winConsider splitting this test file to comply with the 500-line guideline.
The file has grown to 702 lines (40% over the 500-line limit). Based on learnings, oversized test files should be split into focused modules with shared fixtures extracted to a helpers file.
Suggested split:
proposal-file-writer-basic.test.ts(write operations, path resolution)proposal-file-writer-filename.test.ts(filename format tests)proposal-file-writer-formatter.test.ts(formatter option tests)proposal-file-writer-engine-hook.test.ts(ProposalEngine integration)__tests__/helpers/proposal-file-writer-fixtures.ts(shared fixtures likemakeApprovedProposal,viewChange, etc.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/__tests__/proposal-file-writer.test.ts` around lines 1 - 702, The test file exceeds the 500-line guideline (702 lines); split its concerns and shared fixtures into smaller modules: create separate test files for basic write/path tests (proposal-file-writer-basic.test.ts), filename-format tests (proposal-file-writer-filename.test.ts), formatter behavior tests (proposal-file-writer-formatter.test.ts), and engine integration tests (proposal-file-writer-engine-hook.test.ts), and extract common helpers (makeApprovedProposal, viewChange, selfContainedEntityChange, FIXED_NOW, todayUtcDateStamp, tmpDir setup/teardown, etc.) into a shared helpers module (e.g. __tests__/helpers/proposal-file-writer-fixtures.ts); update each new test file to import ProposalFileWriter and createProposalEngine from ../src/server-entry and the shared fixtures so tests referencing symbols like writeApprovedProposal, createProposalEngine, makeApprovedProposal, viewChange, and selfContainedEntityChange keep working.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/engine/code-quality-gate.ts`:
- Around line 20-32: Add a CI smoke test that validates the Bun transpiler shape
used by resolveBun/BunTranspilerLike: instantiate new Bun.Transpiler({ loader:
"ts" }) (or "tsx"), call transformSync on a valid TS/TSX snippet to assert a
string result and call transformSync on invalid syntax to assert it throws; run
this test across the Bun versions you support in your matrix so
checkSourceSyntax (or any caller that uses resolveBun) will break early if Bun’s
API changes.
---
Nitpick comments:
In `@packages/core/__tests__/proposal-file-writer.test.ts`:
- Around line 1-702: The test file exceeds the 500-line guideline (702 lines);
split its concerns and shared fixtures into smaller modules: create separate
test files for basic write/path tests (proposal-file-writer-basic.test.ts),
filename-format tests (proposal-file-writer-filename.test.ts), formatter
behavior tests (proposal-file-writer-formatter.test.ts), and engine integration
tests (proposal-file-writer-engine-hook.test.ts), and extract common helpers
(makeApprovedProposal, viewChange, selfContainedEntityChange, FIXED_NOW,
todayUtcDateStamp, tmpDir setup/teardown, etc.) into a shared helpers module
(e.g. __tests__/helpers/proposal-file-writer-fixtures.ts); update each new test
file to import ProposalFileWriter and createProposalEngine from
../src/server-entry and the shared fixtures so tests referencing symbols like
writeApprovedProposal, createProposalEngine, makeApprovedProposal, viewChange,
and selfContainedEntityChange keep working.
In `@packages/core/src/engine/validation-engine.ts`:
- Around line 1-705: The file validation-engine.ts has grown past the 500-line
guideline; split it into smaller modules to keep files under the limit. Move the
Phase 1 orchestration (validatePhase1 and its top-level helpers like
schemaExists/actionExists/eventExists and the proposedSets logic) into
validation-phase1.ts; extract low-level validators (validateEntity,
validateAction, validateRule, validateStateDef, validateEventDef,
validateViewDef) into validation-validators.ts (or validation-rules.ts) and put
resolveStateMachine and any shared helper utilities into validation-helpers.ts;
then update validateProposal to import validatePhase1 and the helpers and keep
the phase orchestration here (or in validation-proposal.ts) so symbols like
validatePhase1, validateEntity, validateAction, validateRule, validateStateDef,
resolveStateMachine, and validateProposal are all exported/imported accordingly
to preserve behavior and tests.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61a28ddc-3d1f-4f31-83f2-7f1d7bbea565
📒 Files selected for processing (13)
.changeset/g5-materialize-build.mdpackages/core/__tests__/barrel-public-surface.test.tspackages/core/__tests__/proposal-file-writer.test.tspackages/core/src/__tests__/engine/code-quality-gate.test.tspackages/core/src/__tests__/engine/proposal-materializer.test.tspackages/core/src/__tests__/engine/validation-phase2.test.tspackages/core/src/engine/code-quality-gate.tspackages/core/src/engine/proposal-file-writer.tspackages/core/src/engine/proposal-materializer.tspackages/core/src/engine/validation-engine.tspackages/core/src/engine/validation-phase2.tspackages/core/src/exports/server/engines.tspackages/core/src/types/proposal.ts
… Phase 4) (#496) * feat(adapter): proposal code materialization endpoint + review UI (G5 Phase 4) Wire the G5 materializer (#495) into a live, human-driven path. Server — POST /api/proposals/:id/materialize (proposal-materialize-api.ts): - Injectable orchestrator runProposalMaterialization: load draft → generate candidate source via a CodeGenerationProvider (over the configured AI) → Phase 2 syntax gate → persist generatedSource back onto the draft via ProposalEngine.updateProposal. Returns a discriminated outcome. - DRAFT-only (422 otherwise); CommandLayer permission slot runs FIRST and is never skipped; 503 when the command layer or AI provider is unconfigured. NEVER approves/graduates/writes-files/runs code — candidate only. UI — proposal-review page: - 'Materialize code' button on draft cards + outcome banner (materialized/ skipped/failed counts) and a read-only viewer of the generated candidate source per change. materializeProposal client mirrors the graduate client (discriminated result, injectable fetchImpl, encoded id). serializeProposal is exported for the route; it already carries changes (hence generatedSource) so the UI renders source with no extra fetch. Safety: candidate source flows through validation + double human review (draft review + graduation PR) before it can land. No scheduler/cadence. Tests: 11 server + 11 UI client. bun run test PASS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(adapter): defensive guards from gemini review on #496 - runProposalMaterialization: guard a null/undefined return from engine.updateProposal so serializeProposal never dereferences a missing proposal (mirrors the getProposal not-found guard). + test. - proposal-review: optional-chain mat.proposal.changes with a [] fallback so a partial API response can't crash rendering. - proposal-review: include change.operation in the generated-source list React key so a create+update on the same target/name can't collide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: refresh Review Threads after resolving review comments --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
G5 Phase 2 + 3 (engine) — generate and build-validate the irreducibly-code
part of a proposal. Today that is the
ActionDefinition.handlerbody, which adeclarative
ChangeDefinitioncannot express.materializeProposalChanges({ proposal, provider, qualityGate?, maxRetries?, context? })generates TypeScript source for materializable changes via a
CodeGenerationProvider(the feat(ai-provider,core): implement CodeGenerationProvider seam (G5 Phase 1) #494 cap-ai-provider impl), attaching it to thenew optional
ProposalChange.generatedSource. Generate → quality-gate →retry-with-feedback (default 3). Returns a COPY; never mutates input, writes
files, runs code, approves, or graduates. Scope is narrow + extensible:
MATERIALIZABLE_TARGETS = { action }(event is declarative;defineFlowdoesn't exist yet).
checkSourceSyntax()/createSyntaxQualityGate()validate generated source SYNTACTICALLY via Bun's transpiler (no project-aware
type resolution — that would false-positive on project symbol refs; left to the
graduation PR's CI).
validatePhase2()runs it over a proposal'sgeneratedSourceand is now wired intovalidateProposal(was a skipped stub).Warn-only by default;
ValidationContext.strictGeneratedBuildescalates toblocking. All-declarative proposals still see Phase 2 "skipped".
ProposalFileWriterwritesgeneratedSourceverbatim when present (preferred over deterministic codegen), so the AI handler
actually lands in the graduation PR instead of a stub. An empty/whitespace
source is treated as absent (falls back to the scaffold; never writes a blank
file) and is flagged by Phase 2.
Safety boundary
Candidate source only. It flows through validation (Phase 2) and double human
review (draft review + graduation PR) before it can land. "AI never modifies
production directly." Not yet wired into a live HTTP/draft path — a thin
follow-up (a materialize endpoint + review-UI surface).
Review
5 rounds of iterative codex review converged to clean. Findings fixed along the
way: graduation now consumes generatedSource (was dropped); scope narrowed to
action(event is declarative, defineFlow absent); empty source no longerbypasses Phase 2 or writes a blank file; stale source cleared before a failed
re-materialization.
Verification
bun run check/bun run typecheck— cleanbun run test— PASS (new unit tests for the gate, Phase 2, materializer, andthe writer's generatedSource consumption; fake provider — no real model called)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation