feat(code-reviewer) add council review type data model#4500
Conversation
…gy enums against @kilocode/db/schema-types (single source of truth), and defer the council config-save guard to the PR that wires the config-save path.
| platform_project_id: params.platformProjectId ?? null, | ||
| manual_config: params.manualConfig ?? null, | ||
| review_type: params.reviewType ?? 'standard', | ||
| trigger_source: params.triggerSource ?? null, |
There was a problem hiding this comment.
WARNING: New reviews discard their known trigger provenance
Every current production caller omits triggerSource, including manual jobs and the GitHub, GitLab, and Bitbucket webhook paths, so this fallback writes NULL for all new reviews. That makes known manual/webhook origins indistinguishable from legacy rows and leaves the new field unpopulated; have those creation paths pass manual or webhook explicitly.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Fixed in 34a03ae. All five code-review creation paths now pass an explicit triggerSource, so new rows record their origin instead of writing NULL:
lib/code-reviews/manual-code-review-jobs.ts→manuallib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts→webhooklib/integrations/platforms/gitlab/webhook-handlers/merge-request-handler.ts→webhooklib/integrations/platforms/bitbucket/manual-code-review-trigger.ts→manualapp/api/webhooks/bitbucket/[integrationId]/route.ts→webhook
The ?? null fallback in codeReviewInsertValues stays only as a safety net for any future/unknown caller. Regression coverage added: the GitHub and GitLab handler tests assert createCodeReview is called with triggerSource: 'webhook', and the Bitbucket webhook route test asserts the persisted row has trigger_source: 'webhook'. The latest review reflects this file as 0 issues.
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (19 files)
Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit 34a03ae)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 34a03ae)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (18 files)
Fix these issues in Kilo Cloud Previous review (commit c7cf98a)Status: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Reviewed by gpt-5.6-sol-20260709 · Input: 138.9K · Output: 17.7K · Cached: 1.7M Review guidance: REVIEW.md from base branch |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE "cloud_agent_code_reviews" ADD COLUMN "review_type" text DEFAULT 'standard' NOT NULL;--> statement-breakpoint | |||
There was a problem hiding this comment.
**[WARNING]: Migration can block the live reviews table
Each ALTER TABLE needs an ACCESS EXCLUSIVE lock on the populated cloud_agent_code_reviews table. The constant default avoids a table rewrite on modern PostgreSQL, but the migration can still queue behind long-running traffic and then block all reads and writes when it acquires the lock. Use the repository's deployment-safe lock strategy, such as a short lock_timeout with retry or an explicitly controlled maintenance window.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Not addressing this one, by design. A few reasons:
-
No such convention exists in this repo. None of the 97+
ADD COLUMNmigrations underpackages/db/src/migrations/uselock_timeout,SET LOCAL, or a retry wrapper — includingmain's own same-day0182_powerful_blacklash.sql. There is no "repository deployment-safe lock strategy" to adopt here. -
It would violate a hard repo rule.
packages/db/AGENTS.md: "Do not hand-write or edit generated DDL, snapshots, or journal entries." This SQL is Drizzle-generated; adding lock pragmas means editing generated DDL. Only intentional data backfills may be appended after the generated DDL. -
Not specific to this change. On our PostgreSQL, an
ADD COLUMNwith a constant default is a metadata-only operation, so theACCESS EXCLUSIVElock is held briefly. The 'queue behind a long-running transaction' risk applies identically to everyADD COLUMNmigration in the repo; it's a fleet-wide operational/tooling posture, not something to bolt onto one feature migration.
If we want lock-safe migrations, that's a separate change to the migration tooling/convention that should apply to all migrations, tracked on its own.
…terprise plan label
Summary
Adds the data model and entitlement foundation for enterprise "council" (multi-specialist) code reviews. This does not run council reviews yet; it introduces the persisted shape, the review-run type, and a single enterprise-only entitlement gate so later PRs can build execution on top.
Changes
0182adds two columns tocloud_agent_code_reviews:review_type(text NOT NULL DEFAULT 'standard') and nullabletrigger_source. Reflected inpackages/db/src/schema.tsand the generated snapshot/journal.packages/db/src/schema-types.ts:CodeReviewType(standard|council),CodeReviewTriggerSource(manual|webhook), specialist roles,CouncilVote,CouncilAggregationStrategy,CouncilSpecialist, andCodeReviewCouncilConfig. Adds an optionalcouncilfield toCodeReviewAgentConfigSchema.council-entitlement.tswith the entitlement predicate (isCouncilEntitledPlan, enterprise-only) andassertCouncilCreationAllowed, a single enforcement point that throwsFORBIDDENwhen acouncilreview is created without entitlement. Local development bypasses the gate.assertCouncilCreationAllowedinto both review-creation paths (createCodeReviewandcreateCodeReviewIfAbsentInTransaction) and persistsreview_type/trigger_sourceviacodeReviewInsertValues.CreateReviewParamsSchemawith optionalreviewTypeandtriggerSource.@kilocode/worker-utils/review-agents.tsto importCODE_REVIEW_TYPESandCOUNCIL_AGGREGATION_STRATEGIESfrom@kilocode/db/schema-types, so the stored council config and the code-reviewer to cloud-agent wire contract share one source of truth.Verification
code-review-statusroute test fixture. Migration0182is a metadata-only column add (constant default), so no backfill runs.Visual Changes
N/A
Reviewer Notes
aggregation_strategyenum intentionally omitsweighteduntil per-specialist vote weights exist in the config.assertCouncilConfigAllowed) is intentionally deferred to the PR that adds a council config-save path, so it lands with its first caller instead of as an unused export. Running a council review is already gated at the creation boundary.