Skip to content

Commit 62d1086

Browse files
authored
Merge pull request #910 from taras/agent/comment-review-followup
📝 Clarify execution contracts and repair compiled PR review
2 parents 6314e18 + a61e35e commit 62d1086

35 files changed

Lines changed: 5128 additions & 4217 deletions

‎.reviews/ReviewPR.local.md‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,7 @@ const changedTsFiles = pr.files
2323
as="rawDiagnostics"
2424
/>
2525

26-
```ts eval
27-
import { buildDiagnostics } from "@executablemd/code-review-agent";
28-
29-
const diagnostics = buildDiagnostics(rawDiagnostics, pr, doctor);
30-
```
26+
<ReviewDiagnostics raw={rawDiagnostics} pr={pr} doctor={doctor} as="diagnostics" />
3127

3228
<ReleaseSpecWarning files={changedFilePaths} />
3329

‎.reviews/ReviewPR.md‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,7 @@ const changedTsFiles = pr.files
2525
as="rawDiagnostics"
2626
/>
2727

28-
```ts eval
29-
import { buildDiagnostics } from "@executablemd/code-review-agent";
30-
31-
const diagnostics = buildDiagnostics(rawDiagnostics, pr, doctor);
32-
```
28+
<ReviewDiagnostics raw={rawDiagnostics} pr={pr} doctor={doctor} as="diagnostics" />
3329

3430
<ThinkFilter>
3531
<DeepInfraProvider model="Qwen/Qwen3-30B-A3B">

‎AGENTS.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,9 @@ the corpus or enabling in-shard concurrency is not an answer to a miss.
405405
## MUST READ
406406

407407
- https://github.com/thefrontside/effection/blob/v4/AGENTS.md
408-
- `architecture.md` — read fully
408+
- [`architecture.md`](architecture.md) — read fully. Its [reading map](architecture.md#reading-this-architecture)
409+
locates current invariants and their feature-interface owners; it does not
410+
replace the full reading.
409411
- `specs/executable-mdx-spec.md` — read the sections your change touches
410412

411413
## Code Rules

‎README.md‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,21 @@ Three documents own the rest:
2222
- [`specs/executable-mdx-spec.md`](specs/executable-mdx-spec.md) — what the
2323
product does, with the conformance matrix that pins it.
2424

25+
Follow a question into the [architecture reading map](architecture.md#reading-this-architecture):
26+
27+
- [Execution and diagnostic journals](architecture.md#a-document-running): what
28+
runs, what a Plan produces, and which journal can continue a workflow.
29+
- [Partial and completed replay](architecture.md#replay-and-effect-transactions):
30+
why a historical file read survives a later deletion.
31+
- [Native Agent ownership](architecture.md#native-agent-sessions): handoff,
32+
release and executable compatibility.
33+
- [Installed structural regions](architecture.md#the-installed-structural-syntax-boundary):
34+
demand, output and cancellation.
35+
- [Plan recovery](architecture.md#typed-candidate-recovery-around-evaluate): why
36+
cleanup failure stops a candidate retry.
37+
- [Current rules and historical decisions](specs/decisions.md#finding-the-current-rule):
38+
where the architecture, feature contracts and vision differ in authority.
39+
2540
Locate the package that owns the behavior you are about to change:
2641

2742
- `packages/core/` — the engine: the boundary scanner, component expansion,
@@ -33,7 +48,7 @@ Locate the package that owns the behavior you are about to change:
3348
- `packages/durable-streams/` — the journal protocol, replay and divergence.
3449
- `packages/workflow/` — workflow runs, run storage, and the Workspace.
3550
- `packages/git/` — repository collaboration: `<Repository>`, `<Worktree>`,
36-
`<Dir>`, the `Git.*` operations, pull requests and issues, and the GitHub
51+
the `Git.*` operations, pull requests and issues, and the GitHub
3752
adapter behind them. It is the one Plugin `xmd` bundles, active by default for
3853
the run-profile commands — `run`, `plan`, `syntax`, and `workflow start`,
3954
`resume` and `fork`. The `xmd test` root is deliberately not one of them, so a

‎architecture.md‎

Lines changed: 4184 additions & 3550 deletions
Large diffs are not rendered by default.

‎packages/cli/src/plan-writer-profile.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
* with nothing in it. `--approve-all`, `--approve-reads` and `--deny-all`
2323
* configure the approved document, later, and reach none of this.
2424
*
25-
* The document itself is given no Files, command, service or XMD-mediated
26-
* network capability either. It decides what to write; it writes nothing.
25+
* The trusted document reads through the ordinary Files provider and effective
26+
* Permissions. This frame refuses Files mutations, Process, Service and Fetch.
2727
*/
2828

2929
import { ensure, Err, Ok, scoped, until, useScope } from "effection";

‎packages/cli/tests/plan-component.test.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -767,17 +767,11 @@ describe("Tier PC — <Plan> in an ordinary document", () => {
767767
/**
768768
* Cancelling an information read, with its cleanup held open.
769769
*
770-
* The grant is live exactly across `tryContent()` and the teardown the
771-
* projection joins, so the interesting interruption is one that lands
772-
* *inside* the provider: the read is in flight, a cleanup registered with it
773-
* holds, and the halt may not be allowed to complete until that cleanup
774-
* finishes. `joined` read while the cleanup is still held is what proves it
775-
* waited — a flag checked afterwards would be true either way.
776-
*
777-
* What it shows about access is what a cancelled run can show: nothing
778-
* further is performed under the grant. The retained-guard case — a guard
779-
* answering no once its frame has ended — is Tier PLA's, where a scope can
780-
* outlive what it was asking about.
770+
* The interruption lands inside the provider: the read is in flight and
771+
* its registered cleanup is held open. The projection joins that teardown,
772+
* so halting the run must wait until the cleanup finishes. Reading `joined`
773+
* before releasing the cleanup proves the halt waited — a flag checked
774+
* afterwards would be true either way.
781775
*/
782776
it("PI27: cancelling an entered information read waits for its cleanup", function* () {
783777
yield* useWorkingDirectory(function* () {

‎packages/code-review-agent/src/components.md‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
Long-form documentation for the code-review agent's TypeScript components.
22

3-
Six components that gather the evidence a review reasons about: the pull request
4-
and its diff, the checkout's readiness, the Oxlint sensor's diagnostics, the
3+
Seven components gather or group the evidence a review reasons about: the pull
4+
request and its diff, the checkout's readiness, the Oxlint sensor's diagnostics, the
55
repository's source inventory, and the review comments already on the pull
6-
request. Each one *reads* — none of them writes a comment, pushes a branch or
7-
changes a file.
6+
request. They read or transform evidence without writing a comment, pushing a
7+
branch or changing a file.
88

9-
They are reserved: this package claims these six names, so a repository under
9+
They are reserved: this package claims these seven names, so a repository under
1010
review cannot supply its own `ReviewContext.ts` and have a review run it. The
1111
Markdown components that compose them are declared by this package for the same
1212
reason, and are documented by their own bodies rather than here.
1313

14-
Every one of them runs against the working directory. A review of another
15-
checkout establishes that checkout first — with `<Worktree>` or `<Dir>` — and
16-
these report on whatever is current.
14+
The evidence-gathering components run against the working directory. A review
15+
of another checkout establishes that checkout first — with `<Worktree>` or
16+
`<Dir>` — and these report on whatever is current.
1717

1818
## ReviewContext
1919

@@ -67,6 +67,18 @@ cannot use. A silent empty result would be indistinguishable from a clean run,
6767
and a review that reported "no findings" because its sensor died is worse than
6868
one that stopped.
6969

70+
## ReviewDiagnostics
71+
72+
Group normalized Oxlint diagnostics for review policies.
73+
74+
```mdx
75+
<ReviewDiagnostics raw={rawDiagnostics} pr={pr} doctor={doctor} as="diagnostics" />
76+
```
77+
78+
Groups the sensor results by rule, applies the Doctor's import-noise filtering,
79+
and computes categories, density and summary through the package's existing
80+
`buildDiagnostics` helper. This transformation uses only its supplied values.
81+
7082
## RepositoryInventory
7183

7284
Counts what is in the repository, for a review that reasons about the whole tree
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import type { Operation } from "effection";
2+
import type { Diagnostics, DoctorResult, OxlintDiagnostic, PR } from "../types.ts";
3+
import { buildDiagnostics } from "../parse-diagnostics.ts";
4+
import { returns as rawDiagnosticsSchema } from "./OxlintDiagnostics.ts";
5+
import { returns as doctorSchema } from "./Doctor.ts";
6+
7+
export const props = {
8+
type: "object",
9+
properties: {
10+
raw: rawDiagnosticsSchema,
11+
pr: { type: "object" },
12+
doctor: doctorSchema,
13+
},
14+
required: ["raw", "pr", "doctor"],
15+
additionalProperties: false,
16+
};
17+
18+
const groupSchema = {
19+
type: "object",
20+
properties: {
21+
ruleId: { type: "string" },
22+
count: { type: "number" },
23+
files: { type: "array", items: { type: "string" } },
24+
instances: rawDiagnosticsSchema,
25+
},
26+
required: ["ruleId", "count", "files", "instances"],
27+
additionalProperties: false,
28+
};
29+
const groupsSchema = { type: "array", items: groupSchema };
30+
31+
export const returns = {
32+
type: "object",
33+
properties: {
34+
groups: groupsSchema,
35+
total: { type: "number" },
36+
fileCount: { type: "number" },
37+
ruleCount: { type: "number" },
38+
byCategory: {
39+
type: "object",
40+
properties: {
41+
structural: groupsSchema,
42+
verbosity: groupsSchema,
43+
typeAware: groupsSchema,
44+
other: groupsSchema,
45+
},
46+
required: ["structural", "verbosity", "typeAware", "other"],
47+
additionalProperties: false,
48+
},
49+
summary: { type: "string" },
50+
density: { type: "number" },
51+
},
52+
required: ["groups", "total", "fileCount", "ruleCount", "byCategory", "summary", "density"],
53+
additionalProperties: false,
54+
};
55+
56+
interface ReviewDiagnosticsProps {
57+
raw: OxlintDiagnostic[];
58+
pr: PR;
59+
doctor: DoctorResult;
60+
}
61+
62+
export default function* ReviewDiagnostics({
63+
raw,
64+
pr,
65+
doctor,
66+
}: ReviewDiagnosticsProps): Operation<Diagnostics> {
67+
return buildDiagnostics(raw, pr, doctor);
68+
}

‎packages/code-review-agent/src/plugin.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Install the packaged review graph when an operator selects this Plugin.
33
*
44
* Run, syntax, plan and executing workflow actions reserve its thirty-five
5-
* Markdown declarations and six reserved registrations, so the repository being
5+
* Markdown declarations and seven reserved registrations, so the repository being
66
* reviewed cannot replace the review program. Root `xmd test` reserves none;
77
* its isolated `<Execution host="run">` child reinstalls this Plugin with
88
* command `run`.
@@ -100,7 +100,7 @@ const reviewPlugin: Plugin = Plugin({
100100
if (!claimsGraph(request)) {
101101
return undefined;
102102
}
103-
// The six reserved registrations and the documentation describing them, in
103+
// The seven reserved registrations and the documentation describing them, in
104104
// the scope the command runs in. Declarative only: nothing here installs a
105105
// provider, spawns a process, reads a credential or reaches the network.
106106
yield* useReviewComponents();

0 commit comments

Comments
 (0)