Skip to content

Commit 563edae

Browse files
Merge pull request #1360 from Gustav-Simonsson/improve_code_comments
refactor(comments): trim verbose comments across the codebase
2 parents 7af83cd + 7064344 commit 563edae

713 files changed

Lines changed: 12004 additions & 18880 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎e2e/compaction-atomicity.test.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ function mapArchive(dir: string) {
4242
});
4343
}
4444

45-
// A turn sequence carrying one tool_call/tool_result pair — the shape the
46-
// completeness gate checks for evidence coverage.
45+
// One tool_call/tool_result pair — the shape the completeness gate checks for coverage.
4746
function toolExchangeHistory(): ConversationTurn[] {
4847
return [
4948
turn("fact-a"),
@@ -182,9 +181,8 @@ describe("compaction atomicity", () => {
182181
const adopted1 = await first.apply(history1, ctx);
183182
expect(adopted1.record.reason).toBe("compact");
184183

185-
// The new spine never passes through inbound admission, so adoption must
186-
// leave it in the archive — otherwise the next fold rejects it as
187-
// uncovered (a repaired spine is never echoed verbatim).
184+
// The new spine never passes inbound admission, so adoption must leave
185+
// it in the archive.
188186
const handoffs = (await archive.listOccurrences()).filter(
189187
(occurrence) => occurrence.provenance === "compaction-handoff",
190188
);

‎e2e/compaction-baseline.test.ts‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,9 @@ async function snapshot(session: IntegrationSession) {
8585
return { hash: createHash("sha256").update(raw).digest("hex"), turns };
8686
}
8787

88-
// CL-8744: folded-away evidence lives in the fat handoff file, not the live
89-
// prompt. Every fold overwrites the same stable latest key (a per-fold key
90-
// would make each spine novel, which the completeness gate must reject), so
91-
// one read behind the spine's pointer recovers the whole cumulative record.
88+
// Folded-away evidence lives in the handoff file behind the spine's pointer:
89+
// every fold overwrites the same latest key, so one read recovers the whole
90+
// cumulative record.
9291
async function readHandoffFile(
9392
session: IntegrationSession,
9493
key: string,
@@ -265,8 +264,7 @@ describe("integration — compaction mechanics baseline", () => {
265264
skipPermissions: true,
266265
reactorGated: false,
267266
}),
268-
// CL-9007: pin a tiny tail budget so the calibrated growth volumes
269-
// still fold instead of fitting the default live tail.
267+
// Pin a tiny tail budget so the calibrated growth volumes still fold.
270268
compactionShape: { tailBudgetTokens: 10 },
271269
compactionCompletion: async (turns) => {
272270
const context = turns
@@ -384,14 +382,11 @@ describe("integration — compaction mechanics baseline", () => {
384382
folds.push(observation);
385383
expect(qualifyingFold(observation)).toBe(true);
386384
expect(summaryInputs.length).toBe(fold + 1);
387-
// CL-8744: the live prompt carries only the thin spine plus its
388-
// pointer — the spine's cumulative evidence echo keeps every
389-
// required marker inference-visible, so the responder still
390-
// recovers the full set from the reply itself.
385+
// The live prompt carries only the thin spine plus its pointer; the
386+
// cumulative evidence echo keeps markers recoverable from the reply.
391387
const recovered = recoverEvidence(reply);
392388
expect(recovered).toEqual([...REQUIRED_EVIDENCE]);
393-
// The full structured record lives in the fat handoff file behind
394-
// the spine's pointer: every section present, every marker verbatim.
389+
// The full record lives in the handoff file behind the spine's pointer.
395390
const handoffKey = spineHandoffKey(after.turns);
396391
expect(handoffKey).toBe(HANDOFF_LATEST_KEY);
397392
const fileText = await readHandoffFile(session, handoffKey as string);
@@ -414,9 +409,8 @@ describe("integration — compaction mechanics baseline", () => {
414409
.map((fact) => fact.id)
415410
.sort(),
416411
).toEqual(REQUIRED_EVIDENCE.map((fact) => fact.id).sort());
417-
// The live spine may grow with newly discovered tokens; dropped prior
418-
// spines are adopted into the evidence archive so the completeness
419-
// gate still certifies the fold (qualifyingFold above).
412+
// Dropped prior spines are adopted into the archive so the
413+
// completeness gate still certifies the fold.
420414
spineTexts.push(spineText(after.turns));
421415
expect(spineTexts[fold]?.startsWith(COMPACTED_PREFIX)).toBe(true);
422416
expect(spineTexts[fold]).toContain("Handoff:");

‎e2e/compaction-read-paging.test.ts‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ const WireRequest = type({
1515
messages: type({ role: "string", content: "unknown" }).array(),
1616
});
1717

18-
// claude-integration resolves a 200k context window, so a 200k-token usage
19-
// frame reports over the 120k auto-compaction threshold.
18+
// A 200k-token usage frame clears the 120k auto-compaction threshold.
2019
const TRIGGER_USAGE = {
2120
input: 200_000,
2221
output: 1,
@@ -41,16 +40,12 @@ function logLines(count: number): string {
4140
async function openFoldSession(): Promise<E2ESession> {
4241
const session = await openE2ESession({
4342
permissionGate: e2ePermissionGate(),
44-
// A tiny tail budget keeps the fold honest: the default 2500-token
45-
// budget would absorb this small scenario into the live tail and the
46-
// compactor would correctly no-op.
47-
// Budget fits the paged read windows in the live tail but not the bulky
48-
// pad turns, so the fold still fires.
43+
// A tiny tail budget keeps this small scenario from fitting the live tail;
44+
// the budget fits the read windows but not the bulky pad turns, so the
45+
// fold fires.
4946
compactionShape: { tailBudgetTokens: 500 },
50-
// A short handoff, not a prompt echo: complete() receives the summarizer
51-
// prompt turns, and a 4k echo gets truncated off the verify-repair tail
52-
// so the fold aborts. This scenario asserts on kept live bodies, not the
53-
// summary text.
47+
// A short handoff keeps the fold from aborting on a 4k echo; assertions
48+
// target the kept live bodies, not the summary text.
5449
compactionCompletion: async () =>
5550
"Goal: page through var/log/big.log. Next: keep reading remaining windows.",
5651
});
@@ -84,12 +79,9 @@ async function padTurns(session: E2ESession, count: number): Promise<void> {
8479
}
8580

8681
/**
87-
* One user turn that pages through the log and folds mid-turn: each scripted
88-
* read reply reports low usage so the governor stays disarmed until the
89-
* final call, whose 200k usage frame arms the tool.done intercept — the
90-
* compact+emit batch runs, the delivered continuation re-enters inference,
91-
* and the last scripted reply answers the post-fold request. Keeping every
92-
* read inside this turn is what lands the kept windows in the live tail.
82+
* One user turn that pages through the log and folds mid-turn: reads report
83+
* low usage until the final 200k frame arms the compact+emit batch; keeping
84+
* every read in this turn lands the kept windows in the live tail.
9385
*/
9486
async function readChainThenFold(
9587
session: E2ESession,
@@ -130,9 +122,8 @@ describe("e2e — automatic compaction keeps the read resume recipe", () => {
130122
);
131123
expect(spines).toHaveLength(1);
132124

133-
// Each kept window still carries its own body and resume recipe —
134-
// the summary echoed turn text only, so these strings can only come
135-
// from the live kept results.
125+
// Each kept window still carries its own body and resume recipe — the
126+
// summary echoed turn text only, so these strings come from the live results.
136127
expect(body).toContain("Use offset=40 to continue");
137128
expect(body).toContain("Use offset=60 to continue");
138129
expect(body).toContain("log line 25");
@@ -160,8 +151,7 @@ describe("e2e — automatic compaction keeps the read resume recipe", () => {
160151

161152
const body = await lastRequestBody(session);
162153
expect(body).toContain(COMPACTED_PREFIX);
163-
// The newest copy of the identical window keeps its body and notice;
164-
// exactly one older duplicate renders as a stub.
154+
// The newest duplicate stays whole; exactly one older one renders as a stub.
165155
expect(body).toContain("Use offset=40 to continue");
166156
const stubs = body.match(/omitted from context/g) ?? [];
167157
expect(stubs).toHaveLength(1);

‎e2e/crash-finalize.test.ts‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,8 @@ describe("integration — crash finalizes run.json", () => {
4343
const raw = readFileSync(runJsonPath, "utf8");
4444
const state = JSON.parse(raw) as RunState;
4545

46-
// The fixture also parks two unawaited straggler "running" snapshot
47-
// writes behind a test-only gate (setTestWriteGate) that it releases
48-
// only after the crash handler has flipped isCrashed(), guaranteeing
49-
// both are still queued — not dispatched to the kernel — at that
50-
// moment. Without the isCrashed() guard in saveState
51-
// (src/session/state.ts), one of those would win the rename() race
52-
// once released and this would read back "running".
46+
// The fixture parks two "running" writes behind setTestWriteGate until
47+
// isCrashed() flips, so no straggler rename() wins over "crashed".
5348
expect(state.status).toBe("crashed");
5449
expect(state.finishedAt).toBeGreaterThan(0);
5550
expect(state.error).toContain("simulated crash");
@@ -89,10 +84,7 @@ describe("integration — crash finalizes run.json", () => {
8984
expect(exitCode).toBe(1);
9085
expect(stderr).toContain("uncaughtException: Error: simulated crash");
9186

92-
// The bug this pins: the outgoing session's terminal "done" write must
93-
// not clear the active-run handle, or the crash below finds it null
94-
// and never writes a crashed record for the session actually running
95-
// at the time of the crash.
87+
// The terminal "done" write must not clear the active-run handle, or the crash writes no record.
9688
const outgoingRunJsonPath = join(
9789
sessionDir(cwd, sessionId, home),
9890
"run.json",
@@ -143,16 +135,10 @@ describe("integration — crash finalizes run.json", () => {
143135
"uncaughtException: Error: simulated crash during run-end write",
144136
);
145137

146-
// The bug this pins: finalizeRunState used to clear the active-run
147-
// handle only after its own saveState write resolved. With the
148-
// run-end write parked mid-flight (this fixture's gate never
149-
// releases), the handle stayed live for the entire window, so the
150-
// crash handler saw a live run and wrote a "crashed" record via
151-
// saveCrashState — which bypasses the gate — clobbering what should
152-
// have been a clean finish. Clearing the handle before the await
153-
// closes that window: the crash handler finds no active run and
154-
// writes nothing, so the last write to land is the one from the
155-
// initial saveState above ("running"), never "crashed".
138+
// finalizeRunState used to clear the active-run handle only after its
139+
// saveState write resolved; the crash handler then saw a live run and
140+
// wrote "crashed" via saveCrashState, clobbering the clean finish.
141+
// Clearing the handle before the await closes that window.
156142
const runJsonPath = join(stdout.trim(), "run.json");
157143
const raw = readFileSync(runJsonPath, "utf8");
158144
const state = JSON.parse(raw) as RunState;

‎e2e/credential-recovery.test.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,8 @@ describe("e2e — credential recovery switches the live source", () => {
7474
material: { secret: "backup-key" },
7575
},
7676
},
77-
// The production Corbits policy with a stubbed OAuth refresh —
78-
// the refresh succeeds, the retry is armed, the second 401 is
79-
// terminal. A real scheduler lets the zero-delay retry actually
80-
// elapse; the harness scheduler is inert.
77+
// Production retry policy with a stubbed OAuth refresh; a real
78+
// scheduler lets the zero-delay retry actually elapse.
8179
retryPolicy: createCorbitsRetryPolicy({
8280
providerId: PRIMARY.id,
8381
refreshCredential: async () => undefined,
@@ -145,9 +143,8 @@ describe("e2e — credential recovery switches the live source", () => {
145143
"no backup alternative offered",
146144
);
147145

148-
// Phase 2: the real switch path — setSources on the live agent,
149-
// arm on the real director, deliver the continuation, then pump
150-
// the replayed turn through the backup provider.
146+
// Phase 2: switch sources on the live agent, arm the continuation,
147+
// deliver, and pump the replayed turn through the backup provider.
151148
const pump = session.harness.run({ wallClockBudgetMs: Infinity });
152149
const acceptance = applyCredentialRecoverySelection({
153150
state: recovery,
@@ -180,8 +177,7 @@ describe("e2e — credential recovery switches the live source", () => {
180177
const backupHits = requests.filter((r) =>
181178
RequestURL.assert(r).url.includes(BACKUP_HOST),
182179
);
183-
// Two primary hits prove the refresh→retry path ran: an inert or
184-
// api-key provenance would terminal-fail after the first 401.
180+
// Two primary hits prove the refresh→retry path ran.
185181
expect(primaryHits).toHaveLength(2);
186182
expect(backupHits).toHaveLength(1);
187183
// The replay carries the original operator message, not a synthetic

‎e2e/git-push-scoped.test.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/**
2-
* `bin/git-push-scoped` must never write to global git config: not on a
3-
* normal push, not when two pushes race, and not when one is killed
4-
* mid-flight. Every test runs against an isolated HOME/GIT_CONFIG_GLOBAL
5-
* so a bug here can never touch the real machine's config.
2+
* bin/git-push-scoped must never write global git config, even under races
3+
* or mid-flight kills; every test runs with an isolated HOME so a bug can
4+
* never touch the real machine's config.
65
*/
76

87
import { spawn } from "node:child_process";
@@ -71,8 +70,7 @@ beforeEach(() => {
7170
"[user]\n\tname = Sentinel\n\temail = sentinel@example.com\n",
7271
);
7372

74-
// Stub `gh` on PATH so the script's `command -v gh` check passes without
75-
// depending on a real GitHub CLI install or credentials.
73+
// Stub gh on PATH so the command -v gh check passes without a real install.
7674
fakeBinDir = join(root, "fakebin");
7775
mkdirSync(fakeBinDir);
7876
const ghStub = join(fakeBinDir, "gh");
@@ -126,8 +124,7 @@ describe("git-push-scoped", () => {
126124
const remote = initBareRemote("remote-slow.git");
127125
const work = initWorkingRepo("work-slow", remote);
128126

129-
// A pre-receive hook that sleeps gives us a reliable window to kill the
130-
// push while it is in flight.
127+
// A sleeping pre-receive hook gives a reliable window to kill the push in flight.
131128
const hookPath = join(remote, "hooks", "pre-receive");
132129
writeFileSync(hookPath, "#!/usr/bin/env bash\nsleep 5\n");
133130
chmodSync(hookPath, 0o755);
@@ -144,8 +141,7 @@ describe("git-push-scoped", () => {
144141
child.on("exit", (code, signal) => resolve({ code, signal }));
145142
});
146143

147-
// The pre-receive hook sleeps 5s, so the push is still in flight here;
148-
// kill it well inside that window.
144+
// The hook sleeps 5s, so the push is still in flight; kill it inside that window.
149145
await new Promise((resolve) => setTimeout(resolve, 30));
150146
child.kill("SIGKILL");
151147
const { signal } = await exited;

‎e2e/harness.ts‎

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
/**
22
* End-to-end scenario harness: full agent-loop runs against the production
3-
* stack — live tool dispatch, chat director, posix tools plus permission
4-
* middleware, git-backed context store, production compactor — with only
5-
* inference scripted by @intx/inference-testing. `e2e/integration-harness.ts` owns
6-
* the assembly; this layer adds fixture-repo seeding and small scenario
7-
* conveniences so e2e files stay declarative.
3+
* stack (tools, director, permissions, context store, compactor) with only
4+
* inference scripted by @intx/inference-testing.
85
*
9-
* Deliberate boundary: no TUI and no process spawn here — a scenario drives
10-
* `agent.send` through the same seam the runners use. PTY-level coverage is
11-
* a separate layer and out of scope for this harness.
6+
* Layer split: `integration-harness.ts` owns the assembly; this file adds
7+
* fixture-repo seeding and scenario conveniences so e2e files stay
8+
* declarative. No TUI or process spawn: scenarios drive `agent.send` through
9+
* the seam the runners use.
1210
*/
1311

1412
import { cpSync, mkdirSync, writeFileSync } from "node:fs";
@@ -46,15 +44,14 @@ export type E2ESession = IntegrationSession;
4644

4745
export interface OpenE2ESessionOpts extends OpenIntegrationSessionOpts {
4846
/**
49-
* Name of a `fixtures/<name>` directory copied into the session cwd
50-
* before the scenario runs. `node_modules` and `.git` are skipped: fixture
51-
* deps install into the real repo, and the repo marker belongs to the
52-
* `git` option, not to whatever the fixture happens to contain.
47+
* Name of a `fixtures/<name>` directory copied into the session cwd.
48+
* `node_modules` and `.git` are skipped: fixture deps install into the
49+
* real repo, and the repo marker belongs to the `git` option.
5350
*/
5451
fixture?: string;
5552
/**
56-
* Initialize the session cwd as a git repository. Defaults to true — a
57-
* "fixture-repo run" should look like one to worktree-aware permission
53+
* Initialize the session cwd as a git repository; defaults to true.
54+
* A fixture-repo run should look like one to worktree-aware permission
5855
* and trust checks. Pass false only when the scenario asserts on
5956
* non-repo behavior.
6057
*/
@@ -85,7 +82,7 @@ export function e2ePermissionGate(): PermissionGate {
8582
});
8683
}
8784

88-
/** Write (or overwrite) a file inside the session cwd mid-scenario. */
85+
/** Write a file inside the session cwd mid-scenario. */
8986
export function seedFile(
9087
session: E2ESession,
9188
relativePath: string,
@@ -100,16 +97,15 @@ export interface OperatorTurn {
10097
/** The exact message sent — recovery flows key on its identity. */
10198
message: InboundMessage;
10299
events: ReactorEmittedEvent[];
103-
/** The raw send result — a terminally-failed turn is not asserted here. */
100+
/** The raw send result; failures are returned, not thrown. */
104101
outcome: SendOutcome | unknown;
105102
}
106103

107104
/**
108105
* Send one operator message and pump the harness until the send settles,
109-
* whatever the outcome. Unlike runUntilDone this never asserts a reply —
110-
* scenarios that script a terminal failure (credential recovery, provider
111-
* outage) need the failure's event stream plus the send outcome, not a
112-
* thrown expectation.
106+
* whatever the outcome. Never asserts a reply (unlike `runUntilDone`):
107+
* terminal-failure scenarios need the failure's event stream plus the send
108+
* outcome.
113109
*/
114110
export async function sendOperatorTurn(
115111
session: E2ESession,
@@ -130,10 +126,9 @@ export async function sendOperatorTurn(
130126
};
131127
const events: ReactorEmittedEvent[] = [];
132128
const continuationGate = createContinuationGate();
133-
// The collector outlives the send: recovery scenarios deliver a follow-up
134-
// turn after this returns, and its events keep appending to `events`. The
135-
// catch mirrors runUntilSuspended — a stream error must not become an
136-
// unhandled rejection in the shared test process.
129+
// The collector outlives the send so recovery follow-up events keep
130+
// appending; the catch keeps stream errors from becoming unhandled
131+
// rejections in the shared test process.
137132
void (async () => {
138133
for await (const event of session.agent.stream()) {
139134
events.push(event);
@@ -161,9 +156,9 @@ export interface ScriptedReply {
161156

162157
/**
163158
* Queue one model response per inference request, in order — the common
164-
* "model calls tools, then answers" scenario spine. Thin sugar over
165-
* `scenario.replyOnce`; reach for `whenRequestBodyMatches`/`createStream`
166-
* directly when a reply must key on request content or timing.
159+
* "model calls tools, then answers" spine. Use
160+
* `whenRequestBodyMatches`/`createStream` directly when a reply must key on
161+
* request content or timing.
167162
*/
168163
export function scriptReplies(
169164
session: E2ESession,

0 commit comments

Comments
 (0)