mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-27 23:07:35 +00:00
feat(decision): debounce decision re-asks with decision_jobs.not_before (TASK-3916) (#2230)
* feat(decision): debounce decision re-asks with decision_jobs.not_before (TASK-3916) IDEA-3906 half B, unit B2. - Migrations 150 (SQLite) and 120 (Postgres) add decision_jobs.not_before TEXT. A job is claimable only once not_before is NULL or past, in the one decisionJobClaimable predicate both the scan and the claim UPDATE use. - The enqueue sets not_before to the item's latest evaluation for the set plus the set's window, or NULL when there is no window, no prior evaluation (a first evaluation is never delayed), or the window is past. The arithmetic is in Go on decisionTime TEXT, so both backends agree. On conflict COALESCE(old, new) keeps an existing not_before, so a stream of comments cannot push the re-ask back. - A superseded completion RECOMPUTES not_before from the latest evaluation rather than setting now+window (ruling ae2e0a90): the mid-run write read the evaluations before this run's answers landed. A run that stored nothing moves nothing, so it never invents a delay. - Runner.Install wires Store.SetDecisionDebounce: ReaskDebounce (1h) for the item-level sets, none for a PerComment set or an unregistered one. - The dashboard's attention comment says the pending window can now last up to an hour (#2229 r3-D1, ruling ce3e9dc5). - The tick-interval and release comments no longer promise a job is claimable at once: a debounced one waits for its window. - docs/api.md and README updated. Claude-Session: https://claude.ai/code/session_018oQ5EErQKHtBJK8yLpcDNF * docs: a debounced burst costs at most one call an hour (TASK-3916; #2230 r1 codex Low) * docs: the debounce is per question set (TASK-3916; #2230 r2 codex Low) * fix(store): debounce helpers stay inside the app-fence boundary (TASK-3916; #2230 CI TestAppstoreBoundary)
This commit is contained in:
@@ -609,7 +609,7 @@ Pad can ask a small classification model typed questions about your items, such
|
||||
- `pad playbook match -- "<text>"` (`pad_playbook action=match` over MCP) picks the active playbook that free text asks for. It answers `404 decision_provider_unavailable` with no provider, so callers fall back to slug or trigger routing.
|
||||
- `GET /workspaces/{ws}/items/{slug}/decisions` lists an item's latest answers.
|
||||
|
||||
Answers are computed **asynchronously**. Creating, updating, restoring or moving an item, and creating, editing or deleting a comment on it, queues a job, and a server tick makes the provider call, so no write ever waits on the network. Only open items in non-system collections are asked about: a job owed for a closed item is dropped at the tick without a call.
|
||||
Answers are computed **asynchronously**. Creating, updating, restoring or moving an item, and creating, editing or deleting a comment on it, queues a job, and a server tick makes the provider call, so no write ever waits on the network. A re-ask within an hour of the item's last evaluation for a question set waits until that hour is up, so a burst of edits and comments costs at most one call an hour per set, not one per write. Only open items in non-system collections are asked about: a job owed for a closed item is dropped at the tick without a call.
|
||||
|
||||
**Configure** (later sources win, per field): the config file, then the instance-admin setting (**Console → Admin → Settings**), then the environment.
|
||||
|
||||
|
||||
+1
-1
@@ -69,7 +69,7 @@ REST API at `/api/v1/`. Key endpoints:
|
||||
- `GET/POST /workspaces/{ws}/items/{slug}/reminders` — item reminders (IDEA-2641). POST arms one; `remind_at` is an RFC3339 **instant** and a bare date is refused, not read as midnight
|
||||
- `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 /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. 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 ever written. The single-item GET (and `pad item show --format json` / `--agent`) carries the same list as `decisions` when non-empty. Two sets are registered. `attention` (TASK-3118): one Noul, `needs_human_decision`, asked only about OPEN items in NON-SYSTEM collections. 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 ever written. The single-item GET (and `pad item show --format json` / `--agent`) carries the same list as `decisions` when non-empty. Two sets are registered. `attention` (TASK-3118): one Noul, `needs_human_decision`, asked only about OPEN items in NON-SYSTEM collections. 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`
|
||||
|
||||
@@ -70,8 +70,9 @@ var allowedStoreMethodsInsideDoor = map[string]string{
|
||||
"workspaceHasInstalledAppTx": "reads app_installs (does the workspace have an install not uninstalled)",
|
||||
"outboxRowCap": "reads in-memory config; no SQL",
|
||||
"outboxClaimableRowCap": "reads in-memory config; no SQL",
|
||||
"enqueueDecisionJobsTx": "INSERT ... ON CONFLICT DO UPDATE on decision_jobs for the item it is given; no-op without a provider",
|
||||
"enqueueDecisionJobsTx": "INSERT ... ON CONFLICT DO UPDATE on decision_jobs for the item it is given; no-op without a provider. Reads MAX(item_decisions.evaluated_at) for the item when its set has a debounce window (TASK-3916)",
|
||||
"decisionSetResolver": "loads an in-memory pointer; no SQL",
|
||||
"decisionDebounceFunc": "loads an in-memory pointer; no SQL (TASK-3916)",
|
||||
"createActivityQ": "INSERT INTO activities on the executor it is given (the fence's tx)",
|
||||
"recentDebounceCandidateQ": "reads activities",
|
||||
// U2b (TASK-3391).
|
||||
@@ -114,7 +115,11 @@ var reviewedStoreFuncs = map[string]string{
|
||||
// func-value rule is never loosened globally; a new case gets its own entry.
|
||||
var allowedFuncValues = map[string]map[string]string{
|
||||
"(*github.com/PerpetualSoftware/pad/internal/store.Store).enqueueDecisionJobsTx": {
|
||||
"resolve": "the store's DecisionSetResolver; production installs decision.Registry.SetsFor, an in-memory lookup that runs no SQL",
|
||||
"resolve": "the store's DecisionSetResolver; production installs decision.Registry.SetsFor, an in-memory lookup that runs no SQL",
|
||||
"debounce": "the store's DecisionDebounce (TASK-3916), passed to decisionWindowFor; production installs decision.Runner.debounceFor, a registry lookup that runs no SQL",
|
||||
},
|
||||
"github.com/PerpetualSoftware/pad/internal/store.decisionWindowFor": {
|
||||
"debounce": "the store's DecisionDebounce (TASK-3916); production installs decision.Runner.debounceFor, a registry lookup that runs no SQL",
|
||||
},
|
||||
"github.com/PerpetualSoftware/pad/internal/store.collapseChanges": {
|
||||
"isLossySummary": "a closure defined in the same body and never reassigned; the walk inspects its body",
|
||||
|
||||
@@ -121,9 +121,51 @@ func newAttentionFixture(t *testing.T) *attentionFixture {
|
||||
}
|
||||
fx.r = NewRunner(fx.s, p, reg)
|
||||
fx.r.Install(fx.s)
|
||||
// These tests are about currency and eligibility, and re-ask on the next
|
||||
// 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)
|
||||
return fx
|
||||
}
|
||||
|
||||
// Install installs the re-ask window beside the resolver (TASK-3916):
|
||||
// ReaskDebounce for the item-level sets the production registry ships, none
|
||||
// for a set not registered, and a nil runner removes it.
|
||||
func TestInstall_DebouncesTheItemLevelSets(t *testing.T) {
|
||||
fx := newAttentionFixture(t)
|
||||
fx.r.Install(fx.s) // the fixture turned it off; put the production wiring back
|
||||
it := fx.item(t, fx.tasks.ID, "open")
|
||||
fx.tick(t)
|
||||
if _, err := fx.s.CreateComment(fx.ws.ID, it.ID, "", models.CommentCreate{Author: "wren", Body: "more"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
j, err := fx.s.GetDecisionJob(it.ID, AttentionSetName)
|
||||
if err != nil || j == nil || j.NotBefore == "" {
|
||||
t.Fatalf("attention re-ask is not debounced: %+v %v", j, err)
|
||||
}
|
||||
if d := fx.r.debounceFor(ConventionsSetName); d != ReaskDebounce {
|
||||
t.Errorf("conventions window = %v, want %v", d, ReaskDebounce)
|
||||
}
|
||||
if d := fx.r.debounceFor("not-registered"); d != 0 {
|
||||
t.Errorf("an unregistered set has window %v, want none", d)
|
||||
}
|
||||
per := ConventionsCommentsSet()
|
||||
reg := NewRegistry()
|
||||
if err := reg.Register(per); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if d := NewRunner(fx.s, fx.r.provider, reg).debounceFor(per.Name); d != 0 {
|
||||
t.Errorf("a PerComment set has window %v, want none", d)
|
||||
}
|
||||
|
||||
var nilRunner *Runner
|
||||
nilRunner.Install(fx.s)
|
||||
other := fx.item(t, fx.tasks.ID, "open")
|
||||
if j, _ := fx.s.GetDecisionJob(other.ID, AttentionSetName); j != nil {
|
||||
t.Fatalf("a nil runner left the resolver installed: %+v", j)
|
||||
}
|
||||
}
|
||||
|
||||
func (fx *attentionFixture) set(key string, p float64) {
|
||||
fx.mu.Lock()
|
||||
fx.answers[key] = p
|
||||
|
||||
@@ -432,9 +432,27 @@ func (r *Runner) Provider() Provider {
|
||||
func (r *Runner) Install(s *store.Store) {
|
||||
if r == nil {
|
||||
s.SetDecisionSetResolver(nil)
|
||||
s.SetDecisionDebounce(nil)
|
||||
return
|
||||
}
|
||||
s.SetDecisionSetResolver(r.registry.SetsFor)
|
||||
s.SetDecisionDebounce(r.debounceFor)
|
||||
}
|
||||
|
||||
// ReaskDebounce is how long after an item's latest evaluation for a set a new
|
||||
// write's re-ask waits (TASK-3916; the 1h window was accepted for IDEA-3906,
|
||||
// whose trail carries the measured saving). A first evaluation never waits.
|
||||
const ReaskDebounce = time.Hour
|
||||
|
||||
// debounceFor is the window the store applies per set: ReaskDebounce for an
|
||||
// item-level set, none for a PerComment one, whose calls are one per comment
|
||||
// and which a debounce cannot merge, and none for a set not registered.
|
||||
func (r *Runner) debounceFor(setName string) time.Duration {
|
||||
qs, ok := r.registry.Get(setName)
|
||||
if !ok || qs.PerComment {
|
||||
return 0
|
||||
}
|
||||
return ReaskDebounce
|
||||
}
|
||||
|
||||
// ErrItemGone reports that the item no longer exists or is soft-deleted: the
|
||||
|
||||
@@ -28,7 +28,9 @@ import (
|
||||
|
||||
const (
|
||||
// defaultDecisionTickInterval bounds how stale a decision is after a
|
||||
// write: at most one interval plus the provider's latency. Decisions are
|
||||
// write whose job is claimable: at most one interval plus the provider's
|
||||
// latency. A re-ask inside its debounce window (decision.ReaskDebounce,
|
||||
// TASK-3916) waits for the window first. Decisions are
|
||||
// advisory reads, not alarms, so a few seconds is well inside what a
|
||||
// reader would notice, and an idle tick costs one indexed scan of an
|
||||
// empty table.
|
||||
|
||||
@@ -902,9 +902,12 @@ func (s *Server) buildDashboardResponse(workspaceID string, r *http.Request) (*D
|
||||
// is the dependency graph's alone (section e). The text-derived blocked
|
||||
// flag was retired in TASK-3915 (IDEA-3906; the measurements are on that
|
||||
// idea's trail).
|
||||
// Only CURRENT answers at or above the threshold come back — the same
|
||||
// currency rule the item page applies — so the checks below need no
|
||||
// threshold of their own beyond naming which key they read.
|
||||
// Answers at or above the threshold come back when they are CURRENT (the
|
||||
// item page's currency rule), or when the item has moved on but its
|
||||
// re-ask is still pending: a job owed with no recorded failure, for an
|
||||
// item the set still applies to (TASK-3915). With the re-ask debounced
|
||||
// (TASK-3916) that pending window can last up to an hour. So the checks
|
||||
// below need no threshold of their own beyond naming which key they read.
|
||||
if nouls, failing, nerr := s.decisionRunner().WorkspaceFlags(workspaceID, decision.AttentionSetName, decision.AttentionThreshold); nerr != nil {
|
||||
markDegraded("attention.decisions", nerr)
|
||||
} else {
|
||||
|
||||
@@ -147,7 +147,9 @@ func TestDecisions_TickEvaluatesAndReadReportsCurrency(t *testing.T) {
|
||||
}
|
||||
|
||||
// A comment through the API: the stored answer is no longer current, and
|
||||
// the next tick re-evaluates.
|
||||
// the re-ask is DEBOUNCED (TASK-3916): owed, with not_before an hour after
|
||||
// the evaluation, and not taken by a tick inside that hour. Once the
|
||||
// window has passed (aged here in the row), the next tick re-evaluates.
|
||||
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/comments", map[string]string{"body": "new facts"})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("comment: %d %s", rr.Code, rr.Body.String())
|
||||
@@ -155,6 +157,22 @@ func TestDecisions_TickEvaluatesAndReadReportsCurrency(t *testing.T) {
|
||||
if b := getDecisions(t, srv, slug, item.Slug); b.Decisions[0].Current {
|
||||
t.Fatal("answer computed before the comment still reads as current")
|
||||
}
|
||||
j, _ := srv.store.GetDecisionJob(item.ID, "triage")
|
||||
if j == nil || j.NotBefore == "" {
|
||||
t.Fatalf("the comment's re-ask is not debounced through the server's wiring: %+v", j)
|
||||
}
|
||||
for i := 0; i < 3; i++ {
|
||||
select {
|
||||
case tick <- time.Now():
|
||||
case <-time.After(time.Second):
|
||||
}
|
||||
}
|
||||
if n := p.count(); n != 1 {
|
||||
t.Fatalf("a tick inside the window re-asked (calls = %d, want 1)", n)
|
||||
}
|
||||
if _, err := srv.store.DB().Exec(`UPDATE decision_jobs SET not_before = NULL WHERE item_id = '` + item.ID + `'`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
runTickUntil(func() bool { return p.count() == 2 })
|
||||
runTickUntil(func() bool {
|
||||
b := getDecisions(t, srv, slug, item.Slug)
|
||||
|
||||
+131
-15
@@ -55,10 +55,84 @@ func (s *Store) decisionSetResolver() DecisionSetResolver {
|
||||
return *p
|
||||
}
|
||||
|
||||
// DecisionDebounce returns the re-ask window for a question set (TASK-3916):
|
||||
// how long after the item's latest evaluation for that set a new write's job
|
||||
// waits before it is claimable. Zero means no wait.
|
||||
type DecisionDebounce func(questionSet string) time.Duration
|
||||
|
||||
// SetDecisionDebounce installs the window function, or removes it with nil
|
||||
// (no set waits). Atomic for the same reason as the resolver: every write
|
||||
// door reads it.
|
||||
func (s *Store) SetDecisionDebounce(d DecisionDebounce) {
|
||||
if d == nil {
|
||||
s.decisionDebounce.Store(nil)
|
||||
return
|
||||
}
|
||||
s.decisionDebounce.Store(&d)
|
||||
}
|
||||
|
||||
func (s *Store) decisionDebounceFunc() DecisionDebounce {
|
||||
p := s.decisionDebounce.Load()
|
||||
if p == nil {
|
||||
return nil
|
||||
}
|
||||
return *p
|
||||
}
|
||||
|
||||
// decisionWindowFor is debounce(set), or zero when no debounce is installed.
|
||||
func decisionWindowFor(debounce DecisionDebounce, questionSet string) time.Duration {
|
||||
if debounce == nil {
|
||||
return 0
|
||||
}
|
||||
return debounce(questionSet)
|
||||
}
|
||||
|
||||
// latestDecisionEvaluationSQL reads the item's latest evaluation for a set;
|
||||
// bind item_id, question_set. Run on the caller's own handle (a tx or the db)
|
||||
// and passed to decisionNotBefore as the row.
|
||||
const latestDecisionEvaluationSQL = `
|
||||
SELECT MAX(evaluated_at) FROM item_decisions WHERE item_id = ? AND question_set = ?`
|
||||
|
||||
// decisionNotBefore is the not_before a job gets, from the row of
|
||||
// latestDecisionEvaluationSQL: that evaluation plus window, or "" (NULL,
|
||||
// claimable now) when the window is zero, the item has never been evaluated
|
||||
// for the set (a first evaluation is never delayed), or the window has
|
||||
// already passed. The arithmetic is in Go, on decisionTime TEXT, so SQLite
|
||||
// and Postgres agree. A caller with no window skips the read entirely.
|
||||
func decisionNotBefore(latest *sql.Row, window time.Duration, now time.Time) (string, error) {
|
||||
var last sql.NullString
|
||||
if err := latest.Scan(&last); err != nil {
|
||||
return "", fmt.Errorf("read latest evaluation: %w", err)
|
||||
}
|
||||
if !last.Valid || last.String == "" {
|
||||
return "", nil
|
||||
}
|
||||
t, err := time.Parse(time.RFC3339Nano, last.String)
|
||||
if err != nil {
|
||||
// An unreadable timestamp must not hold a job back: ask now.
|
||||
return "", nil
|
||||
}
|
||||
nb := t.Add(window)
|
||||
if !nb.After(now) {
|
||||
return "", nil
|
||||
}
|
||||
return decisionTime(nb), nil
|
||||
}
|
||||
|
||||
func nullableText(s string) any {
|
||||
if s == "" {
|
||||
return nil
|
||||
}
|
||||
return s
|
||||
}
|
||||
|
||||
// decisionSetsPtr is the Store field's type, named so store.go's struct can
|
||||
// declare it without importing sync/atomic for one line.
|
||||
type decisionSetsPtr = atomic.Pointer[DecisionSetResolver]
|
||||
|
||||
// decisionDebouncePtr is the debounce field's type, named for the same reason.
|
||||
type decisionDebouncePtr = atomic.Pointer[DecisionDebounce]
|
||||
|
||||
// decisionTimeFormat is FIXED-WIDTH, unlike RFC3339Nano, which drops trailing
|
||||
// zeros. These columns are compared and ordered as TEXT, and a variable-width
|
||||
// fractional part sorts `…05.5Z` after `…05.123Z` but `…05Z` after both —
|
||||
@@ -76,6 +150,11 @@ func decisionTime(t time.Time) string { return t.UTC().Format(decisionTimeFormat
|
||||
// generation instead of adding a row, and clears the failure history, because
|
||||
// the new generation is new work. enqueued_at is NOT moved: an item edited
|
||||
// continuously would otherwise sort to the back of the claim scan forever.
|
||||
//
|
||||
// DEBOUNCE (TASK-3916): a fresh row gets not_before from decisionNotBefore.
|
||||
// A conflicting enqueue KEEPS the row's existing not_before when it has one
|
||||
// (COALESCE(old, new)), so a stream of comments cannot keep pushing the
|
||||
// re-ask back; it only fills one in when the row had none.
|
||||
func (s *Store) enqueueDecisionJobsTx(tx *sql.Tx, workspaceID, itemID, collectionID string) error {
|
||||
resolve := s.decisionSetResolver()
|
||||
if resolve == nil {
|
||||
@@ -89,16 +168,26 @@ func (s *Store) enqueueDecisionJobsTx(tx *sql.Tx, workspaceID, itemID, collectio
|
||||
if len(sets) == 0 {
|
||||
return nil
|
||||
}
|
||||
ts := decisionTime(time.Now())
|
||||
debounce := s.decisionDebounceFunc()
|
||||
now := time.Now()
|
||||
ts := decisionTime(now)
|
||||
for _, set := range sets {
|
||||
notBefore := ""
|
||||
if window := decisionWindowFor(debounce, set); window > 0 {
|
||||
var err error
|
||||
if notBefore, err = decisionNotBefore(tx.QueryRow(s.q(latestDecisionEvaluationSQL), itemID, set), window, now); err != nil {
|
||||
return fmt.Errorf("enqueue decision job %s/%s: %w", itemID, set, err)
|
||||
}
|
||||
}
|
||||
if _, err := tx.Exec(s.q(`
|
||||
INSERT INTO decision_jobs (item_id, workspace_id, question_set, generation, enqueued_at, attempts)
|
||||
VALUES (?, ?, ?, 1, ?, 0)
|
||||
INSERT INTO decision_jobs (item_id, workspace_id, question_set, generation, enqueued_at, attempts, not_before)
|
||||
VALUES (?, ?, ?, 1, ?, 0, ?)
|
||||
ON CONFLICT (item_id, question_set) DO UPDATE SET
|
||||
generation = decision_jobs.generation + 1,
|
||||
attempts = 0,
|
||||
last_error = NULL`),
|
||||
itemID, workspaceID, set, ts,
|
||||
last_error = NULL,
|
||||
not_before = COALESCE(decision_jobs.not_before, excluded.not_before)`),
|
||||
itemID, workspaceID, set, ts, nullableText(notBefore),
|
||||
); err != nil {
|
||||
return fmt.Errorf("enqueue decision job %s/%s: %w", itemID, set, err)
|
||||
}
|
||||
@@ -134,8 +223,9 @@ type DecisionJob struct {
|
||||
|
||||
// decisionJobClaimable is the single definition of "this job may be taken",
|
||||
// used by BOTH the candidate scan and the claim UPDATE's arbiter so the two
|
||||
// cannot drift (the reminderFireable lesson). Its one parameter is the current
|
||||
// time: an unclaimed row, or one whose lease has expired.
|
||||
// cannot drift (the reminderFireable lesson). Its TWO parameters are both the
|
||||
// current time: an unclaimed row, or one whose lease has expired, and whose
|
||||
// debounce (not_before, TASK-3916) has passed or was never set.
|
||||
//
|
||||
// A job whose WORKSPACE is soft-deleted is not claimable: it is held in place
|
||||
// so a workspace restore finds the work still owed (codex round 5). Nothing
|
||||
@@ -143,6 +233,7 @@ type DecisionJob struct {
|
||||
// the row is the only thing that makes a restore resume. A purge hard-deletes
|
||||
// the items, and the composite FK cascades the held rows away.
|
||||
const decisionJobClaimable = `(claimed_by IS NULL OR lease_expires_at IS NULL OR lease_expires_at < ?)
|
||||
AND (not_before IS NULL OR not_before <= ?)
|
||||
AND NOT EXISTS (SELECT 1 FROM workspaces w WHERE w.id = decision_jobs.workspace_id AND w.deleted_at IS NOT NULL)`
|
||||
|
||||
// ClaimDecisionJobs claims up to limit owed jobs for runner, oldest first,
|
||||
@@ -164,7 +255,7 @@ func (s *Store) ClaimDecisionJobs(runner string, limit int, lease time.Duration)
|
||||
FROM decision_jobs
|
||||
WHERE `+decisionJobClaimable+`
|
||||
ORDER BY enqueued_at, item_id, question_set
|
||||
LIMIT ?`), nowS, limit)
|
||||
LIMIT ?`), nowS, nowS, limit)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("scan decision jobs: %w", err)
|
||||
}
|
||||
@@ -190,7 +281,7 @@ func (s *Store) ClaimDecisionJobs(runner string, limit int, lease time.Duration)
|
||||
UPDATE decision_jobs SET claimed_by = ?, lease_expires_at = ?
|
||||
WHERE item_id = ? AND question_set = ? AND generation = ?
|
||||
AND `+decisionJobClaimable),
|
||||
runner, until, j.ItemID, j.QuestionSet, j.Generation, nowS)
|
||||
runner, until, j.ItemID, j.QuestionSet, j.Generation, nowS, nowS)
|
||||
if err != nil {
|
||||
return claimed, fmt.Errorf("claim decision job %s/%s: %w", j.ItemID, j.QuestionSet, err)
|
||||
}
|
||||
@@ -208,6 +299,13 @@ func (s *Store) ClaimDecisionJobs(runner string, limit int, lease time.Duration)
|
||||
// write re-enqueued it mid-evaluation, the row stays and the claim is
|
||||
// released, so the next tick evaluates the newer state — the finished run
|
||||
// must not delete work it never saw.
|
||||
//
|
||||
// That released row's not_before is RECOMPUTED (TASK-3916): the write that
|
||||
// bumped it read the latest evaluation BEFORE this run's answers landed, so
|
||||
// it may have found the window past and left the row claimable at once. The
|
||||
// recomputation sees them, and the newer state is asked about a window after
|
||||
// this run, not on the very next tick. A run that stored nothing (its state
|
||||
// was already answered) moves nothing, so this never invents a delay.
|
||||
func (s *Store) CompleteDecisionJob(j DecisionJob) error {
|
||||
res, err := s.db.Exec(s.q(`
|
||||
DELETE FROM decision_jobs
|
||||
@@ -219,6 +317,21 @@ func (s *Store) CompleteDecisionJob(j DecisionJob) error {
|
||||
if n, _ := res.RowsAffected(); n == 1 {
|
||||
return nil
|
||||
}
|
||||
notBefore := ""
|
||||
if window := decisionWindowFor(s.decisionDebounceFunc(), j.QuestionSet); window > 0 {
|
||||
if notBefore, err = decisionNotBefore(s.db.QueryRow(s.q(latestDecisionEvaluationSQL), j.ItemID, j.QuestionSet), window, time.Now()); err != nil {
|
||||
return fmt.Errorf("debounce superseded decision job %s/%s: %w", j.ItemID, j.QuestionSet, err)
|
||||
}
|
||||
}
|
||||
if notBefore != "" {
|
||||
if _, err := s.db.Exec(s.q(`
|
||||
UPDATE decision_jobs SET not_before = ?
|
||||
WHERE item_id = ? AND question_set = ? AND claimed_by = ?
|
||||
AND (not_before IS NULL OR not_before < ?)`),
|
||||
notBefore, j.ItemID, j.QuestionSet, j.ClaimedBy, notBefore); err != nil {
|
||||
return fmt.Errorf("debounce superseded decision job %s/%s: %w", j.ItemID, j.QuestionSet, err)
|
||||
}
|
||||
}
|
||||
return s.releaseDecisionJob(j, "", 0)
|
||||
}
|
||||
|
||||
@@ -258,7 +371,8 @@ func (s *Store) DropDecisionJob(j DecisionJob) error {
|
||||
|
||||
// ReleaseDecisionJob gives a claimed job back without recording a failure:
|
||||
// the runner stopped before finishing it (shutdown), which says nothing about
|
||||
// the job. It is claimable again at once.
|
||||
// the job. It is claimable again at once, unless its debounce (not_before)
|
||||
// is still ahead.
|
||||
func (s *Store) ReleaseDecisionJob(j DecisionJob) error {
|
||||
return s.releaseDecisionJob(j, "", 0)
|
||||
}
|
||||
@@ -266,7 +380,7 @@ func (s *Store) ReleaseDecisionJob(j DecisionJob) error {
|
||||
func (s *Store) releaseDecisionJob(j DecisionJob, failure string, retryAfter time.Duration) error {
|
||||
if failure == "" {
|
||||
// Superseded: release the claim so the newer generation is claimable
|
||||
// at once.
|
||||
// as soon as its not_before allows (at once when it has none).
|
||||
_, err := s.db.Exec(s.q(`
|
||||
UPDATE decision_jobs SET claimed_by = NULL, lease_expires_at = NULL
|
||||
WHERE item_id = ? AND question_set = ? AND claimed_by = ?`),
|
||||
@@ -318,23 +432,25 @@ type DecisionJobState struct {
|
||||
Attempts int
|
||||
ClaimedBy string
|
||||
LastError string
|
||||
// NotBefore is the debounce instant (TASK-3916), "" when claimable now.
|
||||
NotBefore string
|
||||
}
|
||||
|
||||
// GetDecisionJob returns a job's row, or nil when no evaluation is owed.
|
||||
func (s *Store) GetDecisionJob(itemID, questionSet string) (*DecisionJobState, error) {
|
||||
var st DecisionJobState
|
||||
var claimedBy, lastError sql.NullString
|
||||
var claimedBy, lastError, notBefore sql.NullString
|
||||
err := s.db.QueryRow(s.q(`
|
||||
SELECT generation, attempts, claimed_by, last_error FROM decision_jobs
|
||||
SELECT generation, attempts, claimed_by, last_error, not_before FROM decision_jobs
|
||||
WHERE item_id = ? AND question_set = ?`), itemID, questionSet).
|
||||
Scan(&st.Generation, &st.Attempts, &claimedBy, &lastError)
|
||||
Scan(&st.Generation, &st.Attempts, &claimedBy, &lastError, ¬Before)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return nil, nil
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("get decision job: %w", err)
|
||||
}
|
||||
st.ClaimedBy, st.LastError = claimedBy.String, lastError.String
|
||||
st.ClaimedBy, st.LastError, st.NotBefore = claimedBy.String, lastError.String, notBefore.String
|
||||
return &st, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,281 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// The re-ask debounce (TASK-3916): decision_jobs.not_before. Runs on both
|
||||
// backends via testStore (PAD_TEST_POSTGRES_URL selects Postgres).
|
||||
|
||||
const testDebounce = time.Hour
|
||||
|
||||
// debounceFixture installs a "triage" set with an hour's window and a
|
||||
// "per-comment" set with none.
|
||||
func debounceFixture(t *testing.T, sets ...string) (*Store, *models.Workspace, *models.Collection) {
|
||||
t.Helper()
|
||||
s, ws, col := decisionFixture(t)
|
||||
if len(sets) == 0 {
|
||||
sets = []string{"triage"}
|
||||
}
|
||||
s.SetDecisionSetResolver(func(string) []string { return sets })
|
||||
s.SetDecisionDebounce(func(set string) time.Duration {
|
||||
if set == "per-comment" {
|
||||
return 0
|
||||
}
|
||||
return testDebounce
|
||||
})
|
||||
return s, ws, col
|
||||
}
|
||||
|
||||
// evaluated stores one answer for (item, set) at `at`, as the runner does when
|
||||
// it completes an evaluation.
|
||||
func evaluated(t *testing.T, s *Store, ws *models.Workspace, itemID, set string, at time.Time) {
|
||||
t.Helper()
|
||||
if err := s.InsertItemDecisions(ws.ID, []models.ItemDecision{{
|
||||
ItemID: itemID, QuestionSet: set, QuestionKey: "k", Kind: "noul",
|
||||
Answer: []byte(`{"type":"noul","noul":0.5}`), Provider: "fake", Model: "m",
|
||||
StateHash: "h-" + at.Format(time.RFC3339Nano), QuestionHash: "q", ItemSeq: 1, EvaluatedAt: decisionTime(at),
|
||||
}}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func claimN(t *testing.T, s *Store) []DecisionJob {
|
||||
t.Helper()
|
||||
c, err := s.ClaimDecisionJobs("r", 10, time.Minute)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return c
|
||||
}
|
||||
|
||||
func retitle(t *testing.T, s *Store, itemID, title string) {
|
||||
t.Helper()
|
||||
if _, err := s.UpdateItem(itemID, models.ItemUpdate{Title: &title}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
// A first evaluation is never delayed: an item with no answers for the set is
|
||||
// claimable at once, with not_before NULL.
|
||||
func TestDecisionJobs_FirstEvaluationNotDebounced(t *testing.T) {
|
||||
s, ws, col := debounceFixture(t)
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Fresh", "")
|
||||
if j := mustJob(t, s, item.ID, "triage"); j.NotBefore != "" {
|
||||
t.Fatalf("a first evaluation was given not_before %q", j.NotBefore)
|
||||
}
|
||||
if c := claimN(t, s); len(c) != 1 {
|
||||
t.Fatalf("claimed %d, want the first evaluation at once", len(c))
|
||||
}
|
||||
}
|
||||
|
||||
// A write inside the window after an evaluation owes a job that is NOT
|
||||
// claimable until latest evaluation + window. The control legs: the same job
|
||||
// IS claimable once the window has passed (the clock is moved by aging the
|
||||
// evaluation), and a write after a window already past is claimable at once.
|
||||
func TestDecisionJobs_DebounceDefersReask(t *testing.T) {
|
||||
s, ws, col := debounceFixture(t)
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Busy", "")
|
||||
c := claimN(t, s)
|
||||
if len(c) != 1 {
|
||||
t.Fatalf("precondition: claim the first job, got %d", len(c))
|
||||
}
|
||||
at := time.Now().Add(-10 * time.Minute)
|
||||
evaluated(t, s, ws, item.ID, "triage", at)
|
||||
if err := s.CompleteDecisionJob(c[0]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
retitle(t, s, item.ID, "Busy, edited")
|
||||
j := mustJob(t, s, item.ID, "triage")
|
||||
if want := decisionTime(at.Add(testDebounce)); j.NotBefore != want {
|
||||
t.Fatalf("not_before = %q, want latest evaluation + window %q", j.NotBefore, want)
|
||||
}
|
||||
if c := claimN(t, s); len(c) != 0 {
|
||||
t.Fatalf("a job inside its window was claimed: %+v", c)
|
||||
}
|
||||
|
||||
// The window passes: age the job's not_before into the past.
|
||||
if _, err := s.db.Exec(s.q(`UPDATE decision_jobs SET not_before = ? WHERE item_id = ?`),
|
||||
decisionTime(time.Now().Add(-time.Second)), item.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if c := claimN(t, s); len(c) != 1 {
|
||||
t.Fatalf("a job whose window has passed was not claimed (%d)", len(c))
|
||||
}
|
||||
|
||||
// An evaluation older than the window: a new write is claimable at once.
|
||||
other := createTestItem(t, s, ws.ID, col.ID, "Quiet", "")
|
||||
if c := claimN(t, s); len(c) != 1 {
|
||||
t.Fatalf("precondition: claim the second item's first job, got %d", len(c))
|
||||
} else {
|
||||
evaluated(t, s, ws, other.ID, "triage", time.Now().Add(-2*testDebounce))
|
||||
if err := s.CompleteDecisionJob(c[0]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
retitle(t, s, other.ID, "Quiet, edited")
|
||||
if j := mustJob(t, s, other.ID, "triage"); j.NotBefore != "" {
|
||||
t.Fatalf("a window already past gave not_before %q", j.NotBefore)
|
||||
}
|
||||
}
|
||||
|
||||
// Racing writes coalesce: the second enqueue keeps the first not_before
|
||||
// (COALESCE(old, new)) rather than pushing it back, so a stream of comments
|
||||
// cannot postpone the re-ask indefinitely.
|
||||
func TestDecisionJobs_DebounceCoalescesRacingComments(t *testing.T) {
|
||||
s, ws, col := debounceFixture(t)
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Chatty", "")
|
||||
c := claimN(t, s)
|
||||
at := time.Now().Add(-30 * time.Minute)
|
||||
evaluated(t, s, ws, item.ID, "triage", at)
|
||||
if err := s.CompleteDecisionJob(c[0]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if _, err := s.CreateComment(ws.ID, item.ID, "", models.CommentCreate{Author: "a", Body: "one"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
first := mustJob(t, s, item.ID, "triage")
|
||||
if first.NotBefore == "" {
|
||||
t.Fatal("precondition: the first comment's job is not debounced")
|
||||
}
|
||||
// A newer evaluation lands (as if another path answered), then a second
|
||||
// comment: its computed not_before would be LATER, and must not win.
|
||||
evaluated(t, s, ws, item.ID, "triage", time.Now())
|
||||
if _, err := s.CreateComment(ws.ID, item.ID, "", models.CommentCreate{Author: "a", Body: "two"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
second := mustJob(t, s, item.ID, "triage")
|
||||
if second.Generation != first.Generation+1 {
|
||||
t.Fatalf("generation %d -> %d, want one coalesced row bumped once", first.Generation, second.Generation)
|
||||
}
|
||||
if second.NotBefore != first.NotBefore {
|
||||
t.Fatalf("not_before moved from %q to %q under a racing comment", first.NotBefore, second.NotBefore)
|
||||
}
|
||||
if n := countDecisionJobs(t, s); n != 1 {
|
||||
t.Fatalf("%d job rows, want 1", n)
|
||||
}
|
||||
}
|
||||
|
||||
// A write that lands DURING an evaluation leaves the row behind (the
|
||||
// generation rule). Its enqueue read the latest evaluation BEFORE the run's
|
||||
// answers landed, so it found no window; the superseded completion recomputes
|
||||
// not_before from the answers the run just stored, so the newer state waits a
|
||||
// window after this run instead of being asked on the very next tick.
|
||||
func TestDecisionJobs_SupersededCompletionSetsNotBefore(t *testing.T) {
|
||||
s, ws, col := debounceFixture(t)
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Race", "")
|
||||
c := claimN(t, s)
|
||||
retitle(t, s, item.ID, "Race, edited mid-evaluation")
|
||||
if j := mustJob(t, s, item.ID, "triage"); j.NotBefore != "" {
|
||||
t.Fatalf("precondition: the mid-run enqueue found an evaluation (%q)", j.NotBefore)
|
||||
}
|
||||
// Backdated so a now+window mutant differs by minutes, not microseconds.
|
||||
ran := time.Now().Add(-5 * time.Minute)
|
||||
evaluated(t, s, ws, item.ID, "triage", ran)
|
||||
if err := s.CompleteDecisionJob(c[0]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
j := mustJob(t, s, item.ID, "triage")
|
||||
if j.Generation != 2 || j.ClaimedBy != "" {
|
||||
t.Fatalf("superseded completion: generation=%d claimed_by=%q, want 2 and released", j.Generation, j.ClaimedBy)
|
||||
}
|
||||
if want := decisionTime(ran.Add(testDebounce)); j.NotBefore != want {
|
||||
t.Fatalf("not_before = %q, want the run's evaluation + window %q", j.NotBefore, want)
|
||||
}
|
||||
if c := claimN(t, s); len(c) != 0 {
|
||||
t.Fatalf("the superseding generation was claimable on the next tick: %+v", c)
|
||||
}
|
||||
}
|
||||
|
||||
// A superseded run that STORED nothing (its state was already answered)
|
||||
// recomputes from the older evaluation and invents no delay (ruling
|
||||
// ae2e0a90): not_before is that evaluation plus the window while it is open,
|
||||
// and NULL once it has passed, never this completion's now plus the window.
|
||||
// One subtest per leg, so each can go red on its own.
|
||||
func TestDecisionJobs_SupersededNoCallKeepsTheOlderWindow(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
age time.Duration
|
||||
want func(older time.Time) string
|
||||
}{
|
||||
{"window past", 3 * testDebounce, func(time.Time) string { return "" }},
|
||||
{"window open", testDebounce / 2, func(older time.Time) string { return decisionTime(older.Add(testDebounce)) }},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
s, ws, col := debounceFixture(t)
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Idle", "")
|
||||
older := time.Now().Add(-tc.age)
|
||||
evaluated(t, s, ws, item.ID, "triage", older)
|
||||
c := claimN(t, s)
|
||||
if len(c) != 1 {
|
||||
t.Fatalf("precondition: claim the item, got %d", len(c))
|
||||
}
|
||||
retitle(t, s, item.ID, "Idle, edited")
|
||||
if err := s.CompleteDecisionJob(c[0]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
j := mustJob(t, s, item.ID, "triage")
|
||||
if want := tc.want(older); j.NotBefore != want || j.ClaimedBy != "" {
|
||||
t.Fatalf("not_before=%q claimed_by=%q, want %q and released", j.NotBefore, j.ClaimedBy, want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The window runs from the LATEST evaluation, not the first: an item asked
|
||||
// two hours ago and again ten minutes ago is inside its window.
|
||||
func TestDecisionJobs_DebounceUsesLatestEvaluation(t *testing.T) {
|
||||
s, ws, col := debounceFixture(t)
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Twice", "")
|
||||
c := claimN(t, s)
|
||||
if len(c) != 1 {
|
||||
t.Fatalf("precondition: claim the first job, got %d", len(c))
|
||||
}
|
||||
evaluated(t, s, ws, item.ID, "triage", time.Now().Add(-2*testDebounce))
|
||||
latest := time.Now().Add(-10 * time.Minute)
|
||||
evaluated(t, s, ws, item.ID, "triage", latest)
|
||||
if err := s.CompleteDecisionJob(c[0]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
retitle(t, s, item.ID, "Twice, edited")
|
||||
if j := mustJob(t, s, item.ID, "triage"); j.NotBefore != decisionTime(latest.Add(testDebounce)) {
|
||||
t.Fatalf("not_before = %q, want the latest evaluation + window %q", j.NotBefore, decisionTime(latest.Add(testDebounce)))
|
||||
}
|
||||
}
|
||||
|
||||
// A set with no window (a per-comment set) is never debounced, beside one
|
||||
// that is: the window is per set, not global.
|
||||
func TestDecisionJobs_PerCommentSetNotDebounced(t *testing.T) {
|
||||
s, ws, col := debounceFixture(t, "triage", "per-comment")
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Both", "")
|
||||
c := claimN(t, s)
|
||||
if len(c) != 2 {
|
||||
t.Fatalf("precondition: claim both first jobs, got %d", len(c))
|
||||
}
|
||||
now := time.Now().Add(-time.Minute)
|
||||
evaluated(t, s, ws, item.ID, "triage", now)
|
||||
evaluated(t, s, ws, item.ID, "per-comment", now)
|
||||
for _, j := range c {
|
||||
if err := s.CompleteDecisionJob(j); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if _, err := s.CreateComment(ws.ID, item.ID, "", models.CommentCreate{Author: "a", Body: "hi"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if j := mustJob(t, s, item.ID, "per-comment"); j.NotBefore != "" {
|
||||
t.Fatalf("the per-comment set was debounced to %q", j.NotBefore)
|
||||
}
|
||||
if j := mustJob(t, s, item.ID, "triage"); j.NotBefore == "" {
|
||||
t.Fatal("control: the item-level set beside it was not debounced")
|
||||
}
|
||||
got := claimN(t, s)
|
||||
if len(got) != 1 || got[0].QuestionSet != "per-comment" {
|
||||
t.Fatalf("claimed %+v, want only the per-comment job", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,12 @@
|
||||
-- Migration 150: decision_jobs.not_before (TASK-3916, IDEA-3906 half B).
|
||||
--
|
||||
-- The re-ask debounce. A job whose not_before is in the future is owed but
|
||||
-- not yet claimable. The enqueue computes it in Go from the item's latest
|
||||
-- evaluation for the set plus the set's window (store.SetDecisionDebounce:
|
||||
-- 1h for the item-level sets, 0 for a per-comment one), so a burst of edits
|
||||
-- and comments costs at most one provider call an hour per set rather than one per write.
|
||||
-- NULL means claimable now: a first evaluation, a window already past, or a
|
||||
-- set with no window. Fixed-width decisionTime TEXT, the same format and the
|
||||
-- same TEXT comparison as lease_expires_at. Never backfilled: a row written
|
||||
-- before this migration is claimable now, as it was.
|
||||
ALTER TABLE decision_jobs ADD COLUMN not_before TEXT;
|
||||
@@ -451,6 +451,7 @@ var nulExcluded = map[string]string{
|
||||
"items.lease_worker_id": "request-derived, but written only after models.ValidWorkerID accepts it (\"wk_\" + 32 lowercase hex, enforced in store.leaseOurs before any SQL), so it cannot hold a NUL (TASK-3613, migration 142)",
|
||||
"items.lease_user_id": "the authenticated caller's server-generated user id, set only by ClaimItemLease from the session; never request-derived (BUG-3341, migration 104)",
|
||||
"decision_jobs.last_error": "Go error string, server-composed",
|
||||
"decision_jobs.not_before": "server timestamp, decisionTime of an evaluated_at plus a server-installed window (TASK-3916, migration 150)",
|
||||
"item_decisions.answer": "json.Marshal of decision.Answer: a NUL anywhere in it is written as the six-byte escape, never a raw byte, and the column is TEXT on both dialects so no jsonb parser decodes it (migration 090)",
|
||||
"item_relation_links.source_field_key": "only written by replaceRelationLinks, as a field definition's key value read from collections.schema in the same transaction; that column's classJSON trigger checks every decoded value; migration 088 states the derivation (BUG-3108 writer list)",
|
||||
"item_yjs_updates_set_aside.update_data": "BINARY (BLOB/BYTEA): op-log frames a schema rebuild moved here verbatim (BUG-3244), so the same raw Yjs bytes as item_yjs_updates.update_data, NULs included, and exempt for the same reason. A bundle import writes it only after base64-decoding the bundle's bytes.",
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
-- Migration 120 (Postgres): decision_jobs.not_before (TASK-3916).
|
||||
-- Postgres counterpart to migrations/150_decision_jobs_not_before.sql; the rationale is there.
|
||||
ALTER TABLE decision_jobs ADD COLUMN IF NOT EXISTS not_before TEXT;
|
||||
@@ -94,6 +94,9 @@ type Store struct {
|
||||
// item (TASK-3117). Empty until the server installs a resolver, which it
|
||||
// does only when a decision provider is configured; see decisions.go.
|
||||
decisionSets decisionSetsPtr
|
||||
// decisionDebounce is the per-set re-ask window (TASK-3916), installed
|
||||
// by the decision runner beside decisionSets. Nil means no set waits.
|
||||
decisionDebounce decisionDebouncePtr
|
||||
|
||||
// outboxRowCapOverride and outboxClaimBudgetOverride are TEST-ONLY seams,
|
||||
// zero in production. They lower MaxOutboxPayloadBytes and
|
||||
|
||||
Reference in New Issue
Block a user