Public: anyone can read this; the crew can change it.
The guard records a cap once per issue per window, and the sentinel's parks signal names the capped issues #2319
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/2286-the-guard-records-a-cap-once-per-issue-p → main 1 commit fork 1 thread · 1 unresolved
3 files
changed +73
−8
src/crew/guard.ts +15 −2
| @@ -189,13 +189,25 @@ export function capNoted( | ||
| 189 | 189 | ); |
| 190 | 190 | } |
| 191 | 191 | |
| 192 | /** Record that `issue` was capped: once per park, beside the note. */ | |
| 192 | /** | |
| 193 | * Record that `issue` was capped: at most once per cap window, however | |
| 194 | * often the clock asks again. Returns whether a decision was written. | |
| 195 | */ | |
| 193 | 196 | export function recordCap( |
| 194 | 197 | db: Database, |
| 195 | 198 | repo: Repo, |
| 196 | 199 | issue: Issue, |
| 197 | 200 | cap: number, |
| 198 | ): void { | |
| 201 | now = new Date(), | |
| 202 | ): boolean { | |
| 203 | const seen = db | |
| 204 | .query<{ id: string }, [string, string]>( | |
| 205 | `SELECT id FROM decision | |
| 206 | WHERE issue_id = ?1 AND kind = 'guard' AND answer = 'capped' | |
| 207 | AND created_at >= ?2 LIMIT 1`, | |
| 208 | ) | |
| 209 | .get(issue.id, capFrom(db, repo, issue, now)); | |
| 210 | if (seen !== null) return false; | |
| 199 | 211 | recordDecision(db, { |
| 200 | 212 | kind: "guard", |
| 201 | 213 | repoId: repo.id, |
| @@ -206,6 +218,7 @@ export function recordCap( | ||
| 206 | 218 | by: "rule", |
| 207 | 219 | ms: 0, |
| 208 | 220 | }); |
| 221 | return true; | |
| 209 | 222 | } |
| 210 | 223 | |
| 211 | 224 | export interface GuardCounts { |
src/crew/sentinel.ts +20 −5
| @@ -235,20 +235,35 @@ export function ledgerStreaks(db: Database, now: number): Finding[] { | ||
| 235 | 235 | } |
| 236 | 236 | // A skip is a run the guard saved — nothing new to act on — not a run it |
| 237 | 237 | // refused; only the decisions that parked work are counted. |
| 238 | const parks = new Map<string, number>(); | |
| 238 | const parks = new Map<string, Set<string>>(); | |
| 239 | 239 | const skips = new Map<string, number>(); |
| 240 | 240 | for (const d of decisionsSince(db, "guard", new Date(now - DAY_MS).toISOString())) { |
| 241 | const into = d.answer === "skipped" ? skips : parks; | |
| 242 | into.set(d.repoId, (into.get(d.repoId) ?? 0) + 1); | |
| 241 | if (d.answer === "skipped") { | |
| 242 | skips.set(d.repoId, (skips.get(d.repoId) ?? 0) + 1); | |
| 243 | continue; | |
| 244 | } | |
| 245 | // A cap is one park per issue however often it was asked again. | |
| 246 | const set = parks.get(d.repoId) ?? new Set<string>(); | |
| 247 | set.add(d.issueId ?? d.id); | |
| 248 | parks.set(d.repoId, set); | |
| 243 | 249 | } |
| 244 | for (const [repoId, n] of parks) { | |
| 250 | for (const [repoId, issues] of parks) { | |
| 251 | const n = issues.size; | |
| 245 | 252 | if (n < 20) continue; |
| 253 | const ids = [...issues]; | |
| 254 | const numbers = db | |
| 255 | .query<{ number: number }, string[]>( | |
| 256 | `SELECT number FROM issue WHERE id IN (${ids.map(() => "?").join(",")}) ORDER BY number LIMIT 5`, | |
| 257 | ) | |
| 258 | .all(...ids) | |
| 259 | .map((r) => `#${r.number}`); | |
| 246 | 260 | const skipped = skips.get(repoId) ?? 0; |
| 247 | 261 | out.push({ |
| 248 | 262 | key: `parks:${repoId}`, |
| 249 | 263 | title: `The guard parked ${n} runs in ${repoSlug(db, repoId)} in a day`, |
| 250 | 264 | evidence: [ |
| 251 | `${n} \`guard\` decisions that parked a run since ${new Date(now - DAY_MS).toISOString()}`, | |
| 265 | `${n} \`guard\` decisions that parked distinct issues since ${new Date(now - DAY_MS).toISOString()}`, | |
| 266 | ...(numbers.length > 0 ? [`capped issues: ${numbers.join(", ")}`] : []), | |
| 252 | 267 | ...(skipped > 0 ? [`${skipped} skips of a run with nothing new are not counted`] : []), |
| 253 | 268 | ], |
| 254 | 269 | pillar: "Throughput: the crew spends on work, not on being refused", |
test/sentinel.test.ts +38 −1
| @@ -14,12 +14,13 @@ import { createRepo } from "../src/actions/repos.ts"; | ||
| 14 | 14 | import { setCapacity } from "../src/core/capacity.ts"; |
| 15 | 15 | import { clearErrors, createLog } from "../src/kernel/log.ts"; |
| 16 | 16 | import { recordDecision } from "../src/core/decisions.ts"; |
| 17 | import { addComment, createLabel, labelsFor, listComments, listIssues } from "../src/core/issues.ts"; | |
| 17 | import { addComment, createIssue, createLabel, labelsFor, listComments, listIssues } from "../src/core/issues.ts"; | |
| 18 | 18 | import { addCrew, createAgentRow } from "../src/core/agents.ts"; |
| 19 | 19 | import { listEvents } from "../src/core/events.ts"; |
| 20 | 20 | import { getRepoById, type Repo } from "../src/core/repos.ts"; |
| 21 | 21 | import { createRun } from "../src/core/runs.ts"; |
| 22 | 22 | import { createUser, type User } from "../src/core/users.ts"; |
| 23 | import { recordCap } from "../src/crew/guard.ts"; | |
| 23 | 24 | import { |
| 24 | 25 | budgetBreaches, |
| 25 | 26 | degradedCauses, |
| @@ -297,6 +298,42 @@ describe("the guard", () => { | ||
| 297 | 298 | }); |
| 298 | 299 | }); |
| 299 | 300 | |
| 301 | describe("a cap", () => { | |
| 302 | const issueOf = (n: number) => { | |
| 303 | const r = createIssue(app.db, { repoId: repo.id, authorId: alice.id, title: `capped ${n}` }); | |
| 304 | if (!r.ok) throw new Error("issue"); | |
| 305 | return r.value; | |
| 306 | }; | |
| 307 | ||
| 308 | test("is recorded once per issue per window, however often it is asked", () => { | |
| 309 | app.db.query("DELETE FROM decision WHERE kind = 'guard'").run(); | |
| 310 | const issue = issueOf(0); | |
| 311 | expect(recordCap(app.db, repo, issue, 6)).toBe(true); | |
| 312 | expect(recordCap(app.db, repo, issue, 6)).toBe(false); | |
| 313 | const n = () => | |
| 314 | app.db.query<{ n: number }, []>("SELECT COUNT(*) AS n FROM decision WHERE kind = 'guard' AND answer = 'capped'").get()?.n; | |
| 315 | expect(n()).toBe(1); | |
| 316 | // A later window is a new park. | |
| 317 | expect(recordCap(app.db, repo, issue, 6, new Date(Date.now() + 2 * DAY))).toBe(true); | |
| 318 | expect(n()).toBe(2); | |
| 319 | }); | |
| 320 | ||
| 321 | test("the parks signal counts distinct issues and names up to five", () => { | |
| 322 | app.db.query("DELETE FROM decision WHERE kind = 'guard'").run(); | |
| 323 | const issues = Array.from({ length: 20 }, (_, i) => issueOf(i + 1)); | |
| 324 | for (const i of issues) { | |
| 325 | recordCap(app.db, repo, i, 6); | |
| 326 | // A second decision for the same issue, as an old clock wrote them. | |
| 327 | recordDecision(app.db, { kind: "guard", repoId: repo.id, issueId: i.id, state: {}, answer: "capped", p: 1, by: "rule", ms: 0 }); | |
| 328 | } | |
| 329 | const parks = findFaults(app.db, Date.now()).find((x) => x.key === `parks:${repo.id}`); | |
| 330 | expect(parks?.title).toContain("parked 20 runs"); | |
| 331 | const line = parks?.evidence.find((e) => e.startsWith("capped issues:")) ?? ""; | |
| 332 | expect(line.split(",").length).toBe(5); | |
| 333 | expect(line).toContain(`#${issues[0]!.number}`); | |
| 334 | }); | |
| 335 | }); | |
| 336 | ||
| 300 | 337 | describe("the log", () => { |
| 301 | 338 | test("the same ERROR line three times is named once, with the line", () => { |
| 302 | 339 | clearErrors(); |
new above
offline
Binds one placeholder per capped issue; with very many capped issues this can hit the SQLite variable limit. Fix: take the lowest five ids in a subquery or slice before binding.