fix(decision): pace the effort backfill on drain; dry run opens a copy; docs eligibility (TASK-3921; r3 M1, L2, L4)

- M1: the backfill owes one tick pass of items per chunk (20 default, wired
  from the tick limit) and owes chunk k+1 only once chunk k's own jobs have
  drained: none claimable now, by ClaimDecisionJobs' own predicate
  (new Store.CountClaimableDecisionJobs). Debounced jobs do not hold it.
  The wait polls at the tick interval, honours ctx, and a cancel writes
  no marker; the marker still follows the last chunk being owed.
- L2: TestEffortBackfillDryRunOnFile copies the named file under GOTMPDIR
  and opens only the copy.
- L4: docs/api.md states the real eligibility and the drain pacing.

Claude-Session: https://claude.ai/code/session_018oQ5EErQKHtBJK8yLpcDNF
This commit is contained in:
xarmian
2026-10-19 13:44:32 +00:00
parent bac8346aeb
commit 51c5458509
8 changed files with 370 additions and 6 deletions
+1 -1
View File
@@ -70,7 +70,7 @@ REST API at `/api/v1/`. Key endpoints:
- `PATCH/DELETE /workspaces/{ws}/reminders/{id}`, `POST /workspaces/{ws}/reminders/{id}/ack` — re-arm (clears both fire marks), disarm, acknowledge. Permission is the ITEM's; the reminder has no separate owner
- `POST /workspaces/{ws}/items/{slug}/claim` and `/release`: the item execution lease (#1221). Body `{holder?, worker_id?, ttl_seconds?}` (ttl on claim only: default 900, 1 to 86400, else 400). Needs edit permission and a signed-in user. A re-claim by the live holder extends the expiry and keeps `acquired_at` (the heartbeat). Release is idempotent: an absent or expired lease answers `released: false`. Expiry is absence on every read, with no reaper. **Identity (TASK-3613):** a lease is held under a WORKER ID plus the account. `worker_id` is `"wk_"` + 32 lowercase hex (128 random bits) that the client mints once per agent session (the CLI stores it in its session registry and sends it as `X-Pad-Worker`); a malformed one is `400 invalid_worker_id`. It comes from the body, else `X-Pad-Worker`, else a `?worker_id=` query, else, for a remote MCP call, an id derived from the account and the MCP session. A lease with a worker id is the caller's iff the worker id AND the account match; `holder` (default: the `X-Pad-Agent` name, else the email) is then display only. A lease without one (a name-only caller, or one claimed before migration 142) keeps the older rule: label plus account, with an unrecorded account honoured on the label alone (BUG-3341). A worker refreshing its own name-only lease binds it. A name-only caller never matches a worker-bound lease, even under the same label: that is refused `409 lease_held`, and the message says how to get a worker id. A contended claim or release answers `409 lease_held` with `details: {ref, holder, worker?, acquired_at, expires_at}`, where `worker` is the SHORT id (`wk_3f2a`). The full id appears only in the claim's own success answer (`worker_id`, for resume); reads (`item.lease`, list decoration) show `holder` and the short `worker`. `GET /next` and the dashboard's `suggested_next` leave out items whose live lease a claim by the same identity would lose, by the same predicate (`store.leaseOurs`).
- `GET/POST /workspaces/{ws}/items/{slug}/verifications` — verification evidence tied to a revision (TASK-3617). One row is one verdict (`pass` / `fail` / `cant_tell`) on one acceptance criterion (`AC-N`) of the item that CARRIES the criteria, optionally at a `code_revision` (full 40-hex SHA) and naming the item that did the work (`via`). POST takes `{run_id?, code_revision?, via?, results: [{criterion, verdict, evidence?, evidence_url?}]}` (at most 100 results; evidence at most 2048 bytes; `evidence_url` an absolute http(s) URL) and is atomic: one bad result refuses the batch. A criterion that is missing, still a placeholder, or appears more than once in the BODY is `400 unknown_criterion`, with `details.criteria` listing the valid ids; a criterion named twice in one REQUEST is a malformed request, `400 validation_error`. A `via` that does not resolve in the workspace, or names an item the caller cannot see, is one answer: `400 unknown_via`. The write is guarded by the item's seq: the criterion hashes are stored against the body they were parsed from, and an item that keeps changing during the write answers `409 item_changed`. Needs edit permission on the item; the read follows item visibility. Writes log a `verified` activity (metadata: `run_id`, a verdict count, never evidence text) and publish `item_updated`. GET returns `{ref, criteria, verifications}`, the newest verdict per criterion in body order, or the whole history with `?all=1`. Each row's `state` is computed at read time from the criterion, never from the item's seq: `criterion_removed` (no longer a real criterion), `criterion_changed` (its text changed since the verdict; whitespace is collapsed first, so re-wrapping is not a change), `superseded` (a newer verdict exists), else `current`. `possibly_stale: {via: "PR #N", merged_sha, merged_at}` is ADVISORY and never changes `state`: a merged `github_pr` on the item, or on its visible `via` item, merged after the verdict at a different SHA. It needs `github_pr.merge_sha` / `merged_at`, which `pad github link` and `pad reconcile` now record (`merge_sha` must be a full 40-hex SHA, `merged_at` RFC3339, or the PR write is `400`). Rows are append-only and travel with a workspace export/import (so `pad db migrate-to-pg` carries them), verbatim except for the remapped item and via ids; the verifier's user id does not travel, and a malformed bundle row is dropped rather than failing the import. They survive an archive and restore, are removed by a hard delete of the item, are not cloned by a copy (the copy's dropped bucket does not report them yet), and outlive a deleted `via` item (whose ref is then omitted)
- `GET /workspaces/{ws}/items/{slug}/decisions` — the item's latest typed-decision answer per question (PLAN-3114 / TASK-3117), each with `current: true` only when it was computed from the item's PRESENT state (a sha256 of the exact state bytes sent to the provider — title, collection, fields, bounded body, and per set: the last 10 comments for `attention`, and the parent and blocking links and NO comments for `conventions`) AND for the question as registered now under the model pinned now — rewording a question or changing the model re-asks. A third condition since TASK-3118: the set must still APPLY to the item (its collection is covered and the set's eligibility predicate accepts the item as it stands). A collection schema edit can make an item terminal without changing a byte of its state, and its answers then read `current: false`. Evaluation is asynchronous: item create / update / move and comment create enqueue a coalesced `decision_jobs` row in their own transaction and a server tick does the provider call, so nothing on a write path waits on the network. A re-ask is debounced (TASK-3916): a write within an hour of the item's latest evaluation for a set owes a job (`decision_jobs.not_before`) that the tick does not claim until that hour has passed, and later writes in the window coalesce into it without moving it. A first evaluation is never delayed, and a per-comment set has no window. System fan-out rewrites (title-rename cascade, field migration, attachment remap) deliberately do NOT enqueue — their items read `current: false` until the next direct write — and neither does workspace import, whose items have no decisions until their next direct write. With no provider configured the list is empty and no job row is written, with one exception: if decisions are disabled while the one-shot effort backfill (TASK-3921) is writing a chunk, that chunk may still commit, and its jobs wait until a provider is configured again. The single-item GET (and `pad item show --format json` / `--agent`) carries the same list as `decisions` when non-empty, with the staged keys (`effort`, `effort_medium`, `high_stakes`) omitted; this endpoint returns them. Two sets are registered. `attention` (TASK-3118): four questions in one call, asked only about OPEN items in NON-SYSTEM collections. `needs_human_decision` (a Noul) is the one surfaced anywhere. `effort` (a Score over four levels), `effort_medium` (a Choice: desk, device, other_person, unclear) and `high_stakes` (a Noul) are STAGED before their reader (TASK-3959, IDEA-3906 B4b): they are stored and returned by this endpoint, but no dashboard entry, chip, catch-up or item brief shows or counts them. A one-shot backfill owes the attention set to every open item that has a `needs_human_decision` answer and no `effort` answer: it is enqueued when the provider is configured (boot, or a later admin enable), runs on the tick under the re-ask debounce, makes no provider call itself, and is gated by the `effort_decisions_backfilled` platform setting, written only after every job was owed. Its `blocked` and `waiting_on_external` Nouls were retired in TASK-3915 / TASK-3924 (the dependency graph owns blocked). Their stored answers stay in the list with `current: false`, because no registered question carries their keys. A job owed for a terminal item is dropped at the tick without a provider call, and reopening the item re-enqueues it. `conventions` (TASK-3119): one Noul per active `always` convention whose `decision_check` is not `off` and whose body is at most 4 KB, keyed `conv:<ref>`, asked about items in non-system collections, at most 12 per provider call; the web shows a "Possibly breaks CONVE-N" chip at p >= 0.9 and nothing below it. Its state carries no comment trail (U2a), so a comment write changes nothing it reads and costs it no call. `conventions_comments` (U2b), which asked the same questions about each COMMENT alone keyed `conv:<ref>@<comment-id>`, is no longer registered (TASK-3924: 42% of provider calls for almost no chip-level answers). Its stored rows read `current: false`, and the web shows no per-comment chip. Each evaluation that reaches the provider logs one `decision evaluation` line with its calls and tokens
- `GET /workspaces/{ws}/items/{slug}/decisions` — the item's latest typed-decision answer per question (PLAN-3114 / TASK-3117), each with `current: true` only when it was computed from the item's PRESENT state (a sha256 of the exact state bytes sent to the provider — title, collection, fields, bounded body, and per set: the last 10 comments for `attention`, and the parent and blocking links and NO comments for `conventions`) AND for the question as registered now under the model pinned now — rewording a question or changing the model re-asks. A third condition since TASK-3118: the set must still APPLY to the item (its collection is covered and the set's eligibility predicate accepts the item as it stands). A collection schema edit can make an item terminal without changing a byte of its state, and its answers then read `current: false`. Evaluation is asynchronous: item create / update / move and comment create enqueue a coalesced `decision_jobs` row in their own transaction and a server tick does the provider call, so nothing on a write path waits on the network. A re-ask is debounced (TASK-3916): a write within an hour of the item's latest evaluation for a set owes a job (`decision_jobs.not_before`) that the tick does not claim until that hour has passed, and later writes in the window coalesce into it without moving it. A first evaluation is never delayed, and a per-comment set has no window. System fan-out rewrites (title-rename cascade, field migration, attachment remap) deliberately do NOT enqueue — their items read `current: false` until the next direct write — and neither does workspace import, whose items have no decisions until their next direct write. With no provider configured the list is empty and no job row is written, with one exception: if decisions are disabled while the one-shot effort backfill (TASK-3921) is writing a chunk, that chunk may still commit, and its jobs wait until a provider is configured again. The single-item GET (and `pad item show --format json` / `--agent`) carries the same list as `decisions` when non-empty, with the staged keys (`effort`, `effort_medium`, `high_stakes`) omitted; this endpoint returns them. Two sets are registered. `attention` (TASK-3118): four questions in one call, asked only about OPEN items in NON-SYSTEM collections. `needs_human_decision` (a Noul) is the one surfaced anywhere. `effort` (a Score over four levels), `effort_medium` (a Choice: desk, device, other_person, unclear) and `high_stakes` (a Noul) are STAGED before their reader (TASK-3959, IDEA-3906 B4b): they are stored and returned by this endpoint, but no dashboard entry, chip, catch-up or item brief shows or counts them. A one-shot backfill owes the attention set to every item that has a `needs_human_decision` answer and no `effort` answer and that the attention set applies to now (open, in a non-system collection that tracks work). It owes them in chunks of one tick pass (20 by default), each chunk only after the previous chunk's own jobs have been taken (no job of it is claimable right now), polling at the tick interval, so a live re-ask interleaves with the backlog rather than waiting behind all of it; a job held by the re-ask debounce cannot be claimed yet and does not hold the next chunk, and because those come due later, a few of them may become claimable together ahead of a live re-ask, once. The marker is written once the last chunk is owed, without waiting for it to drain. It is enqueued when the provider is configured (boot, or a later admin enable), runs on the tick under the re-ask debounce, makes no provider call itself, and is gated by the `effort_decisions_backfilled` platform setting, written only after every job was owed. Its `blocked` and `waiting_on_external` Nouls were retired in TASK-3915 / TASK-3924 (the dependency graph owns blocked). Their stored answers stay in the list with `current: false`, because no registered question carries their keys. A job owed for a terminal item is dropped at the tick without a provider call, and reopening the item re-enqueues it. `conventions` (TASK-3119): one Noul per active `always` convention whose `decision_check` is not `off` and whose body is at most 4 KB, keyed `conv:<ref>`, asked about items in non-system collections, at most 12 per provider call; the web shows a "Possibly breaks CONVE-N" chip at p >= 0.9 and nothing below it. Its state carries no comment trail (U2a), so a comment write changes nothing it reads and costs it no call. `conventions_comments` (U2b), which asked the same questions about each COMMENT alone keyed `conv:<ref>@<comment-id>`, is no longer registered (TASK-3924: 42% of provider calls for almost no chip-level answers). Its stored rows read `current: false`, and the web shows no per-comment chip. Each evaluation that reaches the provider logs one `decision evaluation` line with its calls and tokens
- `POST /workspaces/{ws}/playbooks/{ref}/run` — bind a playbook's declared arguments and return its body (`pad playbook run`, `pad_playbook.action: run`). Side-effect-free: nothing is stored. So it is the ONE POST a read-scoped token (`read` / `pad:read`, PAT or OAuth) may make (TASK-2863). That is an exact-path allowlist in `tokenScopeAllows`; both variable segments must be plain names (RFC 3986 unreserved characters). `/playbooks/match` stays refused for read tokens, as do every write and the item `claim` / `release` routes
- `POST /workspaces/{ws}/playbooks/match {text}` — which ACTIVE playbook (or `none`) free text asks for, answered by the decision provider synchronously (TASK-3120; `pad playbook match -- "<text>"`, `pad_playbook.action: match`). 404 `decision_provider_unavailable` with no provider, so callers fall back to slug/trigger routing. Each call that reaches the provider spends from its own bucket, 30/min per user, burst 5, charged immediately before the provider call, so an unconfigured 404, a 400 for the text, a too-many-playbooks 400 or a zero-playbook answer consumes nothing (a request-too-large 400 is raised inside that call and has spent its token); over it is `429` with `Retry-After` (TASK-3141). No env knob by design: tuning belongs in the instance-admin decision-provider setting
- `GET /workspaces/{ws}/dashboard` — computed project overview (active items, plans, attention, blockers). Also carries `pending_reminders`: fired-but-unacknowledged reminders, which is the delivery path on any instance with no webhook configured. With a decision provider, `attention` also carries `needs_human` entries (the item's latest `needs_human_decision` answer is >= 0.7). `blocked` entries come from the dependency graph only; the text-derived kind was retired in TASK-3915. Only CURRENT answers count, the rule the decisions endpoint applies (the item's present state, the question as registered now, the model pinned now), with one exception (TASK-3915): an item edited or commented since its last evaluation KEEPS its entry while the re-ask is pending, that is while a job is owed for it with no recorded error and the item is still open. It drops off when no job is owed or the job has failed, and the re-ask's answer replaces it. The state check costs one item-plus-trail read per ABOVE-threshold item, never per workspace item. A failing provider (any owed `attention` job with a recorded error) marks the section `attention.decisions` in `degraded_sections`. `pad project stale` filters on type, so it reports graph-`blocked` entries and not `needs_human`
+1
View File
@@ -173,6 +173,7 @@ func newAttentionFixture(t *testing.T) *attentionFixture {
// tick; the re-ask debounce (TASK-3916) has its own tests, which this
// would otherwise make every write-then-tick here wait an hour for.
fx.s.SetDecisionDebounce(nil)
fx.r.backfillPoll = time.Millisecond // the backfill's drain wait polls fast
return fx
}
+57 -2
View File
@@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log/slog"
"time"
)
// The one-shot effort backfill (TASK-3921, IDEA-3906 B4a).
@@ -26,8 +27,14 @@ import (
// candidate was owed.
const EffortBackfillSetting = "effort_decisions_backfilled"
// effortBackfillChunk is how many items one OweDecisionJobs call covers.
const effortBackfillChunk = 500
// effortBackfillChunk is how many items one OweDecisionJobs call covers: one
// tick pass (the server's defaultDecisionTickLimit, which wires the live value
// through SetBackfillPacing).
const effortBackfillChunk = 20
// effortBackfillPoll is how often the backfill re-checks that its last chunk
// drained: the tick interval (the server wires the live value).
const effortBackfillPoll = 5 * time.Second
// EffortBackfillCandidates returns the items the backfill would owe a job
// for, grouped by workspace in workspace order: live, answered for
@@ -92,6 +99,20 @@ func (r *Runner) CountEffortBackfill(ctx context.Context) (int, error) {
// marker. owed is the number of jobs owed by this call (0 when the marker was
// already set). A nil runner does nothing.
//
// PACING (lead ruling on TASK-3921, r3 M1). Claims run oldest-enqueued first
// and evaluation is sequential, so owing every candidate at once would put a
// live edit's re-ask behind the whole backlog. The backfill therefore owes one
// tick pass of items (backfillChunk) and owes the next only once THAT chunk's
// own jobs have drained, where drained means none of the chunk's items has a
// job ClaimDecisionJobs could take right now (CountClaimableDecisionJobs: the
// same predicate). It does NOT gate on total queue depth, which would starve
// the backfill under live load. A job held by the re-ask debounce (not_before
// ahead, up to ~1h) cannot be claimed yet and must not stall the run; a
// claimed, answered or deleted job is drained. Live jobs owed meanwhile queue
// behind at most one chunk. The wait polls at the tick interval and honours
// ctx; a cancel in it returns ctx.Err() with no marker. The marker is written
// after the LAST chunk is owed, without waiting for it to drain.
//
// ctx is checked before each candidate read and before each chunk. A cancel
// that lands before a chunk returns ctx.Err() with no marker, so the next run
// redoes the work. A cancel that lands after the last chunk may still write
@@ -117,6 +138,7 @@ func (r *Runner) BackfillEffort(ctx context.Context) (owed int, err error) {
chunk = effortBackfillChunk
}
n := 0
var prev []string // the previous chunk's items, awaiting drain
for _, ws := range order {
ids := byWS[ws]
for start := 0; start < len(ids); start += chunk {
@@ -124,6 +146,11 @@ func (r *Runner) BackfillEffort(ctx context.Context) (owed int, err error) {
if end > len(ids) {
end = len(ids)
}
if prev != nil {
if err := r.waitChunkDrained(ctx, prev); err != nil {
return owed, err
}
}
if r.beforeBackfillChunk != nil {
if err := r.beforeBackfillChunk(ctx, n); err != nil {
return owed, err
@@ -137,6 +164,7 @@ func (r *Runner) BackfillEffort(ctx context.Context) (owed int, err error) {
return owed, err
}
owed += end - start
prev = ids[start:end]
n++
}
}
@@ -151,3 +179,30 @@ func (r *Runner) BackfillEffort(ctx context.Context) (owed int, err error) {
l.Info("effort backfill: owed attention jobs for items with no effort answer", "owed", owed)
return owed, nil
}
// waitChunkDrained blocks until none of ids has a job claimable now (see
// BackfillEffort), polling at backfillPoll. The check runs first, so an
// already-drained chunk costs one query.
func (r *Runner) waitChunkDrained(ctx context.Context, ids []string) error {
poll := r.backfillPoll
if poll <= 0 {
poll = effortBackfillPoll
}
for {
if err := ctx.Err(); err != nil {
return err
}
claimable, err := r.store.CountClaimableDecisionJobs(AttentionSetName, ids, time.Now())
if err != nil {
return err
}
if claimable == 0 { // THE DRAIN-WAIT LINE: mutate to skip the wait
return nil
}
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(poll):
}
}
}
+187 -2
View File
@@ -76,6 +76,62 @@ func (fx *attentionFixture) marker(t *testing.T) string {
return v
}
// holdDrained plays a tick that claims every job as it appears: it marks each
// decision_jobs row claimed under a far lease (rows stay, so tests still count
// them) until the test ends. A multi-chunk backfill needs it, since each chunk
// waits for the previous one's jobs to be taken.
func (fx *attentionFixture) holdDrained(t *testing.T) {
t.Helper()
stop := make(chan struct{})
done := make(chan struct{})
go func() {
defer close(done)
far := time.Now().Add(time.Hour).UTC().Format(evalTimeFormat)
for {
select {
case <-stop:
return
case <-time.After(time.Millisecond):
_, _ = fx.s.DB().Exec(`UPDATE decision_jobs SET claimed_by = 'test', lease_expires_at = ? WHERE claimed_by IS NULL`, far)
}
}
}()
t.Cleanup(func() { close(stop); <-done })
}
// seedCandidates makes n open items answered for needs_human_decision only and
// clears the jobs their creation owed.
func (fx *attentionFixture) seedCandidates(t *testing.T, n int) []string {
t.Helper()
var ids []string
for i := 0; i < n; i++ {
it := fx.item(t, fx.tasks.ID, "open")
fx.seedAnswers(t, it, time.Now().Add(-3*time.Hour), AttentionNeedsHuman)
ids = append(ids, it.ID)
}
fx.clearJobs(t)
return ids
}
// waitJobs polls until at least n decision_jobs rows exist.
func (fx *attentionFixture) waitJobs(t *testing.T, n int) {
t.Helper()
deadline := time.Now().Add(10 * time.Second)
for {
var c int
if err := fx.s.DB().QueryRow(`SELECT COUNT(*) FROM decision_jobs`).Scan(&c); err != nil {
t.Fatal(err)
}
if c >= n {
return
}
if time.Now().After(deadline) {
t.Fatalf("never saw %d decision_jobs rows", n)
}
time.Sleep(time.Millisecond)
}
}
var allAttentionKeys = []string{AttentionNeedsHuman, AttentionEffort, AttentionEffortMedium, AttentionHighStakes}
func TestEffortBackfill_RunsOnceAndSkipsUnasked(t *testing.T) {
@@ -161,6 +217,7 @@ func TestEffortBackfill_FailingSecondChunkThenRerun(t *testing.T) {
}
fx.clearJobs(t)
fx.r.backfillChunk = 1
fx.holdDrained(t) // chunk k+1 waits for chunk k's jobs to be taken
boom := errors.New("crash")
fx.r.beforeBackfillChunk = func(_ context.Context, chunk int) error {
if chunk == 1 {
@@ -319,9 +376,28 @@ func TestEffortBackfillDryRunOnFile(t *testing.T) {
if err := refuseLiveDatabase(path, os.Getenv("HOME")); err != nil {
t.Fatalf("%v", err)
}
s, err := store.New(path)
// Open a private COPY, never the named file: store.New migrates what it
// opens. The copy lives under GOTMPDIR (disk), not /tmp (RAM tmpfs).
dir := t.TempDir()
if g := os.Getenv("GOTMPDIR"); g != "" {
d, err := os.MkdirTemp(g, "effort-dryrun-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { os.RemoveAll(d) })
dir = d
}
copyPath := filepath.Join(dir, "copy.db")
src, err := os.ReadFile(path)
if err != nil {
t.Fatalf("open %s: %v", path, err)
t.Fatal(err)
}
if err := os.WriteFile(copyPath, src, 0o600); err != nil {
t.Fatal(err)
}
s, err := store.New(copyPath)
if err != nil {
t.Fatalf("open %s: %v", copyPath, err)
}
defer s.Close()
reg, err := ProductionRegistry()
@@ -411,6 +487,7 @@ func TestEffortBackfill_CancelStopsBetweenChunksAndWritesNoMarker(t *testing.T)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
fx.r.backfillChunk = 1
fx.holdDrained(t) // chunk k+1 waits for chunk k's jobs to be taken
fx.r.beforeBackfillChunk = func(_ context.Context, chunk int) error {
if chunk == 1 {
cancel() // lands inside the hook; the chunk-boundary check must see it
@@ -441,6 +518,7 @@ func TestEffortBackfill_StoreFailureMidRunWritesNoMarker(t *testing.T) {
}
fx.clearJobs(t)
fx.r.backfillChunk = 1
fx.holdDrained(t) // chunk k+1 waits for chunk k's jobs to be taken
fx.r.beforeBackfillChunk = func(_ context.Context, chunk int) error {
if chunk == 1 {
if _, err := fx.s.DB().Exec(`ALTER TABLE decision_jobs RENAME TO decision_jobs_gone`); err != nil {
@@ -469,3 +547,110 @@ func TestEffortBackfill_StoreFailureMidRunWritesNoMarker(t *testing.T) {
t.Fatalf("re-run owed %d items; want 3", len(got))
}
}
// Pacing (TASK-3921 r3 M1). The drain wait is ONE condition, marked
// "THE DRAIN-WAIT LINE" in waitChunkDrained (effort_backfill.go); making it
// return at once makes the next two tests go red.
// A live job owed after chunk 1 is claimed before chunk 2's jobs are owed.
func TestEffortBackfill_LiveReaskInterleavesWithChunks(t *testing.T) {
fx := newAttentionFixture(t)
fx.seedCandidates(t, 3)
fx.r.backfillChunk = 1
done := make(chan error, 1)
go func() { _, err := fx.r.BackfillEffort(context.Background()); done <- err }()
fx.waitJobs(t, 1) // chunk 0 is owed and (nobody claims yet) waiting
// A normal write (a new item owes its job): it queues behind chunk 0.
live := fx.item(t, fx.tasks.ID, "open")
// Now a tick: claim one job per pass, oldest first, recording the order.
var order []string
deadline := time.Now().Add(10 * time.Second)
for len(order) < 4 {
if time.Now().After(deadline) {
t.Fatalf("claimed only %v", order)
}
js, err := fx.s.ClaimDecisionJobs("test", 1, time.Hour)
if err != nil {
t.Fatal(err)
}
for _, j := range js {
order = append(order, j.ItemID)
}
time.Sleep(time.Millisecond)
}
if err := <-done; err != nil {
t.Fatal(err)
}
if order[1] != live.ID {
t.Fatalf("claim order %v: the live re-ask (%s) must come right after chunk 0, before chunk 1's job", order, live.ID)
}
}
// With chunk 0's jobs left claimable, chunk 1 is not owed; a cancel in the
// wait returns ctx.Err() with no chunk-1 job and no marker.
func TestEffortBackfill_HoldsNextChunkUntilDrainedThenCancelWritesNoMarker(t *testing.T) {
fx := newAttentionFixture(t)
fx.seedCandidates(t, 3)
fx.r.backfillChunk = 1
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
type res struct {
owed int
err error
}
done := make(chan res, 1)
go func() { o, err := fx.r.BackfillEffort(ctx); done <- res{o, err} }()
fx.waitJobs(t, 1)
time.Sleep(100 * time.Millisecond) // ~100 polls at 1ms
if got := fx.jobItems(t); len(got) != 1 {
t.Fatalf("jobs while chunk 0 is undrained = %v; want only chunk 0's", got)
}
cancel()
r := <-done
if !errors.Is(r.err, context.Canceled) || r.owed != 1 {
t.Fatalf("owed=%d err=%v; want 1 and context.Canceled", r.owed, r.err)
}
if got := fx.jobItems(t); len(got) != 1 {
t.Fatalf("jobs after the cancel = %v", got)
}
if fx.marker(t) != "" {
t.Fatal("marker written by a run cancelled in the drain wait")
}
}
// A chunk whose jobs are ALL debounced (not_before ahead) cannot be claimed and
// so must not hold the next chunk; the run completes and marks.
func TestEffortBackfill_DebouncedChunkDoesNotHoldTheNext(t *testing.T) {
fx := newAttentionFixture(t)
fx.seedCandidates(t, 3)
fx.r.backfillChunk = 1
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
done := make(chan error, 1)
go func() { _, err := fx.r.BackfillEffort(ctx); done <- err }()
// Debounce every job as it appears (an item evaluated minutes ago).
future := time.Now().Add(time.Hour).UTC().Format(evalTimeFormat)
stop := make(chan struct{})
go func() {
for {
select {
case <-stop:
return
case <-time.After(time.Millisecond):
_, _ = fx.s.DB().Exec(`UPDATE decision_jobs SET not_before = ? WHERE not_before IS NULL`, future)
}
}
}()
err := <-done
close(stop)
if err != nil {
t.Fatalf("a debounced chunk held the backfill: %v", err)
}
if got := fx.jobItems(t); len(got) != 3 {
t.Fatalf("jobs = %v; want all three owed", got)
}
if fx.marker(t) == "" {
t.Fatal("marker not written")
}
}
+15 -1
View File
@@ -383,6 +383,7 @@ type Runner struct {
// beforeBackfillChunk, when set, runs before each chunk is owed (chunk
// index from 0) and may fail the backfill there. Both are TEST-ONLY seams.
backfillChunk int
backfillPoll time.Duration
beforeBackfillChunk func(ctx context.Context, chunk int) error
// logger receives the per-evaluation spend line; nil means slog.Default().
@@ -781,10 +782,23 @@ func (r *Runner) SetBackfillSeams(chunk int, hook func(ctx context.Context, chun
if r == nil {
return
}
r.backfillChunk = chunk
if chunk > 0 {
r.backfillChunk = chunk
}
r.beforeBackfillChunk = hook
}
// SetBackfillPacing sets how many items one backfill chunk owes (the tick's
// per-pass limit) and how often the backfill re-checks that the chunk drained
// (the tick interval). Non-positive values keep the defaults.
func (r *Runner) SetBackfillPacing(chunk int, poll time.Duration) {
if r == nil {
return
}
r.backfillChunk = chunk
r.backfillPoll = poll
}
// ItemReadDecisions is Decisions without the staged keys (TASK-3921): what the
// item read, the CLI --agent projection and MCP item get carry. The raw
// Decisions stays behind the decisions endpoint, which shows staged data.
+14
View File
@@ -85,6 +85,20 @@ func (s *Server) reconfigureDecisionsLocked() error {
provider = nil
}
runner := decision.NewRunner(s.store, provider, sets)
if runner != nil {
// Pace the effort backfill by one tick pass (TASK-3921): the tick's
// own per-pass limit and interval, defaults when not yet started.
s.decisionTick.mu.Lock()
chunk, poll := s.decisionTick.limit, s.decisionTick.interval
s.decisionTick.mu.Unlock()
if chunk == 0 {
chunk = defaultDecisionTickLimit
}
if poll == 0 {
poll = defaultDecisionTickInterval
}
runner.SetBackfillPacing(chunk, poll)
}
if runner != nil && s.decisionSettings.backfillHook != nil {
runner.SetBackfillSeams(s.decisionSettings.backfillChunk, s.decisionSettings.backfillHook)
}
+35
View File
@@ -377,6 +377,41 @@ func (s *Store) ClaimDecisionJobs(runner string, limit int, lease time.Duration)
return claimed, nil
}
// CountClaimableDecisionJobs counts the set's jobs for itemIDs that
// ClaimDecisionJobs could take RIGHT NOW at `now`: the very decisionJobClaimable
// predicate, so the two cannot drift. A job that is claimed (lease live), held
// by the re-ask debounce (not_before ahead), held by a soft-deleted workspace,
// answered or deleted is not counted. The effort backfill uses it to tell
// whether its last chunk has drained (TASK-3921).
func (s *Store) CountClaimableDecisionJobs(set string, itemIDs []string, now time.Time) (int, error) {
nowS := decisionTime(now)
total := 0
for start := 0; start < len(itemIDs); start += oweDecisionChunk {
end := start + oweDecisionChunk
if end > len(itemIDs) {
end = len(itemIDs)
}
part := itemIDs[start:end]
args := make([]any, 0, len(part)+3)
args = append(args, set)
marks := make([]string, len(part))
for i, id := range part {
marks[i] = "?"
args = append(args, id)
}
args = append(args, nowS, nowS)
var n int
if err := s.db.QueryRow(s.q(`
SELECT COUNT(*) FROM decision_jobs
WHERE question_set = ? AND item_id IN (`+strings.Join(marks, ",")+`)
AND `+decisionJobClaimable), args...).Scan(&n); err != nil {
return 0, fmt.Errorf("count claimable decision jobs: %w", err)
}
total += n
}
return total, nil
}
// CompleteDecisionJob retires a claimed job.
//
// The job is deleted only if its generation is still the one claimed. If a
+60
View File
@@ -82,3 +82,63 @@ func TestItemsMissingDecisionKey(t *testing.T) {
t.Fatalf("candidates = %+v; want only item A", got)
}
}
// CountClaimableDecisionJobs counts what ClaimDecisionJobs could take now: not
// a claimed job, not a debounced one, not a retired one, nor another item's.
func TestCountClaimableDecisionJobs(t *testing.T) {
s, ws, col := debounceFixture(t)
s.SetDecisionSetResolver(func(string) []string { return nil })
recent := createTestItem(t, s, ws.ID, col.ID, "Recent", "")
a := createTestItem(t, s, ws.ID, col.ID, "A", "")
b := createTestItem(t, s, ws.ID, col.ID, "B", "")
outside := createTestItem(t, s, ws.ID, col.ID, "Outside", "")
evaluated(t, s, ws, recent.ID, "triage", time.Now().Add(-10*time.Minute))
if err := s.OweDecisionJobs(ws.ID, "triage", []string{recent.ID, a.ID, b.ID, outside.ID}); err != nil {
t.Fatal(err)
}
ids := []string{recent.ID, a.ID, b.ID}
count := func(set string, at time.Time) int {
t.Helper()
n, err := s.CountClaimableDecisionJobs(set, ids, at)
if err != nil {
t.Fatal(err)
}
return n
}
now := time.Now()
// recent is debounced; outside is not asked about; the other set has none.
if n := count("triage", now); n != 2 {
t.Fatalf("claimable = %d; want 2 (a, b; recent is debounced)", n)
}
if n := count("other", now); n != 0 {
t.Fatalf("another set counted %d", n)
}
if n, err := s.CountClaimableDecisionJobs("triage", nil, now); err != nil || n != 0 {
t.Fatalf("no ids = %d, %v", n, err)
}
// Claim everything claimable (a, b, outside): none is claimable now.
jobs, err := s.ClaimDecisionJobs("r", 10, time.Hour)
if err != nil || len(jobs) != 3 {
t.Fatalf("claim: %v %v", jobs, err)
}
if n := count("triage", now); n != 0 {
t.Fatalf("after claiming claimable = %d; want 0 (claimed and debounced)", n)
}
// A lease that has run out makes the claimed jobs claimable again (a, b).
if n := count("triage", now.Add(2*time.Hour)); n != 3 {
t.Fatalf("expired leases + passed debounce claimable = %d; want 3", n)
}
// Retire them: none claimable now.
for _, j := range jobs {
if err := s.CompleteDecisionJob(j); err != nil {
t.Fatal(err)
}
}
if n := count("triage", now); n != 0 {
t.Fatalf("after completing claimable = %d; want 0", n)
}
// Two hours on, only recent (debounce passed) remains.
if n := count("triage", now.Add(2*time.Hour)); n != 1 {
t.Fatalf("two hours on claimable = %d; want 1", n)
}
}