#114 Let reviewers emit gated insights into review artifacts - #1031
Merged
Conversation
Revise the Knowledge items section so insights (I{n}) may be emitted by reviewers, replacing the housekeeping-only "never emitted by code review skills" boundary while keeping insights non-severity and never merge-blocking.
Add an Insight gate as strict as the Actionability gate: an insight is non-obvious knowledge a future reader is materially worse off without, distinguished from a Suggestion by whether an action is implied. State it once in the canonical scheme and reference it from review-criteria.
Add a shared review-insight-gate partial for reviewer subagents to include.
Give the five code-review subagents (code, silent-failure, test, simplification, and orchestrated) an insight-gate include so each may emit I{n} insights alongside its findings.
Add the `### Insights` section to the shared review-writes scaffold so every reviewer's artifact carries it, and describe the finalized insight form in the finalize partial: gated per the insight gate, no severity, excluded from the criticality aggregate, and omitted when empty.
Add an `## Insights` section to the review-branch artifact body, placed after `## Areas for improvement` and before `## Technical assessment`, with I{n} items carrying a description and an optional destination. Insights are gated per the insight gate, carry no severity, and do not count against the score; the section is omitted when there are none.
Add the I{n} entry to the issue-numbering list and the 💡 icon to the section-header icon set. review-pr and its platform delegates inherit this body structure unchanged, since the review artifact is produced by review-branch's shared process.
Restructure the wrap-up insight step to read structured insight sources first — the `## Insights` / `### Insights` section of review artifacts and the run-summary — before conversation-scanning, mirroring the structured-source precedence the deferred-item step already uses. Conversation scanning skips any insight already captured from a structured source, so a review-emitted insight and its conversation echo collapse to one item with the structured copy winning. Resolve each insight's destination by honoring a reviewer-suggested destination when present, otherwise applying the existing ticket-comment-versus-devlog heuristic. This makes reviewer insights reach wrap-up in the non-orchestrated review flow, which has no run-summary and previously captured insights only heuristically from conversation prose.
Change the run-summary Insights section from free synthesis to structural aggregation: collect the I{n} insights emitted across the run's reviewer artifacts, dedup an insight several reviewers raised into one entry, and add an orchestrator-level observation only when no reviewer already captured it. The omit-when-empty behavior is unchanged, so the Phase 6 wrap-up trigger still fires only on a non-empty section.
Make wrap-up's two structured insight sources mutually exclusive by run type so no insight is harvested twice. An orchestrated run reads the run-summary `## Insights` aggregate, which already collects and dedups every reviewer-subagent artifact's insights; a non-orchestrated run reads the standalone review artifact, the only structured insight source when no orchestrated run exists. The conversation-scan dedup was structured-vs-conversation only, so when the run-summary aggregate overlapped the per-reviewer artifacts an orchestrated wrap-up listed each reviewer insight twice.
williamthorsen
added
feature
Dependency auditProduction dependency audit passed. |
williamthorsen
marked this pull request as ready for review
July 18, 2026 22:02
williamthorsen
added a commit
that referenced
this pull request
Aug 4, 2026
…e-v0.2.2 codeassembly-v0.4.0 factory-v0.2.2 codeassembly-mcp-v0.2.2 codeassembly-v0.4.0 - #1153 feat: Make codeassembly and kb CLI tools publishable (#1164) - #1091 fix: Anchor a project-deployed link where its target deploys (#1159) - #1156 refactor: Rename packages to publishable names (#1157) - #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155) - #1110 tooling: Migrate Vitest to nmr's centralized model (#1154) - refactor: Refine typescript-preferences - #1150 fix: State doc-description form and make comment mood opt-in (#1151) - deps: Upgrade first-party linting deps to latest version - #1137 feat: Establish personal rulebooks for code layout and TypeScript preferences (#1144) - #1126 feat: Check a package's own guidance content before it ships (#1138) - refactor: Fix lint - tooling: Remove redundant lint rules - #1133 fix: Reject an anchor link that names no heading (#1135) - #1122 feat: Capture lede decisions as an accumulating corpus (#1132) - #1125 feat: Honor invocation tokens in rulebook bodies (#1129) - #1107 feat: Render rulebook links and path tokens per harness (#1124) - #1115 feat: Sync guidance at build and install so an upgrade cannot leave it stale (#1123) - #1114 feat: Adopt a dependency's guidance by naming the package (#1121) - #1088 feat: Deliver project ambient rulebooks per harness (#1113) - #1087 refactor: Rename the authoring rulebook and mark its enforced rules (#1092) - #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102) - #1094 refactor: Remove the ambient ripgrep dependency from the test suite (#1097) - deps: Upgrade all deps to latest version - tooling: Upgrade all deps to latest version & modernize configs - refactor: Fix lint - #1077 feat: Show the proposed edit above post-review menu options (#1078) - #1068 feat: Deliver ambient rulebooks mechanically, retiring GLOBAL.md (#1075) - #1069 feat: Add a no-second-person rule to the lede-voice doctrine (#1070) - #1050 feat: Add a redundancy rule to the lede-voice doctrine (#1058) - #1035 internal: Add lifecycle workspace with the canonical envelope, vocabulary & lane fold (#1049) - #1006 internal: Extend lifecycle-event instrumentation to five high-traffic skills (#1034) - #1028 feat: Rule out absence-of-removed-code tests (#1032) - #114 feat: Let reviewers emit gated insights into review artifacts (#1031) - #1027 internal: Retire input.received and redundant skill.progress emits (#1030) - #1026 fix: Name the side effects an approval ask authorizes (#1029) - #1022 fix: Normalize action and question label identifiers across asks blocks (#1025) factory-v0.2.2 - #1156 refactor: Rename packages to publishable names (#1157) - deps: Upgrade all deps to latest version - #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155) - #1110 tooling: Migrate Vitest to nmr's centralized model (#1154) - tooling: Remove redundant lint rules - deps: Upgrade deps to latest version - deps: Upgrade all deps to latest version - deps: Upgrade all deps to latest version - tooling: Upgrade all deps to latest version & modernize configs - refactor: Fix lint - deps: Upgrade all deps to latest version kb-v0.3.0 - #1153 feat: Make codeassembly and kb CLI tools publishable (#1164) - #1156 refactor: Rename packages to publishable names (#1157) - #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155) - #1110 tooling: Migrate Vitest to nmr's centralized model (#1154) - tooling: Remove redundant lint rules - #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102) - tooling: Upgrade all deps to latest version & modernize configs - refactor: Fix lint codeassembly-lifecycle-v0.2.0 - #1153 feat: Make codeassembly and kb CLI tools publishable (#1164) - #1156 refactor: Rename packages to publishable names (#1157) - #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155) - #1110 tooling: Migrate Vitest to nmr's centralized model (#1154) - tooling: Remove redundant lint rules - #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102) - tooling: Upgrade all deps to latest version & modernize configs - #1038 feat: Add the read-only git adapter for worktree and base-branch ground truth (#1059) - #1051 feat: Bound fold memory and rescan cost with a retention window (#1057) - #1035 internal: Add lifecycle workspace with the canonical envelope, vocabulary & lane fold (#1049) codeassembly-mcp-v0.2.2 - deps: Upgrade all deps to latest version - #1156 refactor: Rename packages to publishable names (#1157) - #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155) - #1110 tooling: Migrate Vitest to nmr's centralized model (#1154) - #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102) - tooling: Upgrade all deps to latest version & modernize configs - refactor: Fix lint codeassembly-run-core-v0.2.2 - #1156 refactor: Rename packages to publishable names (#1157) - #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155) - #1110 tooling: Migrate Vitest to nmr's centralized model (#1154) - #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102) - tooling: Upgrade all deps to latest version & modernize configs - refactor: Fix lint
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Code-review agents are now permitted to record "insights" -- non-obvious knowledge worth preserving, such as a pattern, gotcha, or architectural learning that isn't an actionable finding -- in their reviews. Previously a reviewer who noticed such knowledge had to drop it or force it into a finding it didn't fit. The housekeeping steps that follow a review are now instructed to collect these insights, so the knowledge is carried forward instead of being rediscovered later.
Why
The finding scheme barred reviewers from producing insights, so non-actionable knowledge a reviewer surfaced was either dropped or forced into an
S/Rfinding it didn't fit — eroding the actionability discipline that keeps reviews lean. Outside orchestration, which is the large majority of work, that knowledge had no structured path into housekeeping and was recovered only by heuristically scanning conversation prose, even though findings were already harvested structurally from the same review artifact.Details
🎉 Features
I{n}insights alongside their findings, written into a dedicated section of each reviewer's artifact and excluded from the criticality aggregate.review-branchrenders an## Insightssection, inherited unchanged byreview-prand its platform delegates, that is criticality-free and excluded from the review score.wrap-upreads reviewer insights structurally, selecting one source per run type — the run-summary's deduplicated aggregate for an orchestrated run, the standalone review artifact otherwise — merged with conversation-scanned insights without duplication, and honoring a reviewer-suggested destination (ticket comment versus devlog) when present.## Insightssection now aggregates and deduplicates the insights emitted across the run's reviewer artifacts, adding an orchestrator-level observation only when no reviewer already captured it, in place of free-form synthesis.Closes #114