Skip to content

feat(core): wire review rules + settings into live reviewSummary (T-CR-501/502)#63

Merged
matze4u merged 2 commits into
mainfrom
feat/code-review-settings-rules-wiring
Jun 2, 2026
Merged

feat(core): wire review rules + settings into live reviewSummary (T-CR-501/502)#63
matze4u merged 2 commits into
mainfrom
feat/code-review-settings-rules-wiring

Conversation

@matze4u

@matze4u matze4u commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

What

Wires the shipped-but-dead road-to-code-review Phase-5 seams into the live IDE
review path. GitHandler.reviewSummary (the entry point an IDE "Review changes"
action calls) ran the review engine with hard-coded defaults and never read the
workspace review config — loadReviewRules and the ReviewSettings helpers
(resolveReviewSettings / voteOptionsFromSettings / applySeverityFloor) were
fully tested with zero live callers.

Changes

  • T-CR-501 (rules). reviewSummary loads .event4u-agent/review-rules.md
    best-effort and passes it as pipeline.rules, so a project's review rules
    reach the Stage-1 system prompt.
  • T-CR-502 (settings). New review/settings-source.ts::loadReviewSettings
    reads .agent-settings.yml :: review (fail-open to schema defaults; does
    not extend AgentSettingsSchema → no config → review coupling). Its
    values drive runReview's vote options and the severity floor, applied to
    issues + potentialIssues before summarizeReview so the counts, top
    findings, and potential tally stay consistent.
  • Both readers are optional injected GitHandlerDeps (default to the real file
    readers) — the handler stays pure and unit-testable.

Safety

The severity floor never hides a security finding (security_always_error
exemption) — the trap both council members flagged. A regression test asserts a
low-severity security finding survives a high floor while a low-severity bug
is dropped. Unconfigured workspaces are unchanged (no rules block; default
settings = groupSize 5, floor info).

AI Council (gemini-cli 0.41.2 + codex-cli 0.134.0, 2026-06-02)

  • Q0=A (unanimous) — wire rules + settings together.
  • Q2=A (unanimous) — apply the floor before summarising.
  • Q3=A (unanimous) — unconditional best-effort load (readers fail-open).
  • Q4=A (unanimous) — inject optional loadReviewRules?/loadReviewSettings?
    readers into GitHandlerDeps.
  • Q1 (settings source) — divergence, synthesised. gemini chose inject-into-deps,
    codex chose review-owned-no-coupling. Resolution: the default reader lives in
    review/settings-source.ts (review-owned, no schema coupling) and is
    injected as a dep (handler stays pure). See ADR-051.

Verification

  • task ci exit 0 — lint, format (incl. markdown), build, typecheck, test
    (core 1141 pass / 1 skip; +11 new tests).
  • Pure-core TypeScript: no protocol / DTO / codegen change → JetBrains + package
    matrix jobs unaffected.
  • No checkbox flip — T-CR-501/502 were already [x]; this lands the live
    wiring their cores were built for. Done-comments updated to cite it.

ADR-051.

matze4u and others added 2 commits June 2, 2026 14:06
…R-501/502)

GitHandler.reviewSummary ran the review engine with hard-coded defaults and
never loaded the workspace review config — the shipped, tested Phase-5 seams
(loadReviewRules, ReviewSettings helpers) had zero live callers. Thread both
into the live IDE review path:

- Project `.event4u-agent/review-rules.md` now feeds the Stage-1 prompt via
  pipeline.rules (T-CR-501).
- New review/settings-source.ts loadReviewSettings() reads
  `.agent-settings.yml :: review` (fail-open to schema defaults, no
  AgentSettingsSchema coupling). Its values drive runReview's vote options
  (voteOptionsFromSettings) and the severity floor (applySeverityFloor),
  applied to issues + potentialIssues BEFORE summarizeReview so all counts
  stay consistent (T-CR-502).

Both readers are optional injected GitHandlerDeps (default to the real file
readers) so the handler stays pure and unit-testable. The severity floor never
hides a security finding (security_always_error exemption), regression-tested.

Pure-core, additive: no protocol/DTO/codegen change. Council (gemini + codex,
2026-06-02): Q0/Q2/Q3/Q4 unanimous; Q1 (settings source) synthesised. ADR-051.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Record the decision for threading the dead Phase-5 review config/rules seam
into the live reviewSummary path: the Q0/Q2/Q3/Q4-unanimous council answers,
the Q1 settings-source synthesis (review-owned fail-open reader injected as a
dep, no AgentSettingsSchema extension), and the security-exemption trap both
members flagged. Add the index row and update the T-CR-501/502 done-comments
to cite the live wiring (no checkbox flip — both were already [x]).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@matze4u matze4u merged commit 653dedb into main Jun 2, 2026
8 checks passed
@matze4u matze4u deleted the feat/code-review-settings-rules-wiring branch June 2, 2026 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant