diff --git a/PERF.md b/PERF.md index 4f55162ef3..f0a12d2748 100644 --- a/PERF.md +++ b/PERF.md @@ -1283,6 +1283,46 @@ a rule in two places is the cheapest thing in a budget to spend twice. `wide`, `model` is unchanged. Every em dash came out of every description string, small models tokenising them badly. The tool block went **23,369 → 21,808**. +**Both caps were weighing a conversation nobody has, and are re-measured +(2026-09-12, #996).** Every figure above is real and every one of them was +measured against a fixture with no memory store, no accounts hub, no standing +items and no saved programs — eighteen tools on the full arm, sixteen on the +lean one. A machine somebody has finished setting up carries five more (`stand`, +`search_conversations`, `remember` and the subharness pair), and the shipped +prefix has been **over the cap it was enforcing** for as long as that shape has +existed. A gate pointed at something nobody runs passes without having tested +anything, which is #576's shape. + +| | weighed | cap | target owed | +| --- | --- | --- | --- | +| full (`fixedPrefixBudget`) | **53,141** | 53,141 | 48,000, **5,141** over | +| lean (`leanPrefixBudget`) | **44,916** | 44,916 | 31,500, **13,416** over | + +**The cap is the target plus a dated waiver, and a waiver only ever shrinks.** +`fixedPrefixTarget` and `leanPrefixTarget` are what each arm is supposed to be; +`prefixWaivers` holds what each is over by, in one place, with the date and the +reason. That is `.github/known-red.txt`'s discipline applied to bytes: a wave +that grows the prefix raises a waiver in a diff with its name on it, and a wave +that pays some back lowers it in the same commit. It replaced a `t.Logf`, which +nothing in the Makefile passes `-v` to — the one line saying the prefix was +thousands of bytes over was printed where nobody would ever see it. + +The bill is not spread thin: `stand` is **9,607 bytes**, more than a quarter of +the full tool block and nearly twice the next heaviest, and on a +sixteen-thousand-token window it is roughly one token in six of everything that +person has before they have said anything. Trimming it is a change to what the +model is told, so it is owed by whoever owns the standing belt and not by this +gate. + +**And the prefix is not the same size on every machine, which is why both +figures are the widest machine's.** `grep` says a longer sentence about itself +where ripgrep is absent (**134 bytes**) and `load_capability` names the tool +groups this build has, so `edit_video` — built only where ffmpeg is on PATH — +costs another **27**. The first spelling of the full cap was 53,100, measured at +53,025 on the laptop that wrote it, and it failed CI at 53,132. `widestBelt` +weighs both varying sentences at their widest wherever it runs, so the cap no +longer depends on who runs it. + ## Following through on a completion claim A turn may decline handoff once per request when its own continuation says no diff --git a/cmd/aforge-replay/replay_test.go b/cmd/aforge-replay/replay_test.go index d072c543b7..fc39042b30 100644 --- a/cmd/aforge-replay/replay_test.go +++ b/cmd/aforge-replay/replay_test.go @@ -13,12 +13,14 @@ import ( "sort" "strconv" "strings" + "sync" "testing" "time" "github.com/Agent-Field/aforge-v2/internal/calllog" "github.com/Agent-Field/aforge-v2/internal/callrows" "github.com/Agent-Field/aforge-v2/internal/lane" + "github.com/Agent-Field/aforge-v2/internal/roles" ) // ── THE INSTRUMENT'S OWN LAW ──────────────────────────────────────────────── @@ -241,13 +243,6 @@ func TestEveryTagTheBuildWritesResolvesToARoleSomebodyDeclared(t *testing.T) { // that passes forever the moment somebody builds a tag out of a variable, which // is exactly how the log came to carry 2,309 untagged rows in the first place. var derivedTags = map[string]func(yield func(tag, where string)){ - // internal/session/auxiliary.go: every errand is tagged with its own role - // word, so the whole of internal/roles' vocabulary reaches the log. - "string(role)": func(yield func(tag, where string)) { - for _, word := range roleWords() { - yield(word, "an internal/roles constant, written by internal/session/auxiliary.go") - } - }, // internal/session/toolask.go: a tool's own call carries the tool's name // after the role word, and no walk can enumerate the belt. "toolCallTag(question.tool)": func(yield func(tag, where string)) { @@ -257,6 +252,66 @@ var derivedTags = map[string]func(yield func(tag, where string)){ // cmd/aforge/chat.go's errandContext passes the errand's own name straight // through; its call sites are walked for the literals they hand it. "task": func(yield func(tag, where string)) {}, + // internal/session/clientdoor.go is the ONE place that package writes a tag + // now, and what it writes is whatever purpose its caller handed it. It + // enumerates nothing on its own: the words come from the door's own call + // sites, which the walk below reads through the same rules. + "string(purpose)": func(yield func(tag, where string)) {}, + // internal/session/auxiliary.go hands the door the errand's own role word, so + // the whole of internal/roles' vocabulary reaches the log. The spelling is + // what is left after `callPurpose(...)` comes off the argument. + "role": func(yield func(tag, where string)) { + for _, word := range roleWords() { + yield(word, "an internal/roles constant, written by internal/session/auxiliary.go") + } + }, +} + +// purposeWords and roleWords are the two vocabularies this law joins, each read +// from the ONE FILE that declares it and each read ONCE per run. +// +// They are [constWordsOfType] twice. The purposes that are a role's own name +// need no entry in either table — they arrive through the role vocabulary — and +// what is left in the door's own block is the handful that are not any role. +var purposeWords = sync.OnceValue(func() []string { + return constWordsOfType(filepath.Join("internal", "session", "clientdoor.go"), "callPurpose") +}) + +var roleWords = sync.OnceValue(func() []string { + return constWordsOfType(filepath.Join("internal", "roles", "roles.go"), "Role") +}) + +// constWordsOfType reads one file for the string constants declared with one +// named type, and PANICS ON AN EMPTY ANSWER. +// +// The panic is the point. This law joins two vocabularies by parsing the files +// that declare them, and a parse that silently found nothing would make the join +// trivially complete — every tag resolved, no row missing, the report quietly +// wrong. A file renamed out from under it has to be loud. +func constWordsOfType(relative, typeName string) []string { + path := filepath.Join(moduleRoot, relative) + file, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution) + if err != nil { + panic("reading " + relative + ": " + err.Error()) + } + var words []string + ast.Inspect(file, func(node ast.Node) bool { + spec, isValue := node.(*ast.ValueSpec) + if !isValue || len(spec.Values) != 1 { + return true + } + if kind, isIdent := spec.Type.(*ast.Ident); !isIdent || kind.Name != typeName { + return true + } + if word, literal := stringLiteral(spec.Values[0]); literal && word != "" { + words = append(words, word) + } + return true + }) + if len(words) == 0 { + panic(relative + " declares no " + typeName + " constants; this law is reading the wrong tree") + } + return words } // tagsTheBuildWrites reads the module's sources for every tag that can reach @@ -270,8 +325,8 @@ func tagsTheBuildWrites(t *testing.T) map[string]string { if !isCall { return true } - switch callName(call.Fun) { - case "WithCallTag", "provider.WithCallTag": + switch methodName(call.Fun) { + case "WithCallTag": if len(call.Args) < 2 { return true } @@ -287,6 +342,60 @@ func tagsTheBuildWrites(t *testing.T) map[string]string { return true } derived(func(tag, where string) { found[tag] = where }) + case "completeWithModel", "completeWithNamedModel": + // THE DOOR IS WHERE internal/session'S TAGS ARE STATED NOW. Its + // second argument is a [session.callPurpose], and it reaches + // `calllog.Record.Tag` verbatim (clientdoor.go). Reading it here is + // what keeps this law pointed at the same thing the build is: the + // `WithCallTag` case above sees only the one relay line. + if len(call.Args) < 2 { + return true + } + purpose := call.Args[1] + // A PURPOSE MAY WEAR ITS TYPE. `callPurpose("...")` and a bare + // constant are the same statement, so the conversion comes off + // before the argument is read. + if conversion, isCall := purpose.(*ast.CallExpr); isCall && + methodName(conversion.Fun) == "callPurpose" && len(conversion.Args) == 1 { + if _, literal := stringLiteral(conversion.Args[0]); !literal { + // A ROLE WORN AS A PURPOSE IS STILL A ROLE, and the + // vocabulary it can produce is internal/roles' own. It is + // over-approximated deliberately: this law asks whether + // every tag the build CAN write is named, so answering + // with more of them than one site writes is safe, where + // answering with fewer is the hole it exists to close. + if strings.Contains(source(t, conversion.Args[0]), "Role") { + for _, word := range roleWords() { + found[word] = path + ", a role worn as a purpose" + } + return true + } + } + purpose = conversion.Args[0] + } + if word, literal := stringLiteral(purpose); literal { + if word != "" { + found[word] = path + } + return true + } + // A NAMED CONSTANT IS THE ORDINARY CASE and the door declares them + // all in one block, so they are read from the declaration rather + // than listed here. + if ident, isIdent := purpose.(*ast.Ident); isIdent && strings.HasPrefix(ident.Name, "purpose") { + for _, word := range purposeWords() { + found[word] = "an internal/session/clientdoor.go callPurpose constant" + } + return true + } + spelling := source(t, purpose) + derived, known := derivedTags[spelling] + if !known { + t.Errorf("%s hands the model door a purpose spelled %q, which this law cannot "+ + "read; add it to derivedTags with what it can produce", path, spelling) + return true + } + derived(func(tag, where string) { found[tag] = where }) case "errandContext": // func errandContext(ctx, settings, task string, role lane.Role) if len(call.Args) < 3 { @@ -305,29 +414,123 @@ func tagsTheBuildWrites(t *testing.T) map[string]string { return found } -// roleWords reads internal/roles for the role vocabulary itself, because that -// package's constants ARE tags the moment auxiliary.go writes one. -func roleWords() []string { - var words []string - set := token.NewFileSet() - file, err := parser.ParseFile(set, filepath.Join(moduleRoot, "internal", "roles", "roles.go"), nil, 0) - if err != nil { - panic("reading internal/roles: " + err.Error()) +// TestEveryRoleWordIsDeclaredWhereTheVocabularyIs fails a role named anywhere +// but internal/roles, in either spelling. +// +// [roleWords] parses ONE FILE, because that is where this build says the role +// vocabulary lives and because the tool that reads it cannot import the engine +// to ask the registry at run time — a replay reader would pull in the whole +// session package for a list of words. A role constant declared at its own call +// site is therefore invisible to it, and to every other reader that asks what +// the roles ARE. +// +// THE MEASURED FAILURE. `const spellOutRole roles.Role = "spellout"` lived in +// internal/session/spellout.go. It registered, resolved and tagged its call +// correctly — and reached the cost report as a word nobody had written down, +// which [roleOf] prices as a background errand with nobody waiting. The +// spell-out is the one auxiliary a person sits and watches, so the single call +// whose latency matters most was the one counted as if it did not. Nothing +// failed; a number was quietly wrong. `const standingSentinelRole roles.Role = +// "sentinel"` was the second, caught by this law on the day it was written. +// +// BOTH SPELLINGS, because there are two. A declared type +// (`x roles.Role = "w"`) and a conversion (`x = roles.Role("w")`) produce the +// same constant and only the first has a Type for a reader to match on; +// `roleRepair = roles.Role("repair")` was invisible to the first version of +// this law. The second half — every word handed to [roles.Register] — is the +// one that cannot be spelled around at all, because a role nothing registers +// resolves under no tier. +func TestEveryRoleWordIsDeclaredWhereTheVocabularyIs(t *testing.T) { + const home = "internal/roles/roles.go" + declared := map[string]bool{} + for _, word := range roleWords() { + declared[word] = true } - ast.Inspect(file, func(node ast.Node) bool { - spec, isValue := node.(*ast.ValueSpec) - if !isValue || len(spec.Values) != 1 { - return true - } - if kind, isIdent := spec.Type.(*ast.Ident); !isIdent || kind.Name != "Role" { + walkBuildSources(t, func(path string, file *ast.File) { + ast.Inspect(file, func(node ast.Node) bool { + if call, isCall := node.(*ast.CallExpr); isCall { + if methodName(call.Fun) == "Register" && len(call.Args) > 0 { + if word, spelled := roleWordOf(call.Args[0]); spelled && !declared[word] { + t.Errorf("%s registers the role %q, which %s does not declare. A role's "+ + "WORD belongs there beside the others — spelled out, not conjured at "+ + "the call — because everything that reads the vocabulary reads that "+ + "file and a word it cannot see is priced as an errand nobody is "+ + "waiting on.", path, word, home) + } + } + return true + } + spec, isValue := node.(*ast.ValueSpec) + if !isValue || path == home { + return true + } + for index, name := range spec.Names { + spelled := isRoleType(spec.Type) + if !spelled && index < len(spec.Values) { + _, spelled = roleWordOf(spec.Values[index]) + } + if spelled { + t.Errorf("%s declares the role %s. Every role's WORD belongs in %s, beside "+ + "the others: a role declared at its call site resolves and tags correctly "+ + "and is still invisible to everything that reads the vocabulary, which "+ + "prices its calls as errands nobody is waiting on. Declare it there and "+ + "keep the roles.Register call here.", path, name.Name, home) + } + } return true - } - if word, literal := stringLiteral(spec.Values[0]); literal && word != "" { - words = append(words, word) - } - return true + }) }) - return words +} + +// TestTheVocabularyListsEveryRoleTheFileDeclares keeps [roles.Vocabulary] whole. +// +// THE REGISTRY IS NOT THE VOCABULARY, and that difference cost a real setting. +// A role gets into the registry by registering, which is how it gets a TIER; a +// role with no tier never registers, and `imagegen` is one — a painter is chosen +// by a pin or not at all. Reading the registry as "what are the roles" threw a +// person's `imagegen:` pin away on the way in. +// +// So internal/roles answers both questions, and the vocabulary is a hand-written +// list, which is a thing that falls behind. This is why it cannot: the same +// parse that reads the file for [roleWords] is checked against what the package +// says it has. +func TestTheVocabularyListsEveryRoleTheFileDeclares(t *testing.T) { + listed := map[string]bool{} + for _, role := range roles.Vocabulary() { + listed[string(role)] = true + } + for _, word := range roleWords() { + if !listed[word] { + t.Errorf("internal/roles declares the role %q and roles.Vocabulary does not list it. "+ + "Every reader that asks what the roles ARE reads that list, and a word missing "+ + "from it is a pin thrown away on the way in.", word) + } + } + if len(listed) != len(roleWords()) { + t.Errorf("roles.Vocabulary lists %d roles and the file declares %d; a word in the list "+ + "that is not a constant is a word nothing can ever write", + len(listed), len(roleWords())) + } +} + +// isRoleType reports the `roles.Role` type spelled on a declaration. +func isRoleType(expr ast.Expr) bool { + named, isSelector := expr.(*ast.SelectorExpr) + if !isSelector || named.Sel.Name != "Role" { + return false + } + pkg, isIdent := named.X.(*ast.Ident) + return isIdent && pkg.Name == "roles" +} + +// roleWordOf reads a role written as a conversion — `roles.Role("repair")` — +// and answers the word inside it. +func roleWordOf(expr ast.Expr) (string, bool) { + call, isCall := expr.(*ast.CallExpr) + if !isCall || !isRoleType(call.Fun) || len(call.Args) != 1 { + return "", false + } + return stringLiteral(call.Args[0]) } // moduleRoot is this package's place in the tree, which is two directories up. @@ -365,17 +568,27 @@ func walkBuildSources(t *testing.T, visit func(path string, file *ast.File)) { } } -// callName renders a function expression as the source spells it, so that both -// `WithCallTag` and `provider.WithCallTag` are recognisable without resolving -// imports. -func callName(fun ast.Expr) string { +// methodName is the LAST WORD of a call expression — the function or method +// being called, whatever it was reached through. +// +// THE RECEIVER IS NOT PART OF THE QUESTION THIS LAW ASKS. "Does this call go +// through the door" has the same answer for `completeWithModel`, +// `a.completeWithModel`, `p.agent.completeWithModel` and +// `agentFor(x).completeWithModel`, and for `WithCallTag` whether or not the +// package is spelled in front of it. +// +// It used to be a list of the spellings somebody had seen, rendered by a reader +// that flattened ONE selector level — so `p.agent.completeWithModel` and +// `e.agent.completeWithModel` rendered as "" and three of the six rows in that +// list could never match anything. The ladder looked complete and was half +// dead, which is what a ladder of names always eventually is, and the only +// symptom would have been a tag this law quietly stopped knowing about. +func methodName(fun ast.Expr) string { switch shape := fun.(type) { case *ast.Ident: return shape.Name case *ast.SelectorExpr: - if pkg, isIdent := shape.X.(*ast.Ident); isIdent { - return pkg.Name + "." + shape.Sel.Name - } + return shape.Sel.Name } return "" } diff --git a/cmd/aforge-replay/request.go b/cmd/aforge-replay/request.go index 91ea846cd5..416df97798 100644 --- a/cmd/aforge-replay/request.go +++ b/cmd/aforge-replay/request.go @@ -208,11 +208,12 @@ func measuredFelt(end callrows.Row, role lane.Role) float64 { // delete when the role reaches the row (#928, which carries the acceptance). // // THE RULE IS THAT AN ERRAND'S TAG IS ALREADY ITS ROLE. Every side call of a -// turn is tagged with its own role word — internal/session's auxiliary.go writes -// `WithCallTag(errandCtx, string(role))` for exactly this reason, "so three -// records of one call agree about what to call it" — which resolves `judge`, -// `memory`, `design`, `auxiliary`, `recall`, `tool`, `probe` and `standing` with -// no table at all. What is left is the handful of tags that name a CALL SITE +// turn is tagged with its own role word — internal/session's auxiliary.go hands +// `callPurpose(role)` to the one door, which spells it onto the request +// (clientdoor.go), so three records of one call agree about what to call it — +// which resolves `judge`, `memory`, `design`, `auxiliary`, `recall`, `tool`, +// `probe` and `standing` with no table at all. What is left is the handful of +// tags that name a CALL SITE // rather than a role, and each of those is one row of data below with the file // that sets it. // @@ -248,15 +249,46 @@ func roleOf(tag string) lane.Role { // // IT IS COMPLETE, AND A LAW READS THE TREE TO KEEP IT SO // (`TestEveryTagTheBuildWritesResolvesToARoleSomebodyDeclared`). Every tag this -// build can write is reachable from source: the string literals handed to -// `provider.WithCallTag`, the errand names `cmd/aforge`'s errandContext passes, -// and — because internal/session/auxiliary.go writes `string(role)` — every -// [roles.Role] constant there is. A word missing from here reads as a background +// build can write is reachable from source: the [session.callPurpose] values +// handed to internal/session's one door (clientdoor.go, which is the only file +// in that package that spells `provider.WithCallTag`), the errand names +// `cmd/aforge`'s errandContext passes, and — because auxiliary.go hands the door +// `callPurpose(role)` — every [roles.Role] constant there is. A word missing +// from here reads as a background // errand, which is quiet, plausible and moves every number in the table, so the // law fails the build rather than the report going quietly wrong. var callSiteRoles = map[string]lane.Role{ // internal/session/loop.go's laneRole: a conversation's own turn is talk. "turn": lane.RoleTalk, + // internal/session/checkpoint.go's draft rung: the model that has just spent + // the turn, asked on the turn's own transcript for the document a worker will + // finish from. It is made with lane.RoleAuxiliary and billed to the turn, and + // it reached the log with NO TAG AT ALL until #996 — which made the dearest + // side-call on the ceiling road invisible to every reading of this file. + "handoff-draft": lane.RoleAuxiliary, + // internal/session/subharness_env.go: one AI step of a saved program, which + // runs on the program's own model rather than on any role's. Nobody is + // reading its stream and nobody is waiting on its first word. + "subharness": lane.RoleAuxiliary, + // ── two of the three roads that carry their own provider client ───────── + // + // Each builds a client the one door does not make and completes on it + // directly (internal/session/clientdoor.go's withPurpose), and all three + // reached this file with NO TAG AT ALL until #996 — so the errand this build + // runs most often with nobody there was priced as anonymous. The third, + // `consolidate`, already had a row below because internal/roles has that + // word: the tidy-up and the role are the same errand and the same reading. + // + // internal/session/standing_run.go: one standing item's yes-or-no, on every + // check of every item forever. It is spelled `standing-check` and NOT + // `sentinel`, which is already the resident's own quorum errand above and is + // judged: this one sets lane.RoleStanding on its own context, and pricing it + // as a judge would put a wait nobody is having into the table. + "standing-check": lane.RoleStanding, + // internal/session/tools_doc.go: the model's own eyes on a document the + // `read` tool cannot open as text. A TOOL CALL INSIDE A TURN, so somebody IS + // waiting — the one of the three whose seconds are a person's. + "document": lane.RoleTalk, // The same function, in a task. WHICH LEAF ROLE IT IS CANNOT BE READ FROM // THE ROW — laneRole asks `someoneIsWatching()` at the moment of the call // and nothing records the answer — so the unattended reading is taken, @@ -303,21 +335,42 @@ var callSiteRoles = map[string]lane.Role{ "planner": lane.RoleDesign, "designer": lane.RoleDesign, "division": lane.RoleDesign, - "title": lane.RoleAuxiliary, - "compaction": lane.RoleAuxiliary, - "vision": lane.RoleAuxiliary, - "imagegen": lane.RoleAuxiliary, - "worker": lane.RoleAuxiliary, - "speech": lane.RoleAuxiliary, - "video": lane.RoleAuxiliary, - "handoff": lane.RoleAuxiliary, - "taskname": lane.RoleAuxiliary, - "jobname": lane.RoleAuxiliary, - "caption": lane.RoleAuxiliary, - "shaper": lane.RoleAuxiliary, - "intake": lane.RoleAuxiliary, - "careful": lane.RoleAuxiliary, - "distill": lane.RoleAuxiliary, + // internal/session/taxonomy_boundary.go hands this word to the classifier + // that reads a repair round's evidence, and internal/session/repair_role.go + // resolves the hands the round itself runs on. Both are work inside a task + // with nobody's stream open, which is what an errand IS — the table's own + // default, said out loud because this word only became visible here when the + // role stopped being conjured at its call site as roles.Role("repair"). + "repair": lane.RoleAuxiliary, + "title": lane.RoleAuxiliary, + // internal/session/image.go is the only writer of this tag and it sets + // lane.RoleTalk, deliberately and with the reasoning beside it: a look at an + // image STREAMS INTO THE ROOM the person is reading, delta by delta, during + // their own turn. This row said RoleAuxiliary — a call nobody's seconds are + // being spent on — which is the opposite of what that file says, and it moved + // every number in this table that separates a person's wait from a machine's. + "vision": lane.RoleTalk, + // internal/session/spellout.go declares lane.RoleAuxiliary on its own + // context, so this row is that file's own reading and not a second one. + // + // IT IS WORTH A LOOK AND IT IS NOT THIS FILE'S CALL. A person is SITTING AND + // WATCHING a spell-out — it writes three lines they read before deciding + // whether to keep them — and an auxiliary is by definition a call nobody's + // seconds are being spent on. Either the lane role or the product is wrong + // there; the table's job is to agree with the build. + "spellout": lane.RoleAuxiliary, + "imagegen": lane.RoleAuxiliary, + "worker": lane.RoleAuxiliary, + "speech": lane.RoleAuxiliary, + "video": lane.RoleAuxiliary, + "handoff": lane.RoleAuxiliary, + "taskname": lane.RoleAuxiliary, + "jobname": lane.RoleAuxiliary, + "caption": lane.RoleAuxiliary, + "shaper": lane.RoleAuxiliary, + "intake": lane.RoleAuxiliary, + "careful": lane.RoleAuxiliary, + "distill": lane.RoleAuxiliary, } // requestOf turns a replayed request into the one a chooser is asked. diff --git a/cmd/aforge/chatv3_harness.go b/cmd/aforge/chatv3_harness.go index d3bba193cf..6c2afce94d 100644 --- a/cmd/aforge/chatv3_harness.go +++ b/cmd/aforge/chatv3_harness.go @@ -56,9 +56,9 @@ import ( // is handed to the bridge as the default every agent.loop node rides. // harnessTimeout bounds one node's completion. It matches internal/session's -// own provider timeout: a harness node is a non-streamed call like the -// compaction summary is, and the backstop is against a wedged endpoint rather -// than a limit on how long a run may take. +// own provider timeout: a harness node is a non-streamed call like the title +// call is, and the backstop is against a wedged endpoint rather than a limit on +// how long a run may take. const harnessTimeout = 10 * time.Minute // v3HarnessEntries is the registry as DETECTION reads it: one entry per saved diff --git a/cmd/aforge/chatv3_project_test.go b/cmd/aforge/chatv3_project_test.go index 5f596ac626..cd1d0bb48e 100644 --- a/cmd/aforge/chatv3_project_test.go +++ b/cmd/aforge/chatv3_project_test.go @@ -92,9 +92,9 @@ func TestTheProjectFileBeatsTheProfileAtTheSessionConfig(t *testing.T) { if err != nil || model != "project/title" { t.Fatalf("the title role resolved to %q (%v), want the project's pin", model, err) } - model, err = roles.Resolve(cfg.RolesSource, roles.RoleCompaction, "session/model") + model, err = roles.Resolve(cfg.RolesSource, roles.RoleAuditor, "session/model") if err != nil || model != "project/capable" { - t.Fatalf("compaction resolved to %q (%v), want the project's high tier", model, err) + t.Fatalf("the auditor resolved to %q (%v), want the project's high tier", model, err) } if got, ok := roles.TierModel(cfg.RolesSource, roles.TierLow); !ok || got != "project/cheap" { t.Fatalf("the low tier is %q (ok=%v)", got, ok) diff --git a/cmd/aforge/chatv3_roles_test.go b/cmd/aforge/chatv3_roles_test.go index 9c5a5bf622..eb3bc087ce 100644 --- a/cmd/aforge/chatv3_roles_test.go +++ b/cmd/aforge/chatv3_roles_test.go @@ -68,7 +68,7 @@ func TestTheDoorsRoleLadderCarriesEveryTier(t *testing.T) { }{ {roles.RoleTitle, config.DefaultLowModel}, {roles.RoleWorker, config.DefaultWorkerModel}, - {roles.RoleCompaction, config.DefaultHighModel}, + {roles.RoleAuditor, config.DefaultHighModel}, // The mastermind's value carries a level, and Resolve hands back the id // alone — a colon in a model field is a request for a model nobody serves. {roles.RolePlanner, "z-ai/glm-5.3"}, diff --git a/cmd/aforge/chatv3_wiring_test.go b/cmd/aforge/chatv3_wiring_test.go index 80848a8ae1..1cb3c3d118 100644 --- a/cmd/aforge/chatv3_wiring_test.go +++ b/cmd/aforge/chatv3_wiring_test.go @@ -83,9 +83,9 @@ func TestTheSettingsRowsReachTheSessionConfig(t *testing.T) { if err != nil || model != "pinned/model" { t.Fatalf("the title role resolved to %q (%v), want the pin", model, err) } - model, err = roles.Resolve(cfg.RolesSource, roles.RoleCompaction, "session/model") + model, err = roles.Resolve(cfg.RolesSource, roles.RoleAuditor, "session/model") if err != nil || model != "capable/model" { - t.Fatalf("compaction resolved to %q (%v), want the high tier", model, err) + t.Fatalf("the auditor resolved to %q (%v), want the high tier", model, err) } if got, ok := roles.TierModel(cfg.RolesSource, roles.TierLow); !ok || got != "cheap/model" { t.Fatalf("the low tier is %q (ok=%v)", got, ok) diff --git a/docs/changes/unreleased/1011-turn-seals-without-the-judge.md b/docs/changes/unreleased/1011-turn-seals-without-the-judge.md new file mode 100644 index 0000000000..7591bdd10d --- /dev/null +++ b/docs/changes/unreleased/1011-turn-seals-without-the-judge.md @@ -0,0 +1,27 @@ +--- +kind: changed +title: the turn seals when the answer is done, and the route judge is spent when it lands +pr: 1011 +surface: [chat, engine] +invalidates: + - "The end of a turn waited for BOTH end-of-turn readers. It waits only for the one that can re-open the turn; the work-or-words judge no longer holds the seal, and a yes that lands after the turn ended starts its task then." + - "The status line said `whether that should be work` at the end of a turn. That line is deleted, because the wait it named is gone." + - "`judgeRace.takeAtTheEnd` existed and blocked. It is `judgeRace.spendWhenItLands`, which hands the ruling over when it arrives and never waits." + - "Every reading in `internal/session` ran on the turn's own context. The post-turn judge runs on the session's lifetime (`afterTurn`, sidecar.go), because its effect — starting a task — lands outside the turn anyway." + - "`journalPace` carried `sendMs`, `firstWordMs` and `stepGapMs`. It carries `sealMs` — the model's last word to the seal — and `reaskMs`, how long the request a recall threw away was on the wire." + - "`loop_speed_test.go` asserted two figures. It asserts three: the seal must not be held by any reading whose answer could be spent afterwards." +--- + +The post-turn judge stood between the model's last word and the seal, blocking on +a cheap screen plus, on a yes, a mastermind confirm — with the answer already +fully written on the person's screen. Parked behind it were the seal, the usage +row, the next Submit and the follow-up drain. + +The turn's job was never to know whether the ruling had arrived; it was to say +when one may be spent. It still says exactly that and walks away. + +`reaskMs` is an instrument and not a fix. The pace ledger holds three rows and +one of them has a recall beside it, so the share of turns that pay for a +cut-and-re-ask cannot be derived yet. What the one row does show is that the +≤50 ms law can be satisfied by a request that is then thrown away: `sendMs` read +1 on a turn where the person waited 7,389 ms for their first word. diff --git a/docs/changes/unreleased/996-auxiliary-inventory.md b/docs/changes/unreleased/996-auxiliary-inventory.md new file mode 100644 index 0000000000..cda2a82a63 --- /dev/null +++ b/docs/changes/unreleased/996-auxiliary-inventory.md @@ -0,0 +1,48 @@ +--- +kind: internal +title: every request says what it is for, a dead role goes, and the prefix gate weighs the real belt +pr: 996 +surface: [chat, engine] +invalidates: + - "`auxiliary.go` said `callRole` was \"the one door all auxiliary provider calls pass through\". It never was — ten calls went round it. It is the one door every ERRAND passes through; the one door every REQUEST passes through is `clientdoor.go`'s `completeWithModel`." + - "The call tag was a context value set with `provider.WithCallTag` wherever a caller remembered to. It is an argument now — `callPurpose`, the second parameter of `completeWithModel` — and a call with no purpose does not compile. Nothing in `internal/session` but the door may call `WithCallTag`." + - "Nine auxiliary calls reached the wire with no tag: the guardian, vision, the shaper, the spell-out, the subharness intake, the run planner, the harness designer, the ceiling's handoff draft and a saved program's AI step. Every one of them now names itself in the model-call log." + - "`roles.RoleCompaction` existed, sat on `TierHigh`, had a description and a settings row. It is deleted: no compaction has made a model call since the summariser went, and `Agent.compact` takes `_ context.Context`." + - "`TierHigh` had a built-in tenant in `DefaultAssignment`. It has none — every careful call (`auditor`, `shaper`, `vision`, `careful`, the repair) registers itself from the package that makes it." + - "`TierHigh`'s five-minute patience was justified by \"a compaction summary over a full window\". That call does not exist; the figure is derived from the auditor now." + - "The settings roles list offered a `compaction` row and the crew's careful line read \"audits, compaction, vision\". Neither does; the line reads \"audits, briefs, vision\"." + - "`fixedPrefixBudget` was 48,000 and weighed `v3ShapedAgent` — 18 tools, no memory store, no accounts hub, no standing items, no saved programs. It weighs the fully-wired conversation (`shippedShapeAgent`, 23 tools) and is 53,141, which is that shape's measurement on the widest machine and no headroom." + - "The shipped prefix was believed to be inside 48,000. It is 53,141 — 5,141 over — and has been for as long as the shape existed. 48,000 is a target the test prints the shortfall against, not a cap anything passes today." + - "`v3ShapedAgent` claimed to be the conversation the interactive door configures and built its own thin config — 18 tools, no memory store, no accounts hub, no standing items, no saved programs. Four gates weighed the shipping door through it, including both prefix budgets. It is `shippedShapeAgent` now: there is ONE shipped shape." + - "`leanPrefixBudget` and `fixedPrefixBudget` printed their overage with `t.Logf`, which nothing in the Makefile passes `-v` to — the line saying the prefix was thousands of bytes over its target was printed where nobody would see it. The cap is the target plus a dated waiver in one place (`prefixWaivers`), and a waiver only shrinks." + - "A role's word could be declared at its call site, which internal/roles' own header invited. It cannot: everything that reads the vocabulary reads one file, so `spellout`, `sentinel` and `repair` were each priced as errands nobody waits on. All three are declared in `internal/roles` and a law catches the next one in either spelling, including `roles.Role(\"x\")`." + - "The replay table priced `vision` as an auxiliary; `image.go`, its only writer, sets `lane.RoleTalk` — a look at an image streams into the room during the person's own turn." + - "The manual taught a summariser rung for `/compact`, its `session: summarizer returned nothing` refusal, and a focus appended to the summariser's instructions. There is no summariser: a pass stubs tool results and folds the oldest assistant work to one marker line, asks no model, and never folds your own words." + - "A profile whose `models.roles` row pinned `compaction:` was left unreadable by the role deletion: the panel drew no row for it and any other pin re-serialised the whole string through a writer that refused `\"compaction\" is not a role`, so every pin on that machine was unchangeable until config.json was hand-edited. The reading is symmetric now: a pin for any word that is not a role is dropped on read, never written back, and refused only when it is the pin being ADDED. The row draws what is acted on rather than the raw stored text, and the panel says `compaction is no longer a role — that pin is ignored`." + - "`leanPrefixBudget` was 31,500 and weighed a lean conversation with no memory store, no accounts hub, no standing items and no saved programs — 16 tools. It weighs the shipped shape on a small window and is 44,916. The arm that protects the person with the LEAST room was out by 13,416 bytes, which is more than the budget it was enforcing." + - "`spellout` priced as an errand nobody was waiting on in every cost report, because its Role constant was declared in internal/session and `roleWords` parses internal/roles. The word is `roles.RoleSpellOut` now, and a law fails the next role declared anywhere else." + - "The memory tidy-up, the standing sentinel and the document reader reached `calls.jsonl` with no tag at all. They build their own provider client and never pass the door, so they now say the door's own word through `withPurpose` — `consolidate`, `standing-check`, `document` — and the law fails a file that builds a client and names nothing." + - "The fixed prefix was believed to be the same size everywhere. It is not: `grep` says a longer sentence about itself where ripgrep is absent (134 bytes) and `load_capability` lists the tool groups this build has (27). The gate weighs the widest machine — `bare.WidestGrepDescription` is the one place that knows which spelling that is — so the number does not depend on who runs it." +--- + +Three things this build reported about itself incorrectly. None of them changes +what any model is asked or what it answers. + +The tag is the load-bearing one. A tag that can be forgotten is a tag that will +be, and the log that was supposed to answer "what was this build spending that +model on all night" could not: 2,309 of the 2,839 untagged finishes in the ten +days to 2026-09-10 were this package's. Making the purpose an argument is the +whole fix — the type refuses an anonymous call, and `nohiddenwork_test.go` +refuses one that satisfies the type with a blank. + +The prefix gate is the same disease in a test: a gate pointed at something +nobody runs passes without having tested anything, which is #576's shape. The +5 KB it was hiding is filed rather than paid — `stand`'s schema is 9,607 bytes, +more than a quarter of the tool block, and trimming it is a change to a tool's +contract with the model rather than to a byte count. + +Two seams are reported rather than worked around: the standing sentinel builds +its own `provider.NewClient` and has no `*Agent` to reach either door through, +and four of the nine anonymous calls (guardian, shaper, spell-out, intake) are +genuine errands that could go through `callRole` — which would ask a second +model on failure, and so is a behaviour change rather than a fix. diff --git a/internal/config/selfservice_test.go b/internal/config/selfservice_test.go index 4c3184ab63..7aebbad227 100644 --- a/internal/config/selfservice_test.go +++ b/internal/config/selfservice_test.go @@ -133,6 +133,141 @@ func TestEveryRowSaysWhatItAccepts(t *testing.T) { } } +// A PIN FOR A WORD THAT IS NOT A ROLE IS DROPPED FROM A PROFILE THAT STILL +// HOLDS IT, and the rest of that person's row keeps working. +// +// THE MEASURED FAILURE THIS IS WRITTEN AGAINST. `compaction` was a role for +// months: it had a tier, a description, and a row in the settings panel, and +// nothing ever called it. Deleting it took the row off the panel and left the +// word in every config.json that had pinned it. The panel re-serialises the +// WHOLE `models.roles` string on any change, so the next time that person moved +// ANY OTHER pin the write came back `"compaction" is not a role` — and every +// role pin on that machine was unchangeable until somebody opened config.json by +// hand. A deletion with no reading for what it deleted is a deletion that breaks +// the people who used the thing. +// +// THE READING DOES NOT CARE WHY THE WORD IS DEAD. A name this build retired and +// a name somebody mistyped are the same thing on the way in — a pin nothing will +// consult — so there is no ledger of retired names to keep up to date, and the +// day nobody updates it is not a day this comes back. +func TestAPinForARetiredRoleIsDroppedAndTheRestOfTheRowStillMoves(t *testing.T) { + profile := t.TempDir() + // The string a profile written before the role was deleted actually holds. + const before = "compaction:openai/gpt-5-mini, title:openai/gpt-5-mini, planner:deepseek/deepseek-v4-pro" + + pins, err := ParseModelRoles(before) + if err != nil { + t.Fatalf("a profile that pins a retired role could not be read at all: %v", err) + } + if _, still := pins["compaction"]; still { + t.Error("the retired role came back as a live pin") + } + for _, want := range []string{"title", "planner"} { + if pins[want] == "" { + t.Errorf("the pin for %q was lost along with the retired one: %v", want, pins) + } + } + + // AND THE NEXT CHANGE TO ANY OTHER PIN GOES THROUGH — the whole failure. + // + // The row is seeded the way a profile written before the deletion holds it: + // straight into the file, because the writer is exactly what would refuse it + // today and the person whose machine this is never typed it today either. + if err := writeText(profile, KeyModelRoles, before); err != nil { + t.Fatalf("seeding the profile: %v", err) + } + if err := writeModelRoles(profile, before); err != nil { + t.Fatalf("a row carrying a retired role was refused: %v", err) + } + // AND THE DEAD WORD IS NOT WRITTEN BACK, so the next read has nothing to + // forgive and the row stops carrying a sentence about nothing. + stored, ok := persistedString(profile, KeyModelRoles) + if !ok { + t.Fatal("the row was not stored at all") + } + if strings.Contains(stored, "compaction") { + t.Errorf("the retired role was written back: %q", stored) + } + for _, want := range []string{"title:openai/gpt-5-mini", "planner:deepseek/deepseek-v4-pro"} { + if !strings.Contains(stored, want) { + t.Errorf("the row that was kept lost %q: %q", want, stored) + } + } + // AND THE PERSON IS TOLD, in a sentence and not a log line. + if note := RetiredPinNote(before); !strings.Contains(note, "compaction") { + t.Errorf("the surface is handed %q, want a line naming the pin being ignored", note) + } +} + +// AND A WORD NOBODY EVER PINNED IS STILL REFUSED WHEN IT IS BEING ADDED, which +// is the other half of the same bargain: the refusal earns its keep at the one +// moment a person can act on it, and nowhere else. +func TestANewPinForAWordThatIsNotARoleIsStillRefused(t *testing.T) { + profile := t.TempDir() + if err := writeModelRoles(profile, "title:openai/gpt-5-mini"); err != nil { + t.Fatalf("a real role name was refused: %v", err) + } + if err := writeModelRoles(profile, "title:openai/gpt-5-mini, harness_designer:some/model"); err == nil { + t.Fatal("a pin against a word that is not a role was accepted as it was added") + } +} + +// AND THE SAME WORD IS REFUSED WHEN IT IS TYPED AND FORGIVEN WHEN IT IS FOUND, +// which is the whole asymmetry in one test. `compaction` sitting in a file since +// before the deletion is a sentence about a world that used to exist and the +// person cannot act on being told about it; `compaction` typed into the row just +// now is a person asking for something, who is owed an answer while they are +// still looking at the box. +// +// THE BUG THIS PINS: the check asked [parseModelRoles] whether the word was +// already stored, and that reader drops every retired word by design — so the +// stored half read as the typed half and a machine with an old pin could not +// save any change to any other pin. Nothing failed loudly; the row just would +// not move. +func TestATypedRetiredRoleIsRefusedEvenThoughAStoredOneIsForgiven(t *testing.T) { + profile := t.TempDir() + const stored = "compaction:openai/gpt-5-mini, title:openai/gpt-5-mini" + if err := writeText(profile, KeyModelRoles, stored); err != nil { + t.Fatalf("seeding the profile: %v", err) + } + + // Found in the file: forgiven, and the rest of the row moves. + if err := writeModelRoles(profile, stored); err != nil { + t.Fatalf("the stored retired pin was refused: %v", err) + } + + // Typed just now into a row that no longer has it: refused, by name. + err := writeModelRoles(profile, "title:openai/gpt-5-mini, compaction:some/model") + if err == nil { + t.Fatal("a retired role typed into the row was accepted silently") + } + if !strings.Contains(err.Error(), "compaction") { + t.Errorf("the refusal does not name the word the person typed: %v", err) + } +} + +// AND A ROLE WITH NO TIER IS STILL A ROLE. `imagegen` never calls +// roles.Register — a painter is chosen by a pin or not at all — so it is in the +// vocabulary and not in the registry, and a reader that asked the registry threw +// the pin away on the way in. This is the case that keeps generate_image on a +// belt somebody configured. +func TestAPinForARoleWithNoTierSurvivesTheRead(t *testing.T) { + pins, err := ParseModelRoles("imagegen:paint/model, title:openai/gpt-5-mini") + if err != nil { + t.Fatalf("the row was unreadable: %v", err) + } + if pins["imagegen"] != "paint/model" { + t.Fatalf("the painter pin read back as %q; a role with no tier is still a role", pins["imagegen"]) + } + profile := t.TempDir() + if err := writeModelRoles(profile, "imagegen:paint/model"); err != nil { + t.Fatalf("pinning a painter was refused: %v", err) + } + if stored, _ := persistedString(profile, KeyModelRoles); !strings.Contains(stored, "imagegen") { + t.Fatalf("the painter pin was not written back: %q", stored) + } +} + // A ROLE NOBODY RECOGNISES FAILS SILENTLY FOREVER, which is why this row // validates the name and the model slug beside it does not. A pin written // against a misspelled role is stored, reads back exactly as typed, shows in diff --git a/internal/config/settings.go b/internal/config/settings.go index 1b0991a48a..0ec2727d59 100644 --- a/internal/config/settings.go +++ b/internal/config/settings.go @@ -191,8 +191,8 @@ const ( // chosen for THINKING rather than for a price. Two roles ride it — the // planner that amends an adaptive run's plan after every node, and the // designer that writes a harness page everybody afterwards runs — and both - // were on the careful-work tier beside the compaction summary, which made one - // figure answer two unrelated bills: the careful calls are many and short, + // were on the careful-work tier beside the check on finished work, which made + // one figure answer two unrelated bills: the careful calls are many and short, // these are few and decide what all the other calls do. // // It is the one row whose value may carry a LEVEL as well as a model @@ -2179,8 +2179,8 @@ func (s *Settings) build() []Setting { }, // THE CREW, AND THEN THE FOUR CLASSES IN IT. The tiers are what a person // actually configures for the calls aforge makes on its own — the name it - // gives a session, the summary a compaction writes, the plan an adaptive - // run steers by (internal/roles). Four rows, not one per feature: a new + // gives a session, the check on work a task says is finished, the plan an + // adaptive run steers by (internal/roles). Four rows, not one per feature: a new // call joins a class and needs no row of its own. // // The crew row comes FIRST because it is the only one most people will @@ -2231,7 +2231,7 @@ func (s *Settings) build() []Setting { Key: KeyTierHighModel, Category: CategoryModels, Kind: SettingText, Label: "careful work", EmptyLabel: "follows the conversation", Hint: "the capable model for the things that must not be wrong — the check on " + - "finished task work, the summary a compaction keeps, reading an image.", + "finished task work, the brief a task is shaped into, reading an image.", read: func() string { return TierModelAt(dir, ModelTierHigh) }, write: func(raw string) error { return writeTierModel(dir, ModelTierHigh, raw) }, }, @@ -2252,7 +2252,7 @@ func (s *Settings) build() []Setting { Label: "pinned roles", EmptyLabel: "none", Hint: "exceptions to the five rows above, one per role: `title:openai/gpt-5-mini`. " + "A role not named here follows its class.", - read: func() string { return ModelRolesAt(dir) }, + read: func() string { return LivePinsAt(dir) }, write: func(raw string) error { return writeModelRoles(dir, raw) }, }, Setting{ @@ -3696,11 +3696,95 @@ func ModelRolesAt(profileDir string) string { return "" } +// LivePinsAt is the pinned-roles row AS IT IS ACTED ON: the person's own text +// with the pins this build will never consult taken out. +// +// THE ROW DRAWS WHAT IS TRUE. [ModelRolesAt] is the raw stored string and stays +// that, because the writer needs the text to rewrite; but a row that DISPLAYED +// it would draw `compaction: some/model` as a live pin while the role list under +// it had no such line, and the person would be reading a setting that does +// nothing. What they are told instead is [RetiredPinNote], next to the row. +func LivePinsAt(profileDir string) string { + pins, dropped, err := parseModelRoles(ModelRolesAt(profileDir)) + if err != nil || len(dropped) == 0 { + return ModelRolesAt(profileDir) + } + names := make([]string, 0, len(pins)) + for name := range pins { + names = append(names, name) + } + sort.Strings(names) + kept := make([]string, 0, len(names)) + for _, name := range names { + kept = append(kept, name+":"+pins[name]) + } + return strings.Join(kept, ", ") +} + // ParseModelRoles reads `title:openai/gpt-5-mini` into role → model. The value // is split at the FIRST colon only, because a model slug can carry one of its // own (`…/model:free`). +// +// A PIN FOR A WORD THAT IS NOT A ROLE IS DROPPED, NOT REFUSED — see +// [parseModelRoles], which is this and the names it dropped. func ParseModelRoles(raw string) (map[string]string, error) { - return parsePairs(raw, "role") + pins, _, err := parseModelRoles(raw) + return pins, err +} + +// parseModelRoles is [ParseModelRoles] and also SAYS WHAT IT DROPPED. +// +// A PIN FOR A WORD THAT IS NOT A ROLE IS DROPPED, NOT REFUSED, and the read does +// not care WHY it is not a role. A name this build retired and a name somebody +// mistyped are the same thing here by design: both are a pin no call will ever +// consult, and the row around them is a person's own text that outlives any +// particular build. Refusing the whole row over one dead word is what made every +// OTHER pin on that machine unchangeable when `compaction` was deleted, because +// the panel re-serialises the whole string on any change. +// +// THE DROP IS RETURNED AND NOT LOGGED. This is a pure parser on the settings +// panel's draw path, and a log line here would print on stderr in every headless +// and `--json` door, over the top of a machine-readable answer. Who should hear +// about a dead pin depends on who is asking: the panel says it to the person in +// front of it ([RetiredPinNote]), and the writer just stops writing it back. +func parseModelRoles(raw string) (pins map[string]string, dropped []string, err error) { + pins, err = parsePairs(raw, "role") + if err != nil { + return nil, nil, err + } + for name := range pins { + if !knownRole(name) { + dropped = append(dropped, name) + delete(pins, name) + } + } + sort.Strings(dropped) + return pins, dropped, nil +} + +// knownRole reports a name this build has a role for. +// +// IT ASKS THE VOCABULARY AND NOT THE REGISTRY, which are two different +// questions. The registry is the TIER TABLE: a role gets into it by registering, +// and a role with no tier never does. `imagegen` is exactly that — a painter is +// chosen by a pin or not at all, since a chat model in tiers.high is not a +// statement about painting — so reading the registry here threw away a person's +// `imagegen:` pin, which is a real setting doing real work. +func knownRole(name string) bool { return roles.Known(name) } + +// RetiredPinNote is what a surface with a person in front of it says about pins +// this build will not act on, and "" when there are none. It is a sentence and +// not a log line, because the only reader who wants it is the one looking at the +// row it is about. +func RetiredPinNote(raw string) string { + _, dropped, err := parseModelRoles(raw) + if err != nil || len(dropped) == 0 { + return "" + } + if len(dropped) == 1 { + return fmt.Sprintf("%s is no longer a role — that pin is ignored", dropped[0]) + } + return fmt.Sprintf("%s are no longer roles — those pins are ignored", strings.Join(dropped, " and ")) } // ModelFallbacksAt resolves the fallback chain as the person wrote it. @@ -3971,13 +4055,7 @@ func knownToolApprovalMode(mode string) bool { // obeyed. func parsePairs(raw, subject string) (map[string]string, error) { pairs := map[string]string{} - for _, item := range strings.FieldsFunc(raw, func(r rune) bool { - return r == ',' || r == ';' || r == '\n' - }) { - item = strings.TrimSpace(item) - if item == "" { - continue - } + for _, item := range pairItems(raw) { name, value, found := strings.Cut(item, ":") name = strings.TrimSpace(name) value = strings.TrimSpace(value) @@ -3992,6 +4070,26 @@ func parsePairs(raw, subject string) (map[string]string, error) { return pairs, nil } +// pairItems splits a pair row into its entries, blanks dropped and each one +// trimmed. It is where the separators a person may use are decided — comma, +// semicolon, newline — and it is one function because [parsePairs] and the +// rewriter that takes a dead role back out of a stored row +// ([withoutRetiredRoles]) have to cut the string the same way. A rewriter that +// split it differently would drop an entry the parser kept, or keep one it had +// already refused. +func pairItems(raw string) []string { + fields := strings.FieldsFunc(raw, func(r rune) bool { + return r == ',' || r == ';' || r == '\n' + }) + items := make([]string, 0, len(fields)) + for _, field := range fields { + if item := strings.TrimSpace(field); item != "" { + items = append(items, item) + } + } + return items +} + // writeToolApprovals validates the pairs, then keeps the person's own text. // Storing the text rather than the parsed map is what lets the row read back // exactly as it was typed; the map is derived on every read anyway. @@ -4019,28 +4117,71 @@ func writeToolApprovals(profileDir, raw string) error { // writing the row is guessing the name — and it guessed `harness_designer` the // first time it was asked. func writeModelRoles(profileDir, raw string) error { - pins, err := ParseModelRoles(raw) + pins, dropped, err := parseModelRoles(raw) if err != nil { return err } - known := map[string]bool{} - for _, role := range roles.Registered() { - known[string(role)] = true + // ONLY THE PIN BEING ADDED IS REFUSED. A name already in the stored row has + // been there for a while and may have been a role when it was typed — the + // person is not adding it now and cannot act on being told about it now, and + // refusing it would take the whole row with it. What a person CAN act on is + // the word they just wrote, which is where the refusal has always earned its + // keep: the settings pair guessed `harness_designer` the first time it was + // asked. + // + // AND "ALREADY STORED" IS A QUESTION ABOUT THE PERSON'S TEXT, NOT ABOUT THE + // PARSE. Asking [parseModelRoles] was the wrong reader to ask: it drops every + // retired word by design, so the stored row it reports back can never contain + // the one word this check is looking for, and a name that had sat in the file + // for months read as a name being typed for the first time. + before := storedRoleWords(profileDir) + for _, name := range dropped { + if !before[roles.RoleKey(name)] { + return fmt.Errorf("%q is not a role. The roles are: %s", name, strings.Join(roleNames(), ", ")) + } } - for name := range pins { - if known[strings.ToLower(strings.TrimSpace(name))] { - continue + // AND A WORD THIS BUILD DOES NOT ANSWER TO IS NEVER WRITTEN BACK. The parse + // took it out of the map; this takes it out of the TEXT, which is what is + // actually stored. Without it a person who changes any pin keeps re-saving a + // word nothing will ever read, forever. + // + // THE ROW IS REBUILT ONLY WHEN IT HAS TO BE. It is stored verbatim so it reads + // back the way it was typed, and normalising a person's separators on every + // save would edit their sentence for no reason. + if len(dropped) == 0 { + return writeText(profileDir, KeyModelRoles, raw) + } + kept := make([]string, 0, len(pins)) + for _, item := range pairItems(raw) { + if name, _, _ := strings.Cut(item, ":"); knownRole(name) { + kept = append(kept, item) + } + } + return writeText(profileDir, KeyModelRoles, strings.Join(kept, ", ")) +} + +// storedRoleWords is every role word the stored row NAMES, whether or not this +// build still answers to it — which is the set [writeModelRoles] compares a new +// row against to tell a word being added from a word that was already there. +// +// It reads the text rather than the pins on purpose. Every reader above this +// line drops what it does not recognise, and the word this set exists to hold is +// exactly the word they drop. +func storedRoleWords(profileDir string) map[string]bool { + words := map[string]bool{} + for _, item := range pairItems(ModelRolesAt(profileDir)) { + if name, _, ok := strings.Cut(item, ":"); ok { + words[roles.RoleKey(name)] = true } - return fmt.Errorf("%q is not a role. The roles are: %s", name, strings.Join(roleNames(), ", ")) } - return writeText(profileDir, KeyModelRoles, raw) + return words } // roleNames is the registered roles as a sorted list of plain words, for the // refusal above to name them all rather than make somebody go looking. func roleNames() []string { - names := make([]string, 0, len(roles.Registered())) - for _, role := range roles.Registered() { + names := make([]string, 0, len(roles.Vocabulary())) + for _, role := range roles.Vocabulary() { names = append(names, string(role)) } sort.Strings(names) diff --git a/internal/e2e/families_e2e_test.go b/internal/e2e/families_e2e_test.go index e7ed50cb56..640eb5cab9 100644 --- a/internal/e2e/families_e2e_test.go +++ b/internal/e2e/families_e2e_test.go @@ -121,7 +121,7 @@ func pinEveryTextModel(t *testing.T) { // textRoles is every role in internal/roles that answers with words. The media // four are absent for [pinEveryTextModel]'s reason. var textRoles = []roles.Role{ - roles.RoleTitle, roles.RoleCompaction, roles.RoleConsolidate, roles.RoleGuardian, + roles.RoleTitle, roles.RoleConsolidate, roles.RoleGuardian, roles.RolePlanner, roles.RoleDesigner, roles.RoleWorker, roles.RoleAuditor, roles.RoleReflex, roles.RoleRouter, roles.RoleRouterConfirm, roles.RoleMarkReader, roles.RoleHandoff, roles.RoleTaskName, roles.RoleShaper, roles.RoleIntake, diff --git a/internal/exec/bare/grepfindls.go b/internal/exec/bare/grepfindls.go index ade4ab1bb8..2ce8a9543a 100644 --- a/internal/exec/bare/grepfindls.go +++ b/internal/exec/bare/grepfindls.go @@ -77,6 +77,30 @@ func grepToolDescription(caps Caps) string { return grepFallbackDescription(caps) } +// WidestGrepDescription is the longer of the two sentences `grep` can carry — +// the one a machine with NO ripgrep is handed. +// +// A DESCRIPTION THAT DEPENDS ON THE MACHINE IS STILL A BYTE ON EVERY REQUEST, +// and the gate that bounds the fixed prefix (internal/session's +// prefixbudget_test.go) runs on machines of both kinds: the same commit weighed +// 53,025 bytes on a laptop with ripgrep and 53,132 on a runner without it, so +// the gate passed where it was written and failed where it was proved. The gate +// weighs what the WIDEST machine pays, and this is the one place that knows +// which sentence that is. +func WidestGrepDescription(caps Caps) string { + caps = caps.resolve() + // THE LONGER OF THE TWO, MEASURED, and not the one that happens to be longer + // today. The fallback sentence is the longer one now; a later edit that grew + // the ripgrep sentence past it would have made a function called "widest" + // quietly return the narrower string, and the gate downstream would have gone + // back to depending on who ran it with nothing to say so. + with, without := grepDescription(caps), grepFallbackDescription(caps) + if len(with) > len(without) { + return with + } + return without +} + // grepMatch is one hit, whichever engine found it. type grepMatch struct { filePath string diff --git a/internal/manual/chat/commands.md b/internal/manual/chat/commands.md index 7dbb64739c..fda8c754c1 100644 --- a/internal/manual/chat/commands.md +++ b/internal/manual/chat/commands.md @@ -384,6 +384,15 @@ compact failed: `/compact` has no argument form and no alias. +**A compaction costs nothing and asks no model.** It is two mechanical passes over +the messages this session already has: tool results the model has already used +become pointers to their own bytes, and if that is not enough the oldest assistant +work is replaced by one marker line naming how much went and where it can be read. +Your own words are never folded. There is **no summariser** and there is **no +`compaction` role in settings** — there was one, and it was a priced row wired to +nothing. What the model is handed instead of a summary is the state card, which is +maintained a little at a time by the reader that runs after each turn. + ## /rewind — go back to an earlier point in the conversation `/rewind` (or `/undo`, `/back`) opens the **rewind timeline**: a fullscreen list of the @@ -1698,12 +1707,24 @@ order: 7. **reflex** — `near-free · reads every turn — memory, titles, safety` 8. **small work** — `cheap · the small calls — names, digests, the safety gate` 9. **worker** — `does the work · every task, its parts, every run node — most of the bill` -10. **careful work** — `careful · checks what must not be wrong — audits, compaction, vision` +10. **careful work** — `careful · checks what must not be wrong — audits, briefs, vision` 11. **mastermind** — `thinks · plans runs and designs harnesses — add :low, :medium or :high` 12. **pinned roles**, and hanging off it the **roles** list — one row per auxiliary call aforge makes for itself, grouped under its class. Those rows come from the running binary rather than the settings registry. +**A pin for a role this build no longer has is ignored, and the row stops showing it.** Roles +come and go with the calls that use them — `compaction` was one, and a compaction has not asked +a model since long before it was deleted. A pin left behind for a word like that is dropped +when the row is read, never written back, and the foot line says `compaction is no longer a +role — that pin is ignored` the next time you change any pin. **Every other pin on the row +keeps working**, which is the whole point: the row is one string holding all of them, and +refusing the lot over one dead word would leave you unable to change any of them without +editing `config.json` by hand. + +Typing a word that is not a role is still refused outright, with the real names listed — that +refusal is for the pin you are adding now, which is the one you can do something about. + The four machine rows lead because the endpoint serving your model is part of the same decision as the model, and they used to sit at the foot of the tab, forty rows below it. @@ -1760,7 +1781,7 @@ Connections tab — the foot drops `tab next place`, because the layer has taken ## The roles rows in settings — pinning a role, and del to unpin The **roles** list sits on the **Providers** tab, directly under "pinned roles". Each row is -one call aforge makes outside a turn — `title`, `compaction`, `guardian`, `auditor`, +one call aforge makes outside a turn — `title`, `guardian`, `auditor`, `planner`, `designer`, `worker`, `router`, `vision`, `reflex`, and `spellout`, which is the one of them you ask for yourself with `ctrl+r` (see the keys page) — drawn as ` `, with `pinned` after it when that role has a model of its own. diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index f96a1dda10..fa78ab5fd0 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -312,7 +312,7 @@ class, then the conversation model when that worker class is blank. ## The crew — which models aforge uses on my behalf, and /crew aforge makes calls you did not type: naming a session, naming a piece of work on the roster, -the summary a compaction keeps, the safety gate, the check on finished task work, the second +the brief a task opens on, the safety gate, the check on finished task work, the second look before a task starts itself, the reading of a task's parts before they are handed out, the planner of an adaptive run and the nodes under it, the designer of a saved harness page, looking at an image. Each of those is a @@ -323,7 +323,7 @@ looking at an image. Each of those is a - **small work** — cheap · the small calls — names, digests, the safety gate. - **worker** — does the work · every task you hand off, the parts it divides into, every node of an adaptive run. Most of what a task costs is spent here. -- **careful work** — careful · checks what must not be wrong — audits, compaction, vision. +- **careful work** — careful · checks what must not be wrong — audits, briefs, vision. - **mastermind** — thinks · plans runs and designs harnesses. **All five arrive with a model already in them**, and the five together are the `balanced` @@ -413,7 +413,6 @@ under the class answering it, saying which model comes out. As shipped: | `router` | small work | whether a turn should have been work | | `consolidate` | small work | tidies what is remembered while nobody is here | | `taskname` | small work | the two or three words a task is called | -| `compaction` | careful work | the summary that survives a compaction | | `auditor` | careful work | whether finished-looking work is actually finished | | `vision` | careful work | reads images for a model that cannot see them | | `shaper` | careful work | the brief a task you started yourself is given | @@ -431,9 +430,9 @@ this build rather than a table someone kept up to date. Stop on a row and the li list is that role's own description followed by which class it follows. **What the mastermind's roles have in common is that one answer decides what all the other -calls do.** `planner` and `designer` used to sit on careful work beside the compaction -summary, which made one model id answer two unrelated bills: the careful calls are many and -short, and these are few. A planner that cuts badly spends a whole run on work nobody wanted; +calls do.** `planner` and `designer` used to sit on careful work beside the check on +finished work, which made one model id answer two unrelated bills: the careful calls are +many and short, and these are few. A planner that cuts badly spends a whole run on work nobody wanted; a designer that writes badly puts a wrong answer on the menu with a name on it; `routerconfirm` stands between a cheap model's "that should have been work" and a task starting itself, and it is asked on nothing else, so it costs a call only where something was @@ -473,7 +472,7 @@ only thing that moves it is `/model`, the model row in `/settings`, or naming on The crew is a different dial: the five **classes** aforge makes its own calls on — reflex, small work, worker, careful work, mastermind — used for titles, memory, the safety gate, -the work inside every task, checks on finished work, compaction summaries, adaptive-run +the work inside every task, checks on finished work, the brief a task is shaped into, adaptive-run planners and their nodes, harness pages, and looking at an image. Setting it writes all five class rows in one write, and **it is live from that moment**: the next call aforge makes on its own uses the new crew, with no relaunch and no new session. A task already @@ -634,7 +633,7 @@ for calls you did not type: | 2 | reflex | memory, titles, the safety gate — near-free, reads every turn | | 3 | small work | digests, task names, the safety gate's yes-or-no — cheap | | 4 | worker | every task you hand off, its parts, every run node — most of the bill | -| 5 | careful work | checks on finished work, compaction summaries, vision | +| 5 | careful work | checks on finished work, the brief a task is shaped into, vision | | 6 | mastermind | plans adaptive runs and designs harnesses — thinks | `/crew` shows all six and sets seats two to six in one word — `frugal`, `balanced` or @@ -2200,7 +2199,7 @@ When nothing can say — a model the catalog has never carried, a machine that h reached the catalog yet — the answer is still the 128,000-token default, which is the smallest window this surface routes to and the safe direction for a guess to be wrong in. -## What happens before the conversation is summarized +## What happens before the conversation is compacted aforge does not jump straight to summarizing. There are rungs before it. @@ -2286,36 +2285,43 @@ call, nothing paraphrased. This rung is chosen only when you gave `/compact` no is a workspace, there is page budget, and the model in use can read images. Pages are 120 columns by 64 lines, greyscale, deterministic, footed ` | context page 1 of 4`, and saved under `<workspace>/.aforge-v3/frames/`. The ceiling is **8 pages**; anything past it is -summarized and appended after the pages. +folded to a marker after the pages. -**Rung 3 — the summary.** One call on the session's own model, with no tools, that must -produce six sections — `## Goal`, `## Constraints & Preferences`, `## Progress`, -`## Key Decisions`, `## Next Steps`, `## Critical Context` — and must reproduce any -unanswered question verbatim and preserve exact paths, symbols, commands and error text. +**Rung 3 — the fold.** If the transcript is still too big after stubbing, the oldest +**assistant** work is replaced by one marker line. It is not a summary: nothing is described +and nothing is decided. -## What a compaction pass keeps +``` +[folded 43 messages · grep or read ~/.aforge/v3/projects/-you-work/<session>/journal.jsonl, lines 12..40] +``` -The transcript is cut at a message boundary, walking back from the tail until the keep-recent -budget is spent. The system message is never cut. +**Your own words are never folded.** A person's messages are the one thing in a transcript +nothing else can reconstruct, so the fold walks past them and takes only the assistant's. -What is rebuilt, in order: the system message, then the page images if there are any, then -the summary note if there is one, then **the state block verbatim**, then the kept tail. The -state block — what `track` and `commit` recorded — is injected directly and is **never routed -through the summarizer**, so working state cannot be paraphrased away. +## What a compaction pass keeps -The note the model reads above a summary begins: +**A compaction asks no model, costs nothing, and takes no time you can feel.** There is no +summarizer behind it — there was one, and it was deleted. It paid a model to write prose +about the text it was about to throw away, at the worst possible moment, and the loss was +unrecoverable because the transcript the prose came from went with it. -``` -[context compacted] Everything before this point was summarized to fit the context window. This note is the record of that conversation — it is not something either of us said, and any question inside it is still open. -``` +What replaces it is two mechanical passes over messages this session already has: tool +results become pointers to their own bytes, and then the oldest assistant work becomes one +marker line naming where the whole of it can still be read. + +What the model is handed instead of a summary is the **state card** — what `track` and +`commit` recorded — which rides in the system prompt on every turn and is kept up to date +after each one. So what the conversation is about is never paraphrased, because it was never +written as prose in the first place. -A pass can decline: `session: nothing to compact` (everything already fits in the tail), -`session: a compaction pass is already running`, or `session: summarizer returned nothing`. +A pass can decline: `session: nothing to compact` (everything already fits in the tail), or +`session: a compaction pass is already running`. ## What happens when the conversation gets too long — when compaction happens by itself When the conversation gets too long to fit, nothing is lost and nothing stops: the oldest -part of it is summarized away and the recent tail is kept, which is what compaction is. +part of it is stubbed and folded down to a marker and the recent tail is kept, which is what +compaction is. **Nothing is lost is meant literally, and you can go and look.** The session file keeps every original line, and scrolling up above the boundary is given those rather than the @@ -2346,16 +2352,14 @@ While a pass runs you see `compacting ~84k tokens` (`~842` under a thousand). On ## /compact — compacting now -`/compact` summarizes the conversation on demand. It notes `compacting…` immediately and runs +`/compact` compacts the conversation on demand. It notes `compacting…` immediately and runs the pass off the loop, so the surface stays alive. **Success is silent.** There is no "done" message — a compaction that worked simply leaves the conversation shorter. A failure comes back as `compact failed: ` followed by the error. -A `/compact` given a focus always goes to the summary rung rather than page images, because a -renderer cannot be careful about anything. The focus is *appended* to the summarizer's -instructions and never replaces them, so one careless phrase cannot cost the next session its -file paths. It travels on that one request and no further. +**It costs nothing and asks no model**, so there is no reason not to run it, and no `compaction` +role in settings to point at a model for it. ## Turning automatic compaction off diff --git a/internal/manual/chat/screen.md b/internal/manual/chat/screen.md index f94ef937fb..8d1109939f 100644 --- a/internal/manual/chat/screen.md +++ b/internal/manual/chat/screen.md @@ -3314,7 +3314,7 @@ same clock: | The word | What is happening | |---|---| | `running <tool>` | one call on the belt is executing | -| `checking` | a reader is deciding whether the answer finished the ask, or whether it should have been work. The two are asked **at the same time** rather than one after the other, so this stage lasts as long as the slower of them and not both | +| `checking` | a reader is deciding whether the answer finished your ask. It is the **only** reading left at the end of a turn that you wait for, because it decides whether the turn is over at all | | `checking whether this is safe to run` | the safety stand-in is being asked whether one command is plainly safe, before you are asked about it. This is **the one reading that really does come first** — it decides whether the command runs at all — and it answers in ten seconds or not at all | | `taking stock` | a second model is being shown an account of the work so far and asked what is left of what you asked for. **The work does not stop for it**: the next step goes out straight away and the reading happens alongside it. It happens **once**, and only on a reply that can no longer work where it is — its context full, the loop watch already spent, the wall run out, or a reply that said it had finished and then worked on for another ten rounds. The step it lands beside is stopped whatever the reading says; what the reading supplies is the list the work carries on with (*Tasks*, under *When a reply is taken out of your hands*) | | `tidying` | the conversation is being compacted | @@ -3366,7 +3366,9 @@ answer, never in front of it: - the **naming** of the conversation, on your first message (*Why does my tab say Untitled*); - the **work-or-words judge**, asking whether what you typed was really a job for - a task (*Tasks*); + a task (*Tasks*). It outlives your turn: it is the one reading that keeps + thinking after the answer is sealed, because what it does with a yes — start a + task — lands outside the turn anyway; - the **step captions** over a running batch; - the **reading of a long answer**, asking what is left of your question; - the **keeping pass** after the turn, deciding whether the exchange held anything @@ -3384,10 +3386,24 @@ there is nothing for it to run alongside and nothing it could be applied to afterwards. It gets ten seconds and the status line says `checking whether this is safe to run` for the whole of them. -**The end of an answer is a different wait.** Once the model has stopped writing -there is no work left to run beside, so the two readers that decide whether the -answer finished your ask and whether it should have been work are asked **at the -same time** and the line says `checking` until the slower of them is back. +**The end of an answer used to be a different wait, and is not any more.** Once +the model has stopped writing, two readers are still out: one asking whether the +answer finished your ask, and one asking whether what you typed should have been +work. + +Only the first can keep you. It decides whether the turn is **re-opened**, so +there is no turn to end until it has answered, and the line says `checking` while +it is out — normally a few seconds. + +The second cannot keep you and no longer does. What it does when it says yes is +**start a task**, which is just as good a minute later as it is now: the turn +ends, your answer is sealed, you can type again, and if the reading does come +back with a yes you get the ordinary one-line note — +`this looked like work, so task 4 started: …` — with the task on the rail beside +it. Before this it held the whole end of the turn open while it thought, with +your answer already finished on the screen and the line reading +`whether that should be work`. **That line is gone**, because the wait it named +is gone. ## A stage that lasts minutes keeps drawing — the phase went blank, the status line disappeared while it was still working, does a slow stage stop being shown diff --git a/internal/manual/chat/sessions-and-rewind.md b/internal/manual/chat/sessions-and-rewind.md index 907ab9fdae..f64d81e686 100644 --- a/internal/manual/chat/sessions-and-rewind.md +++ b/internal/manual/chat/sessions-and-rewind.md @@ -251,7 +251,7 @@ session: 12 is not a rewind point; the nearest is 10 conversation is being compacted. This is a refusal rather than a wait on purpose: interrupt first, rewind after, which is one keystroke. - `session: nothing to rewind` — a session nobody has spoken to yet, or one whose whole - transcript is a compaction summary. + transcript has been folded away by a compaction. - `session: agent is closed`. - The "not a rewind point" sentence names the nearest legal point. The index is never rounded to it for you. diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 33b6c641e0..2aec90559e 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -619,6 +619,15 @@ nobody had seen yet, and the task itself answers it better by existing — it is roster, it says what it is doing, and `x` stops it like any other task. Nothing else is different about it: a row, a room, a report, and everything else on this page. +**It never holds up your answer, and the line can arrive after the turn ends.** +The reading outlives the turn that bought it: your answer is sealed, the total is +drawn and you can type again while the two models are still thinking. A yes that +lands after that gets the same dim line in the same place, and the task is on the +rail beside it — nothing about it arrives differently for being a moment late. +It used to be the other way round: the whole end of the turn waited on this +reading, with the answer already finished on the screen and the status line +reading `whether that should be work`. That wait is gone and so is that line. + **What keeps it from becoming a nuisance:** - **At most one start every three turns.** Two can never arrive back to back, so a diff --git a/internal/roles/roles.go b/internal/roles/roles.go index 3489da23be..4dc8344047 100644 --- a/internal/roles/roles.go +++ b/internal/roles/roles.go @@ -2,8 +2,8 @@ // call the surface makes, and one rule for which model answers it. // // An auxiliary call is any call the person did not type — the title a session -// names itself, the summary a compaction writes, the advisor's aside, a commit -// message. Each is a ROLE. Roles group under TIERS, and a tier is what the +// names itself, the judge that asks whether a turn should have been work, the +// gate that reads one tool call for safety. Each is a ROLE. Roles group under TIERS, and a tier is what the // person actually configures: "low model = X, high model = Y", set once. New // auxiliary calls register a role, inherit their tier's model, and need no // settings of their own; the person never learns a new knob per feature. @@ -37,8 +37,6 @@ type Role string const ( // RoleTitle names a session from its opening exchange. RoleTitle Role = "title" - // RoleCompaction summarizes a transcript that outgrew its window. - RoleCompaction Role = "compaction" // RoleConsolidate is the dreaming pass over what a session has remembered: // one call over fifty short lines whose whole instruction is "merge the // duplicates and drop what is superseded" (internal/session's @@ -214,6 +212,46 @@ const ( // internal/session/subharness_intake.go, which owns the call. RoleIntake Role = "intake" + // RoleRepair is the hands that fix what the cheap hands got wrong. + // + // THE TIER IS HIGH AND NOT THE MASTERMIND'S, for [RoleCareful]'s reason: a + // repair round is many turns of ordinary work done by a model that can be + // trusted with something subtle, not one answer that decides what every other + // call does. Registered from internal/session/repair_role.go, which owns the + // call. + RoleRepair Role = "repair" + + // RoleSentinel is one standing item's cheap yes-or-no: is what this check + // found worth telling the person about. + // + // IT SITS LOW for the guardian's reason — it reads a few kilobytes and + // answers one binary question, and a wrong no costs a check that said nothing + // rather than money. It is also the call this build makes most often with + // nobody in front of it, once per check of every item forever. Registered + // from internal/session/standing_run.go, which owns the call. + RoleSentinel Role = "sentinel" + + // RoleSpellOut expands a half-written request into what it obviously meant, + // for the person to read and keep or drop. + // + // IT SITS LOW, with the namer and the sentinel rather than with the shaper. + // The shaper writes the only document an autonomous worker will ever read, so + // a vague answer there costs a whole task's spend; this one writes three lines + // a person reads on screen before deciding whether to keep them, and a weak + // answer costs one esc. A person who wants it thought about harder pins it + // (`roles.spellout: <model>`). Registered from internal/session/spellout.go, + // which owns the call. + // + // THE WORD LIVES HERE AND NOT AT THE CALL SITE, which is the whole reason + // this constant moved. It was `const spellOutRole roles.Role = "spellout"` in + // internal/session, and every reader that asks this package what the role + // vocabulary IS — cmd/aforge-replay's roleWords, which parses this file — could + // not see it. So `spellout` reached the cost report as an unrecognised tag and + // was priced as a background errand with nobody waiting, when it is the one + // auxiliary a person sits and watches. A role declared anywhere else is a role + // that is invisible to everything that reads roles. + RoleSpellOut Role = "spellout" + // RoleDivision reviews a DIVISION as a plan. A worker halfway through its // own work has named the parts it wants to hand out, and this is the one // call that reads them TOGETHER — the evidence, the parent's own brief, and @@ -290,7 +328,6 @@ const ( // designer that writes badly puts a wrong answer on the menu with a name on // it; a division reviewed badly hands four workers four briefs that nobody // inside them can correct. The first two were on the high tier, beside the - // compaction summary and the // auditor, which made a person choosing "the capable model" choose one // figure for two very different bills: the careful calls are many and short, // the mastermind's are few and worth thinking about. Separating them is what @@ -317,10 +354,10 @@ func known(tier Tier) bool { // DefaultAssignment is the tier each built-in role starts on. // // Titles are disposable prose: a wrong one costs a glance and is rewritten by -// the next session, so it goes to the cheap model. A compaction summary is the -// session's memory — everything before the cut is gone and only the summary -// survives it — so it goes to the capable one. The asymmetry is about what a -// bad answer destroys, not about how hard the task reads. +// the next session, so it goes to the cheap model. The planner's amendment is +// the opposite — every node the rest of a run pays for is a node it cut — so it +// goes to the most capable one there is. The asymmetry is about what a bad +// answer destroys, not about how hard the task reads. // // THE THREE RUN ROLES ARE ASSIGNED HERE rather than from the files that make // their calls, which is the arrangement guardian, vision and the auditor keep. @@ -330,14 +367,13 @@ func known(tier Tier) bool { // happens, many cheap ones that do it. Split across three files, that balance // is three unrelated lines nobody reads together. var DefaultAssignment = map[Role]Tier{ - RoleTitle: TierLow, - RoleCaption: TierLow, - RoleCompaction: TierHigh, - RolePlanner: TierMastermind, - RoleDesigner: TierMastermind, - RoleWorker: TierWorker, - RoleRouter: TierLow, - RoleReflex: TierReflex, + RoleTitle: TierLow, + RoleCaption: TierLow, + RolePlanner: TierMastermind, + RoleDesigner: TierMastermind, + RoleWorker: TierWorker, + RoleRouter: TierLow, + RoleReflex: TierReflex, } // ErrUnknownRole is returned by [Resolve] for a role that was never @@ -383,15 +419,14 @@ func TierKey(tier Tier) string { return tierPrefix + string(tier) } // A package that does pass one to [Register] overwrites its entry here, which is // the same last-one-wins rule the tier assignment keeps. var roleDescriptions = map[Role]string{ - RolePlanner: "the plan that steers an adaptive run", - RoleDesigner: "writes and reviews a harness page", - RoleAuditor: "whether finished-looking work is actually finished", - RoleCompaction: "the summary that survives a compaction", - RoleWorker: "one node of an adaptive run", - RoleTitle: "the name a session gives itself", - RoleIntake: "filling in a program's form from what was already said", - RoleGuardian: "is this one tool call plainly safe", - RoleRouter: "whether a turn should have been work", + RolePlanner: "the plan that steers an adaptive run", + RoleDesigner: "writes and reviews a harness page", + RoleAuditor: "whether finished-looking work is actually finished", + RoleWorker: "one node of an adaptive run", + RoleTitle: "the name a session gives itself", + RoleIntake: "filling in a program's form from what was already said", + RoleGuardian: "is this one tool call plainly safe", + RoleRouter: "whether a turn should have been work", // The cascade's second half, and the two calls a division makes. RoleRouterConfirm: "a second look before work starts itself", RoleMarkReader: "what is left of a long answer, and whether it has parts", @@ -482,6 +517,68 @@ func Registered() []Role { return roles } +// Vocabulary is EVERY role this build has a word for, which is not the same set +// as [Registered]. +// +// A ROLE WITH NO TIER IS STILL A ROLE. [RoleImageGen] is the standing example: +// a painter is chosen by a pin or not at all — "a chat model in tiers.high is +// not a statement about painting" — so nothing ever calls [Register] for it and +// the registry has never heard of it. Reading the registry as the answer to +// "what are the roles" therefore throws a person's `imagegen:` pin away, which +// is a real setting doing real work. +// +// So this is the vocabulary and the registry is the tier table. They are +// different questions and this package now answers both. +// +// THE LIST IS COMPLETE BECAUSE A LAW SAYS SO, not because somebody remembers: +// cmd/aforge-replay's TestEveryRoleWordIsDeclaredWhereTheVocabularyIs already +// parses this file for every Role constant, and fails when one of them is +// missing from here. +func Vocabulary() []Role { + return append([]Role(nil), vocabulary...) +} + +// Known reports a written name as one of this build's roles, registered or not. +func Known(name string) bool { + key := RoleKey(name) + for _, role := range vocabulary { + if RoleKey(string(role)) == key { + return true + } + } + return false +} + +var vocabulary = []Role{ + RoleAuditor, RoleCaption, RoleCareful, RoleConsolidate, + RoleDesigner, RoleDivision, RoleGuardian, RoleHandoff, + RoleImageGen, RoleIntake, RoleJobName, RoleMarkReader, + RolePlanner, RoleReflex, RoleRepair, RoleRouter, + RoleRouterConfirm, RoleSentinel, RoleShaper, RoleSpeech, + RoleSpellOut, RoleTaskName, RoleTitle, RoleVideo, + RoleVision, RoleWorker, +} + +// RoleKey is how a role name written by a person is compared: trimmed and +// lower-cased. +// +// IT IS ONE SPELLING BECAUSE A SECOND ONE EVENTUALLY DISAGREES. A row typed +// `Compaction: x` and one typed `compaction:x` are the same pin, and every +// reader that decides whether a word is a role has to answer that the same way +// or one of them will recognise a name the others do not. +// +// THERE IS NO LEDGER OF RETIRED ROLES, DELIBERATELY. One was written here and +// then deleted: a map of the words this build used to answer to, so a dropped +// pin could be told apart from a typo. It could not earn its keep. Both are a +// pin no call will ever consult, both should stop being written back, and the +// only reader who can act on either is the person adding a word right now — +// which is the one place internal/config still refuses (its writeModelRoles). +// Keeping the ledger meant every future deletion owed it an entry, and the day +// somebody forgot, the old failure would come back exactly as it was. +func RoleKey(name string) string { + return strings.ToLower(strings.TrimSpace(name)) +} + // TierOf reports the tier a role resolves under, and false if the role was // never registered. func TierOf(role Role) (Tier, bool) { @@ -610,9 +707,12 @@ func Patience(tier Tier) time.Duration { // already cost more than the answer is worth. return 45 * time.Second case TierHigh: - // Long enough for a compaction summary over a full window — the one - // call on this tier that legitimately reads a great deal before it - // writes anything. + // Long enough for the AUDITOR over a whole piece of finished-looking + // work — the one call on this tier that legitimately reads a great deal, + // and gathers evidence of its own, before it writes anything. (It was + // derived from the compaction summary until that role was deleted; the + // summariser had already stopped existing, and the figure it justified + // had outlived it.) return 5 * time.Minute case TierMastermind: // A planner reading a whole run, or a designer writing a page everybody diff --git a/internal/roles/roles_test.go b/internal/roles/roles_test.go index df0251f325..1bc52dd378 100644 --- a/internal/roles/roles_test.go +++ b/internal/roles/roles_test.go @@ -34,6 +34,11 @@ func restore(t *testing.T) { } func TestResolvePrecedence(t *testing.T) { + // THE HIGH TIER HAS NO BUILT-IN TENANT. Every careful call — the auditor, + // the shaper, vision, the repair — is registered from the package that makes + // it, so a case about that tier registers one here through the same door. + restore(t) + Register(Role("audit"), TierHigh) tests := []struct { name string src Source @@ -65,19 +70,22 @@ func TestResolvePrecedence(t *testing.T) { { // The default assignment reaching for the other tier is the whole // point of tiers: two roles, one setting each, different models. - name: "compaction takes the high tier", + // The high tier has no BUILT-IN tenant — every careful call is + // registered from the package that makes it — so the case registers + // one, which is the same door those packages use. + name: "a high-tier role takes the high tier", src: settings(map[string]string{ "tiers.low": "low-model", "tiers.high": "high-model", }), - role: RoleCompaction, + role: Role("audit"), fallback: "session-model", want: "high-model", }, { name: "session default when nothing is set", src: settings(map[string]string{}), - role: RoleCompaction, + role: RoleTitle, fallback: "session-model", want: "session-model", }, @@ -228,9 +236,9 @@ func TestResolvePrecedence(t *testing.T) { { name: "a pin resolves without any tier configured", src: settings(map[string]string{ - "roles.compaction": "pinned-model", + "roles.title": "pinned-model", }), - role: RoleCompaction, + role: RoleTitle, fallback: "", want: "pinned-model", }, @@ -344,7 +352,7 @@ func TestRegisteredIsSortedAndComplete(t *testing.T) { Register(Role("commit"), TierLow) want := []Role{ - Role("advisor"), RoleCaption, Role("commit"), RoleCompaction, + Role("advisor"), RoleCaption, Role("commit"), RoleDesigner, RolePlanner, RoleReflex, RoleRouter, RoleTitle, RoleWorker, } for range 5 { // map order varies per iteration; the answer must not @@ -364,11 +372,10 @@ func TestDefaultAssignment(t *testing.T) { // Disposable prose cheap, the session's memory capable — and the run's // balance: one careful call deciding what happens, many cheap ones doing it. for role, want := range map[Role]Tier{ - RoleTitle: TierLow, - RoleCaption: TierLow, - RoleCompaction: TierHigh, - // The two masterminds. They were on the high tier beside the compaction - // summary, which made one figure answer two different bills. + RoleTitle: TierLow, + RoleCaption: TierLow, + // The two masterminds. They were on the high tier beside the auditor, + // which made one figure answer two different bills. RolePlanner: TierMastermind, RoleDesigner: TierMastermind, RoleWorker: TierWorker, @@ -394,8 +401,8 @@ func TestReadHelpers(t *testing.T) { if model, ok := Pinned(src, RoleTitle); !ok || model != "pinned-model" { t.Fatalf("Pinned(title) = %q, %v, want pinned-model, true", model, ok) } - if _, ok := Pinned(src, RoleCompaction); ok { - t.Fatalf("Pinned(compaction) reported a pin that is not set") + if _, ok := Pinned(src, RoleWorker); ok { + t.Fatalf("Pinned(worker) reported a pin that is not set") } if model, ok := TierModel(src, TierHigh); !ok || model != "high-model" { t.Fatalf("TierModel(high) = %q, %v, want high-model, true", model, ok) @@ -406,8 +413,8 @@ func TestReadHelpers(t *testing.T) { } func TestKeys(t *testing.T) { - if got := PinKey(RoleCompaction); got != "roles.compaction" { - t.Fatalf("PinKey(compaction) = %q, want roles.compaction", got) + if got := PinKey(RoleWorker); got != "roles.worker" { + t.Fatalf("PinKey(worker) = %q, want roles.worker", got) } if got := TierKey(TierLow); got != "tiers.low" { t.Fatalf("TierKey(low) = %q, want tiers.low", got) diff --git a/internal/session/afterturn_law_test.go b/internal/session/afterturn_law_test.go new file mode 100644 index 0000000000..13abd8d8f9 --- /dev/null +++ b/internal/session/afterturn_law_test.go @@ -0,0 +1,62 @@ +package session + +import ( + "context" + "testing" + "time" +) + +// A READING THAT OUTLIVES ITS TURN MUST NOT OUTLIVE THE SESSION. +func TestAfterTurnRefusesOnceTheSessionIsClosing(t *testing.T) { + after := newAfterTurn() + ctx, done, ok := after.begin() + if !ok || ctx == nil { + t.Fatal("a fresh lifetime refused work") + } + landed := make(chan struct{}) + go func() { <-ctx.Done(); close(landed) }() + settled := make(chan struct{}) + go func() { after.settle(time.Second); close(settled) }() + select { + case <-landed: + case <-time.After(2 * time.Second): + t.Fatal("settle did not cancel the work in flight") + } + done() + select { + case <-settled: + case <-time.After(2 * time.Second): + t.Fatal("settle did not return once the work unwound") + } + if _, _, ok := after.begin(); ok { + t.Fatal("a closed lifetime started new work") + } + // A second settle is a no-op rather than a second cancel or a panic. + after.settle(time.Millisecond) +} + +// AND THE GRACE BOUNDS IT: work that ignores its cancellation cannot hold a quit. +func TestAfterTurnDoesNotWaitForeverOnWorkThatIgnoresIt(t *testing.T) { + after := newAfterTurn() + _, done, ok := after.begin() + if !ok { + t.Fatal("a fresh lifetime refused work") + } + began := time.Now() + after.settle(50 * time.Millisecond) + if waited := time.Since(began); waited > time.Second { + t.Fatalf("settle waited %s on work that never unwound, want about its grace", waited) + } + done() +} + +// AND A NIL LIFETIME ANSWERS, because every door here is reached on a session +// that may not have built one. +func TestANilAfterTurnAnswersWithoutPanicking(t *testing.T) { + var after *afterTurn + if _, _, ok := after.begin(); ok { + t.Fatal("a nil lifetime said it had started work") + } + after.settle(time.Millisecond) + _ = context.Background() +} diff --git a/internal/session/agent.go b/internal/session/agent.go index 1ea51a5b2f..509a48774e 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -164,6 +164,10 @@ func newAgent(config Config, client Completer) (*Agent, error) { // session may name itself and the errand starts on the first message rather // than at the end of a turn (title.go). agent.titleCtx, agent.titleStop = context.WithCancel(context.Background()) + // AND THE LIFETIME EVERY READING THAT OUTLIVES ITS TURN RUNS ON, minted for + // the same reason the two above are: a turn that ends is not the end of the + // question it asked (sidecar.go's [afterTurn]). + agent.after = newAfterTurn() // The registry is built before the belt because the belt closes over it: // bash's background path and the jobs tool are both views onto this one // object, and it is the agent's own steering queue they report into. @@ -2274,6 +2278,11 @@ func (a *Agent) Close() error { // that has already earned a name owes the journal one line, and cutting the // process between the answer and the append would lose it (title.go). a.waitForTitle() + // AND NO READING THAT OUTLIVED ITS TURN OUTLIVES THE SESSION. It is cancelled + // and then joined, on the namer's order and for the namer's reason: what is + // under here owes no store a write, so a decision nobody is going to read is + // worth none of a person's quit (sidecar.go's [afterTurn.settle]). + a.after.settle(closeGrace) if done != nil { timer := time.NewTimer(closeGrace) select { diff --git a/internal/session/auxiliary.go b/internal/session/auxiliary.go index ff93f447c7..22058bc2c7 100644 --- a/internal/session/auxiliary.go +++ b/internal/session/auxiliary.go @@ -123,10 +123,24 @@ func (a *Agent) callRoleChecked(ctx context.Context, role roles.Role, sessionDef if err != nil { return nil, "", err } - // EVERY ERRAND ARMS ITS CUT MONEY HERE. [Agent.callRole] is the one door all - // auxiliary provider calls pass through, including calls whose own deadline - // is not derived from a turn; arming ten callers separately would leave the - // next errand able to lose the receipt this shared seam exists to keep. + // EVERY ERRAND ARMS ITS CUT MONEY HERE. [Agent.callRole] is the one door every + // ERRAND passes through, including errands whose own deadline is not derived + // from a turn; arming them separately would leave the next one able to lose + // the receipt this shared seam exists to keep. + // + // AND "ERRAND" IS NARROWER THAN "MODEL CALL", WHICH IS WHAT THIS SENTENCE USED + // TO CLAIM. It said this was the door all auxiliary provider calls pass + // through, and ten did not: the guardian, vision, the shaper, the spell-out, + // the intake, the planner, the designer, the ceiling's handoff draft, a saved + // program's own step and the standing sentinel each reached the wire by + // another road. Some of them still do, legitimately — vision STREAMS into the + // room in the person's own turn, the handoff draft is billed to the turn on + // the turn's own model, a program's step runs on the program's model and no + // role's — and an errand ladder wrapped round any of those would be the wrong + // shape twice over. What every one of them owes is the TAG, and that is the + // claim that is now true and enforced: clientdoor.go's [callPurpose] is the + // one door every REQUEST passes through, this one is the one door every + // ERRAND passes through, and neither pretends to be the other. ctx = provider.WithReconcile(ctx, a.reconciled) if len(rungs) > roleFallThroughs+1 { rungs = rungs[:roleFallThroughs+1] @@ -181,19 +195,13 @@ func (a *Agent) callRoleChecked(ctx context.Context, role roles.Role, sessionDef errandCtx, endErrand := context.WithTimeout(ctx, patience) // AND THE ERRAND SAYS WHAT IT IS FOR, ON EVERY ROW IT WRITES. // - // The model-call log names a call by its tag, and a tag is either set here - // or derived from a routing slot the planning packages open — and this - // package opens none. So every errand this session makes, from every one of - // the ten callers below, landed in the log with no tag at all: 2,309 of the - // 2,839 untagged finishes in the ten days to 2026-09-10, and with them the - // answer to "what was this build spending deepseek-v4-flash on all night" - // (docs/design/recovery/census-20260910.md §8, finding 9). - // // THE ROLE IS THE TAG, because the role is what the errand IS — naming a // conversation, judging a route, writing a caption — and it is the same word // internal/lane's roles.go and the journal's own call line already use, so - // three records of one call agree about what to call it. - errandCtx = provider.WithCallTag(errandCtx, string(role)) + // three records of one call agree about what to call it. It is carried to + // the door as a [callPurpose] rather than stamped on the context here: the + // door is what every request in this package passes over, and it is the only + // thing that spells the tag (clientdoor.go says why that had to move). defer endErrand() tell := errandWatchFrom(ctx) @@ -275,7 +283,7 @@ func (a *Agent) callRoleChecked(ctx context.Context, role roles.Role, sessionDef served := &provider.ServedEndpoint{} callCtx = provider.WithServedEndpoint(callCtx, served) callCtx, releaseRung := errandRungContext(callCtx, len(rungs)-attempt-1) - response, called, callErr := a.completeWithNamedModel(callCtx, messages, rung.Model, options...) + response, called, callErr := a.completeWithNamedModel(callCtx, callPurpose(role), messages, rung.Model, options...) if strings.TrimSpace(called) == "" { called = rung.Model } diff --git a/internal/session/belt_wiring_test.go b/internal/session/belt_wiring_test.go index b671513a7b..690df231ad 100644 --- a/internal/session/belt_wiring_test.go +++ b/internal/session/belt_wiring_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - configpkg "github.com/Agent-Field/aforge-v2/internal/config" "github.com/Agent-Field/aforge-v2/internal/subharness" ) @@ -30,20 +29,54 @@ import ( // a registry to design into, a runner to run what is designed, an adaptive // runner, and a surface that answers questions. -// v3ShapedAgent is a conversation configured the way the interactive door -// configures one. +// v3ShapedAgent is the conversation the interactive door configures, which is +// [shippedShapeAgent] and is not a second opinion about it. +// +// IT USED TO BE A SECOND OPINION, AND THAT WAS THE MEASURED HOLE. This function +// built its own thin config — no memory store, no accounts hub, no standing +// items, no saved programs, eighteen tools — and four tests weighed the shipping +// door through it, including both prefix budgets. A machine somebody has +// finished setting up carries five more tools and fourteen kilobytes more +// prefix, so every one of those gates reported on a conversation nobody has. +// There is ONE shipped shape now and this is a name for it. func v3ShapedAgent(t *testing.T) *Agent { t.Helper() - agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { - config.AskConsent = true - config.BashBackgroundAfterSeconds = configpkg.DefaultBashBackgroundAfter - config.HarnessStore = subharness.At(t.TempDir()) - config.RunHarness = func(context.Context, string, string, string, func(subharness.Trail)) (string, subharness.Usage, error) { - return "", subharness.Usage{}, nil - } - config.OrchestrateRunner = func(context.Context, string, string, float64) (string, error) { - return "", nil + return shippedShapeAgent(t) +} + +// shippedBeltShape names the row of [beltShapes] that IS the shipping +// conversation, fully wired. +// +// THE INDEX IS NOT THE IDENTITY. `beltShapes[0]` read the fullest shape only for +// as long as nobody put a row in front of it — and what reads it is a budget +// whose whole job is to be the number for the SHIPPED belt, which would have +// gone on passing, quietly, against whatever shape had moved into slot zero. +// That is the same failure this fixture exists to repair, one layer down. +const shippedBeltShape = "a conversation that remembers" + +// beltShapeNamed is one row of [beltShapes] by its own name, and fails loudly +// rather than returning a zero shape that would build an agent out of nothing. +func beltShapeNamed(t *testing.T, name string) beltShape { + t.Helper() + names := make([]string, 0, len(beltShapes)) + for _, shape := range beltShapes { + if shape.name == name { + return shape } + names = append(names, shape.name) + } + t.Fatalf("there is no belt shape called %q; the shapes are %q", name, names) + return beltShape{} +} + +func shippedShapeAgent(t *testing.T) *Agent { + t.Helper() + shape := beltShapeNamed(t, shippedBeltShape) + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + // The rendered page is what the budget weighs, so this shape renders its + // own rather than taking newTestAgent's fixed one. + config.System = "" + shape.build(t, config) }) return agent } diff --git a/internal/session/checkpoint.go b/internal/session/checkpoint.go index d2411b51b9..4d2de08d63 100644 --- a/internal/session/checkpoint.go +++ b/internal/session/checkpoint.go @@ -4859,8 +4859,16 @@ func (a *Agent) checkpointBrief(ctx context.Context, turn *Usage, model string) // answer rather than the answer itself, so it is priced and drawn as the // errand it is: a person is reading the turn this ends, and the clock over // their answer is not this call's to move (internal/lane's roles.go). + // + // AND IT SAYS WHAT IT IS, which until now it did not: this rung reached the + // wire with no tag, so the dearest side-call of the whole ceiling road — + // a mastermind reading the turn's entire transcript — was billed to the turn + // and filed under nothing. It is its own purpose rather than the turn's, + // because it is not the turn's question: the turn asked what the person + // asked, and this asks what is left of it (clientdoor.go). response, err := a.completeWithModel( - provider.WithRole(provider.WithoutStream(ctx), lane.RoleAuxiliary), messages, + provider.WithRole(provider.WithoutStream(ctx), lane.RoleAuxiliary), + purposeHandoffDraft, messages, model) if err != nil || response == nil { // AND THE FAULT IS CARRIED OUT OF HERE RATHER THAN SPELLED AS SILENCE. This diff --git a/internal/session/checkpoint_test.go b/internal/session/checkpoint_test.go index da2dbe3e04..02e897fb2c 100644 --- a/internal/session/checkpoint_test.go +++ b/internal/session/checkpoint_test.go @@ -1443,10 +1443,7 @@ func TestADowryOfMachineMarkupIsRefusedAndNeverBecomesTheName(t *testing.T) { // this line now, on purpose (#333), and the fixture answers it with no name // ([answerTheReadingsOffTheQueue]) — so what is left to read here is the // told-after line as the road itself draws it, off the person's own sentence. - notice := routeNotice(collected) - if notice == "" { - t.Fatalf("no task was announced; notices were %q", noticeTexts(collected)) - } + notice := routeSaid(t, agent, collected) if strings.Contains(notice, dsmlSentinel) || strings.Contains(notice, "DSML") { t.Errorf("the sentinel became the task's name: %q", notice) } @@ -1507,8 +1504,8 @@ func TestAContinuationSayingNothingIsLeftDropsTheCeilingHandover(t *testing.T) { t.Errorf("the person was told their answer was being moved and then watched it finish where "+ "it was; notices were %q", noticeTexts(collected)) } - if routeNotice(collected) != "" { - t.Errorf("a task was announced: %q", routeNotice(collected)) + if routeNotice(agent, collected) != "" { + t.Errorf("a task was announced: %q", routeNotice(agent, collected)) } // AND THE TURN'S OWN ANSWER STANDS. The transcript ends on the model's words, // not on a line the harness wrote over the top of them. diff --git a/internal/session/checkwindow_test.go b/internal/session/checkwindow_test.go index f12018075a..d4cc8827f4 100644 --- a/internal/session/checkwindow_test.go +++ b/internal/session/checkwindow_test.go @@ -113,7 +113,7 @@ func TestAWindowedCallIsToldTheTimeItHasLeft(t *testing.T) { send := func(ctx context.Context) int { t.Helper() before := server.requests() - if _, err := agent.completeWithModel(provider.WithLaneChoice(ctx, lanes.Choice{Order: []string{lane}}), ask, model); err != nil { + if _, err := agent.completeWithModel(provider.WithLaneChoice(ctx, lanes.Choice{Order: []string{lane}}), purposeTurn, ask, model); err != nil { t.Fatalf("completeWithModel: %v", err) } return server.reasoningBudget(t, before) diff --git a/internal/session/clientdoor.go b/internal/session/clientdoor.go index aa99c122d9..8b4322fe41 100644 --- a/internal/session/clientdoor.go +++ b/internal/session/clientdoor.go @@ -300,7 +300,7 @@ func (c modelRoutingCompleter) CompleteWithMessages(ctx context.Context, message if model == "" { model = c.agent.Model() } - return c.agent.completeWithModel(ctx, messages, model, options...) + return c.agent.completeWithModel(ctx, purposeInherited, messages, model, options...) } // routedCompleter returns a completer safe to hand to a package that chooses a @@ -421,11 +421,110 @@ func (a *Agent) completerFor(model string) (Completer, string, string, error) { return inner, wire, called, nil } +// ── EVERY REQUEST SAYS WHAT IT IS FOR ─────────────────────────────────────── +// +// callPurpose is what one request this package makes is FOR, in the word the +// model-call log files it under (internal/provider's [provider.WithCallTag]). +// +// IT IS AN ARGUMENT AND NOT A CONTEXT VALUE, and that is the whole of the fix. +// The tag was a context value that a caller could set or forget, and nine of the +// eleven callers of this door forgot: the guardian, vision, the shaper, the +// spell-out, the intake, the planner, the designer, the handoff draft and a +// saved program's own step all reached the wire with no tag at all. That is +// 2,309 of the 2,839 untagged finishes in the ten days to 2026-09-10, and with +// them the answer to "what was this build spending that model on all night" +// (docs/design/recovery/census-20260910.md §8, finding 9). A tag that can be +// forgotten is a tag that will be, so it is spelled where it cannot be: a call +// with no purpose does not compile. +// +// AND IT IS STAMPED HERE AND NOWHERE ELSE. The turn, the ask tool and the +// errand ladder each used to stamp their own, which is three spellings of one +// fact; they pass a purpose now and this door is the only thing in the package +// that calls [provider.WithCallTag]. nohiddenwork_test.go is the law. +type callPurpose string + +// The purposes that are not a role's own name. A role-resolved call passes its +// role ([Agent.callRole]) and a tool ask passes the tool, so both are derived +// rather than listed; these are the three requests that are not either. +const ( + // purposeTurn is the person's own question, answered in their conversation. + purposeTurn callPurpose = "turn" + // purposeTask is the same request made inside a piece of work. + purposeTask callPurpose = "task" + // purposeSubharness is one AI step of a saved program, which runs on the + // program's own model rather than on any role's (subharness_env.go). + purposeSubharness callPurpose = "subharness" + // purposeHandoffDraft is the ceiling's draft rung: the model that has just + // spent the turn, asked on the turn's own transcript for the document a + // worker will finish from (checkpoint.go). It is not [roles.RoleHandoff], + // which is the rung BELOW it — the writer that composes a brief out of the + // digest when this one cannot be had — and naming them alike would put the + // dearest call on the road and its cheap understudy on the same row. + purposeHandoffDraft callPurpose = "handoff-draft" + + // ── the three roads that carry their own client ───────────────────────── + // + // These do not come through [Agent.completeWithNamedModel] at all: each + // builds a [provider.Client] of a shape the door does not make — a longer + // timeout, static routing pinned to the parent's choice, a document config + // with its own key rule — and completes on it directly. That is a real + // difference and not an oversight, so the answer is not to force them + // through the door; it is to make them say the same word the door says. + // + // THEY REACHED THE CALL LOG WITH NO TAG AT ALL UNTIL THIS PR, which meant the + // two slowest unattended errands this build makes — a memory tidy-up and a + // standing item's check, both of which run while nobody is there — were + // indistinguishable from a turn that had lost its name. They carry the most + // money per call of anything nobody is waiting for. + + // EVERY WORD IN THIS BLOCK IS A WORD THAT IS NOT A ROLE. Where a purpose IS a + // role, the call site writes `callPurpose(roles.RoleX)` and no constant is + // declared here — restating a role's string would be the same word in two + // places, and the point of a small closed block is that what IS in it stands + // out. The memory tidy-up is [roles.RoleConsolidate] and says so at its own + // call; the two below are not roles at all. + // + // purposeSentinel is one standing item's yes-or-no on evidence somebody else + // already gathered, run on every check of every item forever. It is the ONE + // DELIBERATE DIVERGENCE: the call resolves on [roles.RoleSentinel], but the + // tag is `standing-check`, because cmd/aforge already writes `sentinel` for + // the resident's quorum errand and two different calls under one tag is one + // reading of neither. THE RENAME IS A HOLDING ACTION: it moves the collision + // rather than fixing it, and it is fixed by carrying [lane.Role] on the + // calllog record beside the tag, where the two calls are told apart by what + // they ARE and both can spell `sentinel` again. That is issue #1010, whose + // spine is #928. + purposeSentinel callPurpose = "standing-check" + // purposeDocument is a rung of the document reader — the model's own eyes on + // a PDF the `read` tool cannot open as text. It is not a role because the + // reader resolves its model from the document settings and never from the + // role table (tools_doc.go). + purposeDocument callPurpose = "document" + + // purposeInherited is the ONE PURPOSE THAT IS THE ABSENCE OF ONE, and it is + // named so that the absence is a statement rather than a gap. + // + // [modelRoutingCompleter] is the view of a live agent that LEAVES this + // package — internal/reflex and the media hands reach the wire through it — + // and each of those callers names its own call on the context before it gets + // here (reflex.go's "reflex"). A word stamped at this point would overwrite + // what they said with a word about the wrapper rather than about the request, + // so this one keeps whatever the caller already put there. + // + // It exists because the alternative was a law that had to KNOW about this + // call site: a walk that tracked which function it was inside, a name to + // compare against, and a reader for generic receivers — thirty lines of + // apparatus to excuse one bare `""`. A named constant says the same thing in + // the place a reader is already looking. + purposeInherited callPurpose = "" +) + // completeWithModel is [Agent.completerFor] joined to the one wire-model -// option. Keeping the two operations inseparable makes it impossible to change -// a slug while accidentally retaining another service's address and bearer. -func (a *Agent) completeWithModel(ctx context.Context, messages []ai.Message, model string, options ...ai.Option) (*ai.Response, error) { - response, _, err := a.completeWithNamedModel(ctx, messages, model, options...) +// option and to the one statement of what the call is for. Keeping the three +// inseparable makes it impossible to change a slug while accidentally retaining +// another service's address and bearer, or to reach the wire anonymously. +func (a *Agent) completeWithModel(ctx context.Context, purpose callPurpose, messages []ai.Message, model string, options ...ai.Option) (*ai.Response, error) { + response, _, err := a.completeWithNamedModel(ctx, purpose, messages, model, options...) return response, err } @@ -433,17 +532,42 @@ func (a *Agent) completeWithModel(ctx context.Context, messages []ai.Message, mo // resolution that chose its account. Errand receipts need that identity when a // missing default-service key moved the call onto the live seat; asking the pool // a second time afterwards could observe a different model or source set. -func (a *Agent) completeWithNamedModel(ctx context.Context, messages []ai.Message, model string, options ...ai.Option) (*ai.Response, string, error) { +func (a *Agent) completeWithNamedModel(ctx context.Context, purpose callPurpose, messages []ai.Message, model string, options ...ai.Option) (*ai.Response, string, error) { client, wire, called, err := a.completerFor(model) if err != nil { return nil, called, err } + // THE PURPOSE BECOMES THE TAG, on the one line every request in this package + // passes over — through [withPurpose], which is the only function in + // internal/session that spells it, here and for the three roads that carry a + // client of their own. + ctx = withPurpose(ctx, purpose) // A CALL UNDER A TOLD WINDOW IS TOLD IT HERE, at the last moment the context // is this package's to change (callwindow.go says why it cannot be earlier). response, err := client.CompleteWithMessages(toldItsWindow(ctx), messages, append(options, ai.WithModel(wire))...) return response, called, err } +// withPurpose stamps a purpose on a context, and it is THE ONE PLACE IN +// internal/session THAT SPELLS `provider.WithCallTag` — a law says exactly once +// (nohiddenwork_test.go). +// +// [Agent.completeWithNamedModel] calls it, so every request through the one door +// is stamped here; so do the three roads that complete on a client of their own, +// because each needs a client shape the door does not make. What those roads +// needed was never permission to write a tag: it was a door of their own that is +// the same door. +// +// AN EMPTY PURPOSE IS LEFT ALONE rather than written as a blank. A tag nobody +// stated is what the log already knows how to say nothing about, and a row +// reading `""` would be worse than one reading nothing. +func withPurpose(ctx context.Context, purpose callPurpose) context.Context { + if purpose == "" { + return ctx + } + return provider.WithCallTag(ctx, string(purpose)) +} + type unavailableCompleter struct{ err error } func (u unavailableCompleter) CompleteWithMessages(context.Context, []ai.Message, ...ai.Option) (*ai.Response, error) { diff --git a/internal/session/clientdoor_wire_test.go b/internal/session/clientdoor_wire_test.go index 8e7673363b..8f7c0731e2 100644 --- a/internal/session/clientdoor_wire_test.go +++ b/internal/session/clientdoor_wire_test.go @@ -445,7 +445,7 @@ func TestTheCrewNeverReachesAServiceThatDoesNotServeIt(t *testing.T) { t.Fatal(err) } t.Cleanup(func() { _ = child.Close() }) - if _, err := child.completeWithModel(t.Context(), []ai.Message{textMessage("user", "work")}, "crew/worker"); err != nil { + if _, err := child.completeWithModel(t.Context(), purposeTask, []ai.Message{textMessage("user", "work")}, "crew/worker"); err != nil { t.Fatal(err) } drainTurn(t, agent, "chat") diff --git a/internal/session/guardian.go b/internal/session/guardian.go index 08b26c2ba9..f18ea5e580 100644 --- a/internal/session/guardian.go +++ b/internal/session/guardian.go @@ -176,6 +176,7 @@ func (a *Agent) guardianAllows(ctx context.Context, hub *eventHub, call ai.ToolC // (internal/lane's roles.go). response, err := a.completeWithModel( provider.WithRole(provider.WithoutStream(judgeCtx), lane.RoleJudge), + callPurpose(roles.RoleGuardian), []ai.Message{ textMessage("system", guardianPrompt), textMessage("user", guardianQuestion(call, decision)), diff --git a/internal/session/harness_build.go b/internal/session/harness_build.go index 6977c43530..4ddc57e2b8 100644 --- a/internal/session/harness_build.go +++ b/internal/session/harness_build.go @@ -1033,6 +1033,7 @@ func (a *Agent) harnessComplete(ctx context.Context, messages []ai.Message, mode // rather than through its token stream (internal/lane's roles.go). response, err := a.completeWithModel( provider.WithRole(streamCtx, lane.RoleDesign), + callPurpose(roles.RoleDesigner), messages, model) close(done) diff --git a/internal/session/image.go b/internal/session/image.go index 56bbbe630f..c5deceb40a 100644 --- a/internal/session/image.go +++ b/internal/session/image.go @@ -490,7 +490,8 @@ func (a *Agent) runVision(ctx context.Context, hub *eventHub, live ai.Message, s // is reading, delta by delta, above. So it is talk, and it owns the clock // for as long as it is writing (internal/lane's roles.go). response, err := a.completeWithModel( - provider.WithRole(ctx, lane.RoleTalk), []ai.Message{live}, seer) + provider.WithRole(ctx, lane.RoleTalk), callPurpose(roles.RoleVision), + []ai.Message{live}, seer) if err == nil && response != nil { a.addAuxiliaryUsage(response, seer, 1) } diff --git a/internal/session/loop.go b/internal/session/loop.go index 0636f5ce1d..6f5e8c7f06 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -749,6 +749,14 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo // of a conversation on the strength of a conversation that already ended. meter := &checkpointMeter{} + // AND WHATEVER THE SESSION OWED THE PERSON AND HAD NO STREAM FOR. A reading + // that outlives its turn decides after the hub is closed — the post-turn + // memory pass, the route judge's ruling — and this is the first stream it + // gets (memory.go's [Agent.sayLate]). It is here rather than in the memory + // refresh, which is where it used to be: that refresh does not run at all on + // a session with memory off, so any other caller's line was held for ever. + a.sayHeldLines(hub) + // AND THE MARK'S READING RIDES BESIDE THE WORK (checkpoint.go's [markAside]). // It belongs to the turn for the meter's reason — it is a fact about ONE // answer — and it is let go of on every way out, including the ones that end @@ -861,6 +869,12 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo // closes. pace.sending(sentAt) response, answered, err := a.completeWithRetryReasoning(ctx, hub, model, rung, partial, reached, reasoning, warm, forming, frozenToolHistory) + // AND THE OTHER END OF THE SAME LAW. The request is back, so this is the + // last instant the model was writing — moved on every step, because the + // step that ends the turn is the one whose stamp the seal is measured + // from. It is taken here rather than at the ending itself so that no road + // out of the loop can forget it. + pace.answered(time.Now()) a.config.beat.ended() // THE MODEL THIS TURN IS ON CAN CHANGE UNDER IT. A step whose budget of // cut streams ran out moves to the next model in the chain and says so, @@ -1127,17 +1141,39 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo // request alone could not have told it. A second small model reads // what was asked and the shape of what came back, and a yes STARTS it as // a task and says so on the transcript (route_judge.go). It is silent - // when it cannot work, it is rate-limited to one start every few turns, - // and it is asked before the turn is sealed so that the work it starts is - // on the rail by the time the person reads the answer. - a.tellPhase(provider.PhaseChecking, "whether that should be work", time.Now()) - ruling, _ := judge.takeAtTheEnd() - a.endPhase() - // AND THE TURN IS WHAT SPENDS IT, here, past the two roads above that - // would have made it wrong: this turn answered in words, nothing is - // re-opening it and nothing is moving it, so work started now is work - // started on a turn that is really over. - a.applyRouteJudge(hub, ruling) + // when it cannot work, and it is rate-limited to one start every few turns. + // + // ── AND THE TURN NO LONGER WAITS FOR IT ───────────────────────────── + // + // IT USED TO HOLD THE SEAL. `judge.takeAtTheEnd()` stood on this line + // and blocked, between the model's last word and [Agent.sealTurn] — a + // cheap screen and, on a yes, a mastermind confirm, measured at up to + // half a minute after the answer was already fully written and on the + // person's screen. What they saw for the whole of it was a phase line + // reading "whether that should be work", and what was actually parked + // behind it was everything: the seal, the usage row, the next Submit, + // the follow-up drain. + // + // THE TURN'S JOB WAS NEVER TO KNOW WHETHER THE RULING HAD ARRIVED. It + // was to say WHEN one may be spent — which is HERE, past the two roads + // above that would have made it wrong: this turn answered in words, + // nothing is re-opening it and nothing is moving it, so work started + // on this ruling is work started on a turn that is really over. It says + // exactly that and walks away; the reading runs on the session's own + // lifetime and the ruling is spent the moment it lands + // (sidecar.go's [sidecar.spendWhenItLands], route_judge.go's + // [judgeAhead] for why this one reading outlives its turn). + // + // NOTHING IS LOST BY NOT WAITING. A yes starts a task, which appears on + // the rail and says so in one line whenever it arrives; there was never + // anything about it that had to land before the answer was sealed. + // + // THE LINE IT MAY SAY IS NOT SENT TO THIS HUB, and the nil is how it + // says so: a ruling that lands after the seal lands after this hub has + // closed, so the told-after line is put on whichever stream is live + // when it arrives ([Agent.applyRouteJudge]). + judge.spendWhenItLands(func(ruling judgeRuling) { a.applyRouteJudge(nil, ruling) }) + pace.sealing(time.Now()) hub.send(Event{Kind: EventTurnDone, Usage: a.sealTurn(turn, started, model)}) // The name comes after the turn is done and before the hub closes: // the person is not kept waiting on a title, and the event still has @@ -1315,6 +1351,17 @@ type turnPace struct { results time.Time worstGap time.Duration steps int + // lastWord is when the model stopped writing and sealed is when the turn was + // closed. THE GAP BETWEEN THEM IS THE THIRD FIGURE THIS FILE'S LAW IS ABOUT + // and the one that had no name: the front of a turn and the middle of it were + // measured, and the END was not — which is where the post-turn judge stood + // blocking for up to half a minute, in front of a person who was already + // reading their answer. + lastWord time.Time + sealed time.Time + // reask is when a recall that landed first cut the request in flight, so the + // row can say what the throw-away cost (memory.go's [recallAside.cutAt]). + reask time.Time } // sending stamps a request leaving, and closes whichever gap it ends: the @@ -1345,6 +1392,24 @@ func (p *turnPace) word(now time.Time) { } } +// answered stamps the instant the model stopped writing. It is moved on every +// step, because the last one to move it is the one that ended the turn. +func (p *turnPace) answered(now time.Time) { + p.mu.Lock() + defer p.mu.Unlock() + p.lastWord = now +} + +// sealing stamps the turn being closed, which is the moment the person gets +// their session back. +func (p *turnPace) sealing(now time.Time) { + p.mu.Lock() + defer p.mu.Unlock() + if p.sealed.IsZero() { + p.sealed = now + } +} + // resultsIn stamps the instant a tool batch's results are in hand, which is the // moment the NEXT request becomes owed. func (p *turnPace) resultsIn(now time.Time) { @@ -1353,6 +1418,14 @@ func (p *turnPace) resultsIn(now time.Time) { p.results = now } +// reasked stamps the moment a re-ask cut the first request. It is the caller's +// fact rather than this clock's, which is why it arrives rather than is taken. +func (p *turnPace) reasked(at time.Time) { + p.mu.Lock() + defer p.mu.Unlock() + p.reask = at +} + // row is the turn as the journal holds it, with the aside names the caller knows // and this clock does not. func (p *turnPace) row(aside []string) journalPace { @@ -1373,6 +1446,18 @@ func (p *turnPace) row(aside []string) journalPace { row.FirstWordMS = p.firstWord.Sub(p.firstSend).Milliseconds() } row.StepGapMS = p.worstGap.Milliseconds() + // THE END OF THE TURN, and it is written only when both ends of it are known: + // a turn that was stopped, handed over or broken never sealed, and a figure + // invented for one of those would be a measurement of something else (the + // emptiness law). + if !p.lastWord.IsZero() && !p.sealed.IsZero() { + row.SealMS = p.sealed.Sub(p.lastWord).Milliseconds() + } + // AND WHAT THE ONE RE-ASK THREW AWAY. The first send to the cut, which is + // the whole of what cutting costs over having assembled the block first. + if !p.reask.IsZero() { + row.ReaskMS = p.reask.Sub(p.firstSend).Milliseconds() + } return row } @@ -1391,6 +1476,13 @@ func (a *Agent) journalTurnPace(pace *turnPace, recall *recallAside, marked *mar // THE RE-ASK IS NAMED because it is the one move this law buys with a // second request, and an audit that could not see it could not price it. aside = append(aside, "recall:reasked") + // AND IT IS PRICED, which naming it alone never did. `SendMS` reads 1 + // on a re-asked turn because the first request really did leave in a + // millisecond — and was then thrown away, so the person waited the + // recall out anyway. The row could report the law kept on a turn that + // cost seven seconds; now it reports both (memory.go's + // [recallAside.cutAt]). + pace.reasked(recall.cutMoment()) } if recall.wasLate() { // AND SO IS THE LATENESS, which is the quality half: the block was @@ -1600,9 +1692,9 @@ func (a *Agent) sealTurn(turn Usage, started time.Time, model string) Usage { // // The reasoning level is stamped HERE, on the request path and nowhere else, so // it reaches every step and every retry of the turn and reaches nothing else: -// the compaction summary and the title call are the session's own errands, not -// the person's question, and a level they asked for their conversation to be -// thought about would be an odd thing to spend on naming it. +// the title call and the memory reflex are the session's own errands, not the +// person's question, and a level they asked for their conversation to be thought +// about would be an odd thing to spend on naming it. // // The stamp is [provider.WithConfiguredReasoningEffort] — the OPERATOR-explicit // setter — because this level is exactly that: a person turned a knob. The other @@ -1784,11 +1876,14 @@ func (a *Agent) completeWithRetryReasoning(ctx context.Context, hub *eventHub, m // turn and a task child's turn run the identical loop, and the one thing // that tells them apart is whether this agent IS a node — so the word // follows that, and the node it is gets named beside it. + // + // THE WORD GOES TO THE DOOR AND THE NODE STAYS HERE. A purpose is what a + // request is FOR and the door is the one place it is spelled onto a call + // (clientdoor.go); which node made it is a fact only this line knows. + purpose := purposeTurn if a.config.taskID != 0 { - attemptCtx = provider.WithCallNode(provider.WithCallTag(attemptCtx, "task"), - strconv.FormatUint(a.config.taskID, 10)) - } else { - attemptCtx = provider.WithCallTag(attemptCtx, "turn") + purpose = purposeTask + attemptCtx = provider.WithCallNode(attemptCtx, strconv.FormatUint(a.config.taskID, 10)) } messages, carried := a.snapshotWithReasoning() // OLD FROZEN TOOL RESULTS ARE ALREADY CONSUMED EVIDENCE. The live @@ -1804,7 +1899,7 @@ func (a *Agent) completeWithRetryReasoning(ctx context.Context, hub *eventHub, m func(message ai.Message) string { return a.fullResultPointer(message, place) }) attemptCtx = provider.WithMessageReasoning(attemptCtx, carried) attemptCtx, generation := a.beginGeneration(attemptCtx, reached) - response, err := a.completeWithModel(attemptCtx, messages, model, + response, err := a.completeWithModel(attemptCtx, purpose, messages, model, ai.WithTools(a.beltDefinitions())) cause := a.endGeneration(generation) if errors.Is(cause, errSteerCut) { diff --git a/internal/session/loop_speed_test.go b/internal/session/loop_speed_test.go index 73213329b7..05b273c1af 100644 --- a/internal/session/loop_speed_test.go +++ b/internal/session/loop_speed_test.go @@ -56,6 +56,17 @@ const paceLawBudget = lanes.SpokenWithin / 20 // budget above by a factor of sixty. const paceSlowReading = 3 * time.Second +// paceJudgeReading is what the POST-TURN JUDGE costs in this fixture. It is an +// order of magnitude over [paceSlowReading] so that a turn which waited for it +// cannot be mistaken for a turn that waited for the reader beside it — the one +// wait at the end of a turn that is honest. +const paceJudgeReading = 30 * time.Second + +// paceCutMargin is how long the recall takes in the re-ask fixture: long enough +// that the cut lands a measurable distance after the send, short enough that it +// still wins a race against a request that never answers at all. +const paceCutMargin = 25 * time.Millisecond + // ── the fixture ───────────────────────────────────────────────────────────── // paceCompleter answers the turn instantly and EVERY reading beside it slowly. @@ -71,6 +82,13 @@ type paceCompleter struct { taken int // slow is what every reading costs. slow time.Duration + // judgeSlow is what the POST-TURN JUDGE costs, when it differs. It is its own + // figure because the judge is the one reading at the end of a turn whose + // answer can be spent after the turn has sealed — the re-open reader beside + // it decides whether there IS a turn to seal, and has nothing to be applied + // to afterwards — so the only way to see the seal being held by the right one + // of the two is to make them cost different amounts. + judgeSlow time.Duration // route is what the memory router answers, and sketch what a mark's reader // draws. route string @@ -92,6 +110,9 @@ func (p *paceCompleter) CompleteWithMessages(ctx context.Context, messages []ai. } p.readings[kind]++ slow := p.slow + if kind == "judge" && p.judgeSlow > 0 { + slow = p.judgeSlow + } p.mu.Unlock() select { case <-time.After(slow): @@ -257,10 +278,15 @@ func TestTheOnlyWaitAPersonExperiencesIsTheModelGenerating(t *testing.T) { // pins is that the reading which DOES happen is never in front of the person. rounds := checkpointMarkAt(checkpointMarks) + 2 completer := &paceCompleter{ - steps: paceRounds(rounds, "all done"), - slow: paceSlowReading, - route: `{"inject":[],"cmd":null}`, - sketch: checkpointChainSketch, + steps: paceRounds(rounds, "all done"), + slow: paceSlowReading, + // AND THE POST-TURN JUDGE IS THE SLOWEST THING IN THE FIXTURE, which is + // what makes the seal assertion below say something. In the product it + // usually is: the re-open reader beside it is one call, and the judge is a + // cheap screen and then, on a yes, a mastermind confirm, in series. + judgeSlow: paceJudgeReading, + route: `{"inject":[],"cmd":null}`, + sketch: checkpointChainSketch, } agent, brain, journal := paceAgent(t, completer) remember(t, brain, "prefers tabs", "prefers tabs over spaces in Go") @@ -285,6 +311,29 @@ func TestTheOnlyWaitAPersonExperiencesIsTheModelGenerating(t *testing.T) { if floor := checkpointMarkAt(checkpointMarks); pace.Steps < floor { t.Fatalf("the turn took %d steps, want at least %d — the script did not run", pace.Steps, floor) } + // AND THE END OF THE TURN IS THE THIRD PLACE A READING CAN STAND IN FRONT OF + // A PERSON, which is where the post-turn judge stood: the answer is written, + // it is on their screen, and the turn is held open — the seal, the usage row, + // the next Submit and the follow-up drain all parked — while a cheap screen + // and a mastermind confirm decide whether it should have been work. + // + // It is the same budget as the two above because it is the same claim: what + // separates sealing a turn in memory from awaiting one model call is three + // orders of magnitude, so any figure inside that gap is the same test. + // The seal is held by the RE-OPEN READER and by nothing else. That one is + // [Agent.checkpointReopen], and it is the guardian's named exception read at + // the other end of the turn: it decides whether there is a turn to seal at + // all, so there is nothing left to apply its answer to afterwards and it has + // to precede. Every other reading at the end of a turn CAN be spent after the + // seal, and the post-turn judge is the one that had to be taught to be — so + // the fixture makes it far the slowest thing here and the seal must not have + // noticed. + if seal := time.Duration(pace.SealMS) * time.Millisecond; seal >= paceJudgeReading { + t.Fatalf("the turn stayed open %s after the model's last word, want well under the %s "+ + "the post-turn judge takes — a reading whose answer could have been spent after the "+ + "seal was awaited in front of it", + seal, paceJudgeReading) + } // AND THE READINGS REALLY WERE ASKED. A law that passed because nothing ran // beside the work would be the law deleted rather than kept, which is exactly @@ -331,8 +380,13 @@ func TestARecallThatLandsBeforeTheFirstWordIsAskedAgainWithIt(t *testing.T) { return textResponse("reformatted"), nil }, }, - // Instant: this reading is meant to win the race. - slow: 0, + // FAST ENOUGH TO WIN THE RACE AND SLOW ENOUGH TO BE MEASURED. The first + // step above blocks on its own context, so any figure at all wins; a + // reading that answered instantly would cut the request inside the same + // millisecond it was sent, and the row's own price for the throw-away + // would round to nothing. That is a property of the fixture and not of + // the move, so the fixture is what moves. + slow: paceCutMargin, } agent, brain, journal := paceAgent(t, completer) tabs := remember(t, brain, "prefers tabs", "prefers tabs over spaces in Go") @@ -352,6 +406,48 @@ func TestARecallThatLandsBeforeTheFirstWordIsAskedAgainWithIt(t *testing.T) { if !strings.Contains(completer.request(1), "prefers tabs over spaces in Go") { t.Fatalf("the re-asked request did not carry the routed memory:\n%s", completer.request(1)) } + // AND THE ROW SAYS WHAT IT COST, which naming it never did. + // + // `SendMS` reads a millisecond on a turn like this because the first request + // really did leave in a millisecond — and was then thrown away, so the person + // waited the recall out regardless. A row that reported only the send would + // report this file's own law kept on a turn where a reading cost the person + // every second of its own duration, which is how the move went unpriced for + // as long as it did (memory.go's [recallAside.cutAt]). + // + // The assertion is that the figure EXISTS on a re-asked turn and not that it + // is any particular size: what it measures is a real provider's silence, and + // a bound on it here would be a bound on the fixture. + if pace.ReaskMS <= 0 { + t.Fatalf("the row names a re-ask and does not price it (reaskMs %d) — "+ + "`recall:reasked` without the throw-away's life is a name nobody can weigh", + pace.ReaskMS) + } +} + +// AND A TURN THAT DID NOT RE-ASK IS PRICED AT NOTHING, which is the emptiness +// law on the same field: a figure written for a turn that threw nothing away +// would be a measurement of something that did not happen. +func TestATurnThatDidNotReAskPricesNoThrowAway(t *testing.T) { + completer := &paceCompleter{ + steps: []step{func(context.Context, []ai.Message) (*ai.Response, error) { + return textResponse("nothing to recall here"), nil + }}, + slow: paceSlowReading, + route: `{"inject":[],"cmd":null}`, + } + agent, brain, journal := paceAgent(t, completer) + remember(t, brain, "prefers tabs", "prefers tabs over spaces in Go") + + collect(t, mustSubmit(t, agent, "reformat this file the way I like it")) + + pace := pacedTurn(t, journal) + if namesAside(pace.Aside, "recall:reasked") { + t.Fatalf("the decomposition names %v, want no re-ask on a routing that injected nothing", pace.Aside) + } + if pace.ReaskMS != 0 { + t.Fatalf("a turn that threw nothing away priced a throw-away at %dms", pace.ReaskMS) + } } // A RECALL THAT LANDS AFTER THE FIRST WORD RIDES THE NEXT STEP, and says so. diff --git a/internal/session/memory.go b/internal/session/memory.go index ef83d2a51b..20e03c4558 100644 --- a/internal/session/memory.go +++ b/internal/session/memory.go @@ -113,11 +113,6 @@ type memoryBrain struct { // ([store.Store.RecordMemoryOutcome]). It is replaced per turn rather than // accumulated: what a turn retrieved belongs to that turn. injected []reflex.Stub - // said holds the dim lines this session owes the person and has had no - // stream to say them on. The post-turn pass writes memories after the turn - // is sealed and its hub is closed, so a supersession settled there has - // nowhere to land; the next turn's refresh flushes them. - said []string // imported records that the legacy memory.md has already been looked at. // The rename on disk is the durable answer; this is what keeps a session // from stat-ing the same absent file every turn. @@ -171,8 +166,8 @@ func (a *Agent) reflexClient() reflex.Completer { // billedCompleter is what makes a reflex call cost something a person can see. // // THE PERSON PAYS FOR IT, SO IT CANNOT BE FREE — the argument -// [Agent.addAuxiliaryUsage] states for the title and the compaction summary, -// and it bites harder here: this is the only auxiliary call made twice EVERY +// [Agent.addAuxiliaryUsage] states for the title call, and it bites harder +// here: this is the only auxiliary call made twice EVERY // turn, so a feature whose whole claim is that it is nearly free is exactly the // one that has to prove it on the bill. It is folded into the SESSION total and // never into a turn's, because no turn asked for it. @@ -432,14 +427,6 @@ func (a *Agent) refreshMemory(ctx context.Context, cue string) bool { // standing preferences lived in memory.md is answered out of them on the // very first turn after the upgrade rather than the second. a.importMemoryFile() - // AND WHATEVER THE LAST POST-TURN PASS HAD NOWHERE TO SAY. It writes after - // the turn is sealed and its hub closed, so a supersession settled there has - // no stream; this is the first one it gets, or, when this reading has - // outlived its own turn, the next. - for _, line := range a.memory.takeNotices() { - a.sayMemory(line) - } - // AND THERE IS NO PHASE WORD ON IT ANY MORE. `preparing saved context` was an // honest sentence about a wait the person really was serving — and it is not a // wait any more, so a status line that still said it would be this surface @@ -520,6 +507,39 @@ type recallAside struct { // late says the block landed behind the first token and is riding the next // step instead of this one. The turn's own decomposition row reads it. late atomic.Bool + // cutAt is when the re-ask cut the request in flight, in Unix nanoseconds, + // and zero where no re-ask happened. + // + // ── WHY THE MOMENT AND NOT THE FACT ───────────────────────────────────── + // + // [recallAside.reasked] already says a re-ask happened, and the decomposition + // row already names it. What neither of them can say is what it COST, and + // without that the move cannot be judged at all. + // + // The turn's own law asserts `message → the main request on the wire ≤ 50 ms`, + // and a re-asked turn passes it: the first request really does leave in a + // millisecond. It is then THROWN AWAY. On the one such turn the ledger holds + // (2026-09-12T03:06Z), SendMS was 1 and the person waited 7,389 ms for their + // first word, because the request that carried it did not leave until the + // recall landed. The instrument reported the law kept on a turn where a + // reading cost the person seven seconds — satisfied in the letter by sending + // a request nobody intended to read. + // + // So the row carries how long the discarded request was on the wire, which is + // exactly the difference between the two moves available here: cutting and + // re-asking, and simply assembling the block before the first request. Both + // reach the first word at the same instant — the recall's own duration plus + // one time-to-first-token — and the re-ask pays for two requests to get + // there. Which is right depends on whether the second request is cheaper for + // having warmed the prefix cache, and that is a question about a population + // of turns rather than about one. + // + // THE LEDGER CANNOT ANSWER IT YET, WHICH IS WHY THIS IS AN INSTRUMENT AND NOT + // A RULE. `journalPace` landed days ago: there are three pace rows on disk + // and one of them has a recall beside it. Choosing between the two moves on + // n=1 would be a threshold nobody could derive, which is the one thing the + // architecture bar refuses outright. + cutAt atomic.Int64 } // startRecallLocked launches the recall beside the title, from the one place a @@ -598,6 +618,7 @@ func (r *recallAside) applyOrDefer() { // keeps its one re-ask for a turn that actually needs one. if r.agent.cutGeneration(errRecallCut) { r.resent.Store(true) + r.cutAt.Store(time.Now().UnixNano()) } } @@ -611,6 +632,19 @@ func (r *recallAside) wasLate() bool { return r != nil && r.late.Load() } // reasked reports that this turn spent its one cut-and-re-ask on the block. func (r *recallAside) reasked() bool { return r != nil && r.resent.Load() } +// cutMoment is when the re-ask cut the request in flight, and the zero time +// where there was none. See [recallAside.cutAt] for what it is for. +func (r *recallAside) cutMoment() time.Time { + if r == nil { + return time.Time{} + } + nanos := r.cutAt.Load() + if nanos == 0 { + return time.Time{} + } + return time.Unix(0, nanos) +} + // end discards the recall, whether or not it has answered — [sidecar.end]. func (r *recallAside) end() { if r == nil { @@ -818,22 +852,6 @@ func (m *memoryBrain) takeInjected() []reflex.Stub { return stubs } -// queueNotice holds one dim line until there is somewhere to say it. -func (m *memoryBrain) queueNotice(text string) { - m.mu.Lock() - m.said = append(m.said, text) - m.mu.Unlock() -} - -// takeNotices drains the held lines. -func (m *memoryBrain) takeNotices() []string { - m.mu.Lock() - held := m.said - m.said = nil - m.mu.Unlock() - return held -} - // ── the post-turn pass ────────────────────────────────────────────────────── // learnFromTurn is what happens after the model has finished answering: the @@ -1066,13 +1084,58 @@ func (a *Agent) saySuperseded(oldTitle, newTitle string) { // explanation of why holding a line is rare, and no longer the reason the door // is correct. A hub send is an append and a signal, so the lock is held for // nothing longer than the steer drains already hold it. -func (a *Agent) sayMemory(text string) { +func (a *Agent) sayMemory(text string) { a.sayLate(text) } + +// ── A DIM LINE THAT MAY HAVE NO STREAM YET ────────────────────────────────── +// +// sayLate puts one dim line in front of the person: on the turn's own stream +// when there is one, and on the next turn's when there is not. +// +// THERE IS ONE MECHANISM FOR THIS SHAPE AND THIS IS IT. Everything in this +// package that decides something AFTER a turn has sealed owes the person a line +// and has nowhere to put it — the post-turn memory pass, which writes once the +// hub is closed, and now the route judge, whose ruling is spent whenever it +// lands (route_judge.go). It was the memory brain's own queue, which meant a +// session with memory off had no queue at all and the second caller would have +// quietly dropped its line. +// +// THE HUB ITSELF SAYS WHETHER THE LINE LANDED, and that is what makes "when +// there is one" true — not an ordering between this door and the roads that end +// turns. [eventHub.send] reports it from under the hub's own lock, which is the +// only place the answer is not already stale, and a nil hub answers false by the +// same door, so there is one road here and not two. +// +// The line is said under a.mu, which is how this reads the pointer at all — and +// because every road that ends a turn clears a.hub under this lock before +// closing the hub, the usual case is that a live hub takes it. A hub send is an +// append and a signal, so the lock is held for nothing longer than the steer +// drains already hold it. +func (a *Agent) sayLate(text string) { + if strings.TrimSpace(text) == "" { + return + } a.mu.Lock() defer a.mu.Unlock() if a.hub.send(Event{Kind: EventNotice, Text: text}) { return } - a.memory.queueNotice(text) + a.heldLines = append(a.heldLines, text) +} + +// sayHeldLines flushes whatever had no stream when it was said, onto the stream +// of the turn that is starting. +// +// IT IS THE TOP OF EVERY TURN and not the memory refresh, which is where the +// drain used to be: a session with memory off never ran that refresh, so the +// moment anything but the memory pass held a line, the line was held for ever. +func (a *Agent) sayHeldLines(hub *eventHub) { + a.mu.Lock() + held := a.heldLines + a.heldLines = nil + a.mu.Unlock() + for _, line := range held { + hub.send(Event{Kind: EventNotice, Text: line}) + } } // ── what a person and the model can ask for by hand ───────────────────────── diff --git a/internal/session/memory_consolidate.go b/internal/session/memory_consolidate.go index 3ac32a39c9..2b3947e4ec 100644 --- a/internal/session/memory_consolidate.go +++ b/internal/session/memory_consolidate.go @@ -352,8 +352,14 @@ func consolidateAsk(ctx context.Context, client Completer, model string, batch [ // a consolidation is the memory reflex's slow half, unattended and // judged on nothing but whether its answer is usable (internal/lane's // roles.go). + // And the purpose, which is what the call log will have to answer with + // when somebody asks what the machine spent the night on. This road + // builds its own client and never passes the door, so it says the word + // the door would have said (clientdoor.go's [withPurpose]). provider.WithRole( - provider.WithRoutingIntent(provider.WithoutStream(ctx), provider.IntentBackground), + provider.WithRoutingIntent( + provider.WithoutStream(withPurpose(ctx, callPurpose(roles.RoleConsolidate))), + provider.IntentBackground), lane.RoleMemory), []ai.Message{ textMessage("system", consolidatePrompt), diff --git a/internal/session/memory_test.go b/internal/session/memory_test.go index b17d93534d..131a91663a 100644 --- a/internal/session/memory_test.go +++ b/internal/session/memory_test.go @@ -1210,11 +1210,18 @@ func TestASupersessionSaysWhatItReplaced(t *testing.T) { // own test beside another suite, where the drain logged `ctxErr=context // canceled hubClosed=true` on every run that lost the line. A line taken with no // turn live now goes back on the queue, and the next turn says it. +// +// AND THE QUEUE IS THE SESSION'S RATHER THAN THE BRAIN'S, which is where it moved +// when the route judge became the second caller that decides after a turn has +// sealed: a session with memory off had no queue at all, so the second caller's +// line would have been dropped in silence (memory.go's [Agent.sayLate]). func TestAHeldLineOutlivesTheReadingThatTookIt(t *testing.T) { const line = "superseded · deploys on Fridays → deploys on Tuesdays" agent, _ := brainAgent(t, &reflexScript{route: `{"inject":[],"cmd":null}`}, nil) watchReadings(t, agent) - agent.memory.queueNotice(line) + // Said with no turn live, which is what a reading that outlived its own turn + // does: it goes onto the queue rather than onto a stream nobody is reading. + agent.sayLate(line) // The recall of a turn that has already ended, running now: no stream is live. agent.refreshMemory(context.Background(), "we moved deploys to Tuesday mornings") @@ -1224,6 +1231,34 @@ func TestAHeldLineOutlivesTheReadingThatTookIt(t *testing.T) { } } +// AND THE QUEUE IS NOT THE MEMORY BRAIN'S, which is the half that had no test +// because it had no second caller. A session with memory OFF still owes a line +// to whatever decided after its turn sealed — the route judge's told-after +// sentence is the one that made this real — and before the queue moved onto the +// session there was nowhere to put it. +func TestASessionWithNoMemoryStillHoldsALineItCouldNotSay(t *testing.T) { + const line = "this looked like work, so task 4 started: audit the pricing code" + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return textResponse("here is the answer"), nil + }, + }}, nil) + if agent.remembers() { + t.Fatal("this fixture is supposed to have no brain, so the queue under test is the session's") + } + agent.sayLate(line) + + var said string + for _, event := range collect(t, mustSubmit(t, agent, "what else is on today")) { + if event.Kind == EventNotice && event.Text == line { + said = event.Text + } + } + if said != line { + t.Fatalf("a session with no brain said %q, want the held line on the next turn", said) + } +} + // TestALineSaidOntoAClosedStreamIsHeldInstead is the property the held line // rests on, and the reason it is a property and not an ordering: a reading that // outlived its turn may still be holding the pointer to that turn's hub, and diff --git a/internal/session/nohiddenwork_test.go b/internal/session/nohiddenwork_test.go index ff47404dff..f9ebbf8d85 100644 --- a/internal/session/nohiddenwork_test.go +++ b/internal/session/nohiddenwork_test.go @@ -315,3 +315,237 @@ func TestTheSenseSaysWhichSenseItIsUsing(t *testing.T) { } } } + +// ── AND EVERY REQUEST SAYS WHAT IT IS FOR ─────────────────────────────────── +// +// THE STRUCTURAL HALF OF clientdoor.go's [callPurpose]. The type already makes +// an anonymous call fail to compile; what the type cannot see is a caller that +// satisfies it by passing the empty string, which is the same anonymous request +// wearing the argument. Nine of the eleven callers of this door reached the +// wire with no tag before the purpose was an argument — the guardian, vision, +// the shaper, the spell-out, the intake, the planner, the designer, the handoff +// draft and a saved program's step — and 2,309 of 2,839 untagged finishes in +// ten days were this package's. +// +// SO THE LAW IS: A PURPOSE IS A WORD, NOT A BLANK. One caller is exempt and it +// is named rather than inferred — [modelRoutingCompleter], which is the view of +// a live agent that LEAVES this package, whose callers each name their own call +// and whose tag this door must not overwrite. +// +// AND THE TAG IS SPELLED IN ONE PLACE. [provider.WithCallTag] may be called by +// the door and by nothing else in this package: three callers used to stamp +// their own, which is three spellings of one fact and the reason the other nine +// could forget it existed. +// +// IT READS THE TREE ITSELF, so it runs on the laws gate of every pull request +// (scripts/laws.sh finds it by this import). +func TestEveryRequestThroughTheDoorSaysWhatItIsFor(t *testing.T) { + files := sessionSources(t) + // theDoors are the two spellings of the one door. There is no exemption for a + // blank purpose any more and no apparatus to grant one: the single caller that + // keeps whatever the context already carries passes [purposeInherited], which + // says so in the place a reader is already looking. + theDoors := map[string]bool{"completeWithModel": true, "completeWithNamedModel": true} + purposes, tags := 0, 0 + // AND THE OTHER HALF OF THE SAME LAW. Three roads in this package build a + // [provider.Client] of their own — the memory tidy-up, a standing item's + // check, the document reader — because each needs a client shape the door + // does not make. They are allowed to; what they are not allowed to do is + // reach the wire anonymously, which all three did until #996. + // + // THE PURPOSE IS REQUIRED PER CALL AND NOT PER FILE. A file-level pairing is + // satisfied by one road naming itself while the road beside it stays + // anonymous, which is the shape of the failure this law exists for: the + // forgettable one is always the second one. + ownClients := map[string]bool{} + for name, file := range files { + ast.Inspect(file, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + switch selector.Sel.Name { + case "WithCallTag": + tags++ + if name != "clientdoor.go" { + t.Errorf("%s:%d calls provider.WithCallTag. The tag is spelled by withPurpose "+ + "and nowhere else — pass a callPurpose to completeWithModel instead "+ + "(clientdoor.go)", name, sessionLine(call)) + } + case "NewClient": + if name != "clientdoor.go" { + ownClients[name] = true + } + } + if !theDoors[selector.Sel.Name] || len(call.Args) < 2 { + return true + } + purposes++ + if blankPurpose(call.Args[1]) { + t.Errorf("%s:%d reaches the wire with a purpose spelled as a bare blank. Every "+ + "request this package makes says what it is FOR, and the one call that keeps "+ + "the caller's own word says THAT, by name: pass purposeInherited "+ + "(clientdoor.go's [callPurpose])", name, sessionLine(call)) + } + return true + }) + } + // THE TAG IS SPELLED EXACTLY ONCE. Not "only in this file" — once, full stop, + // because [withPurpose] is now the single road to it and a second spelling + // beside it inside clientdoor.go would be the same drift starting over in the + // one place the law was not looking. + if tags != 1 { + t.Errorf("provider.WithCallTag is written %d times in this package; it is written once, "+ + "by withPurpose, and every other road says what it is for by handing that function "+ + "a callPurpose (clientdoor.go)", tags) + } + // SECOND PASS, over the files that build their own client. It is a second + // pass because a file has to be known to be one of those roads before its + // completions can be judged, and `provider.NewClient` may be written below + // the call it serves. It reads the SAME parse, not a new one. + ownCalls := 0 + for name := range ownClients { + carries := contextsCarryingAPurpose(files[name]) + ast.Inspect(files[name], func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok || !ownClientCompletions[selector.Sel.Name] || len(call.Args) == 0 { + return true + } + ownCalls++ + if !purposeReaches(call.Args[0], carries) { + t.Errorf("%s:%d calls %s on a client this package built itself, with a context "+ + "that carries no purpose. A road that needs a client the one door does not "+ + "make is allowed one; a road that reaches the wire anonymously is not, "+ + "because the call log then cannot say what this build spent its night on. "+ + "Wrap the context in withPurpose (clientdoor.go)", + name, sessionLine(call), selector.Sel.Name) + } + return true + }) + } + // AND THE LAW IS READING THE TREE IT THINKS IT IS. A walk that matched + // nothing would pass for ever, which is how a structural law rots. + if purposes < 8 { + t.Fatalf("only %d calls through the one door were found; the law is reading the wrong tree", purposes) + } + if ownCalls < 3 { + t.Fatalf("only %d completions were found in the %d files that build their own client; "+ + "the law is reading the wrong tree", ownCalls, len(ownClients)) + } +} + +// ownClientCompletions are the verbs that reach the wire on a client this +// package built itself. They are [provider.Client]'s own methods, so the list is +// that type's surface and not a guess: a road that completes by some other verb +// tomorrow joins it, and the count check above is what makes a missing one show +// up as a law that suddenly reads less than it did. +var ownClientCompletions = map[string]bool{ + "CompleteWithMessages": true, + "ParseDocument": true, +} + +// contextsCarryingAPurpose names the local variables in a file that were built +// from a [withPurpose] call. +// +// A PURPOSE FOLDED INTO A CONTEXT STAYS IN IT. standing_run.go writes +// `callCtx := provider.WithRole(provider.WithRoutingIntent(provider.WithoutStream( +// withPurpose(ctx, …)), …), …)` and then completes on `callCtx` several lines +// later, which is the ordinary shape — the purpose is one of several facts +// wrapped onto the same context — and a reader that looked only at the argument +// of the completion would call that road anonymous. +func contextsCarryingAPurpose(file *ast.File) map[string]bool { + carries := map[string]bool{} + // To a fixed point, because one context is often built from another. + for again := true; again; { + again = false + ast.Inspect(file, func(node ast.Node) bool { + var names []ast.Expr + var values []ast.Expr + switch shape := node.(type) { + case *ast.AssignStmt: + names, values = shape.Lhs, shape.Rhs + case *ast.ValueSpec: + values = shape.Values + for _, name := range shape.Names { + names = append(names, name) + } + default: + return true + } + if len(names) != len(values) { + return true + } + for index, value := range values { + named, isIdent := names[index].(*ast.Ident) + if !isIdent || carries[named.Name] { + continue + } + if purposeReaches(value, carries) { + carries[named.Name] = true + again = true + } + } + return true + }) + } + return carries +} + +// purposeReaches reports whether a purpose is anywhere inside an expression — +// written there, or carried in by one of the contexts `carries` names. +func purposeReaches(expr ast.Expr, carries map[string]bool) bool { + found := false + ast.Inspect(expr, func(node ast.Node) bool { + if found { + return false + } + switch shape := node.(type) { + case *ast.CallExpr: + if named, isIdent := shape.Fun.(*ast.Ident); isIdent && named.Name == "withPurpose" { + found = true + } + case *ast.Ident: + if carries[shape.Name] { + found = true + } + } + return !found + }) + return found +} + +// blankPurpose reports an argument that names no purpose: the empty literal, or +// a conversion of one. +func blankPurpose(arg ast.Expr) bool { + switch node := arg.(type) { + case *ast.BasicLit: + return node.Kind == token.STRING && (node.Value == `""` || node.Value == "``") + case *ast.CallExpr: + // callPurpose("") is the same blank wearing its type. + return len(node.Args) == 1 && blankPurpose(node.Args[0]) + } + return false +} + +// receiverTypeName is the bare type a method hangs off, and "" for a function. +func receiverTypeName(decl *ast.FuncDecl) string { + if decl.Recv == nil || len(decl.Recv.List) == 0 { + return "" + } + expr := decl.Recv.List[0].Type + if star, ok := expr.(*ast.StarExpr); ok { + expr = star.X + } + if ident, ok := expr.(*ast.Ident); ok { + return ident.Name + } + return "" +} diff --git a/internal/session/orchestrate.go b/internal/session/orchestrate.go index f47e73a7b1..4ac832ebb6 100644 --- a/internal/session/orchestrate.go +++ b/internal/session/orchestrate.go @@ -710,6 +710,7 @@ func (p *orchestratePlanner) ask(ctx context.Context, messages []ai.Message) (st // The plan a run is steered by: nobody reads it arriving, and it has to // be right rather than soon (internal/lane's roles.go). provider.WithRole(provider.WithoutStream(ctx), lane.RoleDesign), + callPurpose(roles.RolePlanner), messages, p.call.model) if err != nil { diff --git a/internal/session/prefixbudget_test.go b/internal/session/prefixbudget_test.go index 450a2bf98f..a4634babe9 100644 --- a/internal/session/prefixbudget_test.go +++ b/internal/session/prefixbudget_test.go @@ -34,23 +34,89 @@ package session // is. import ( - "context" "encoding/json" "fmt" + "runtime" "sort" "strings" "testing" "time" - configpkg "github.com/Agent-Field/aforge-v2/internal/config" - "github.com/Agent-Field/aforge-v2/internal/subharness" + "github.com/Agent-Field/aforge-v2/internal/exec/bare" + "github.com/Agent-Field/agentfield/sdk/go/ai" ) // fixedPrefixBudget bounds the system prompt plus the marshalled tool block of // the belt the shipping conversation door assembles (belt_wiring_test.go's -// [v3ShapedAgent] is that shape). It is the post-diet measurement with about a -// tenth of headroom on top, which is room for a genuinely new law and not room -// for a paragraph of prose about one that is already stated. +// [shippedShapeAgent] is that shape). +// +// ── IT WEIGHED THE WRONG BELT FOR MONTHS, AND THIS IS THAT REPAIR ─────────── +// +// The sentence above has always said "the shipping conversation". The fixture +// under it was [v3ShapedAgent]: a conversation with NO memory store, NO accounts +// hub, NO standing items and NO saved programs — eighteen tools, 39,073 bytes, +// comfortably green. A machine somebody has finished setting up carries five +// more (`stand`, `search_conversations`, `remember`, and the subharness pair) +// and weighs 53,025. So the gate reported eight kilobytes of headroom on a +// prefix that was five kilobytes OVER the cap it was enforcing, and it reported +// it every day. That is #576's shape exactly: a gate pointed at something nobody +// runs passes without having tested anything. +// +// ── SO THE FIGURE IS A RATCHET NOW, AND NOT A BUDGET WITH ROOM IN IT ──────── +// +// It used to be "the post-diet measurement plus about a tenth", which was right +// when the number came out of a diet that had just been paid for: the headroom +// was room for a genuinely new law. It is wrong here, because this measurement +// is not the end of a diet — it is the first honest weighing of a prefix that +// has never been weighed, and it is already past what the last diet aimed at. A +// tenth of headroom on top of that would be five more kilobytes nobody chose. +// +// So it is the measurement and nothing else. Anything that grows the shipped +// prefix fails the build and has to be paid for out of what is already here, +// which is what a ratchet is for and what the old figure could not do. +// +// ── AND IT IS THE WIDEST MACHINE'S MEASUREMENT, NOT THIS ONE'S ────────────── +// +// The first spelling of this ratchet was 53,100 — the laptop it was written on +// weighing 53,025 — and it failed on the runner that proved it, at 53,132. The +// prefix is not the same size everywhere: `grep` says a longer sentence about +// itself where ripgrep is absent (134 bytes, and [widestBelt] now weighs that +// spelling wherever it runs), and `load_capability` lists the tool groups THIS +// BUILD has, which is 27 more bytes on a machine that can edit video than on one +// that cannot. A gate whose number depends on who runs it is a gate that passes +// where it is written and fails where it is proved, so the figure is the widest +// machine's and the swap that makes the biggest term machine-independent lives +// in [widestBelt]. +// +// The residual is the shelf, and it is owed rather than done: `load_capability` +// would have to name what this build COULD have rather than what it has, which +// is a change to what the model is told and belongs to whoever owns the shelf. +// It is bounded — one short clause per group — and it is why this number was +// measured on the machine that carries every group. It is filed as issue #1010, +// which is also where the `grep` sentence and [widestBelt] itself are deleted: +// removing the variance at its source is the only thing that makes this number +// one number for everybody. +// +// ── WHAT IS OWED, AND WHERE IT HAS TO COME FROM ───────────────────────────── +// +// THE TARGET IS STILL [fixedPrefixTarget] and the shipped prefix is over it by the +// difference between the two constants below. The bill +// is not spread thin — one tool is more than a quarter of the whole tool block: +// +// stand 9,607 the standing-item verb's schema +// propose_task 4,363 +// tasks 2,272 +// search_conversations 1,613 +// watch 1,396 +// +// `stand` alone is nearly twice the next heaviest and more than the whole overage. +// It is not this file's to cut: what a tool's schema says is its contract with +// the model, and trimming it is a change to what the model is told rather than +// to a byte count. It belongs to whoever owns internal/session's standing belt, +// with the same discipline the 2026-09-10 diet used on `propose_task` — one +// clause per field, no rule stated twice, no em dashes — and it is filed here +// rather than done here because a gate is not the place to decide what a verb +// means. // // It is a byte count and not a token count deliberately: bytes are what this // process can measure exactly, and every tokenizer this build talks to is within @@ -318,7 +384,45 @@ import ( // `THERE IS NO PLANNER ON YOUR BELT` paragraph — and both are pinned by // TestTheBeltRoutesWideWorkToOneWorkerAndNotToAPlanner, so paying it back is a // change to that test's mind and not only to the bytes. -const fixedPrefixBudget = 48_000 +// THE MEASUREMENT, AND NOTHING ON TOP OF IT. 53,141 bytes on 2026-09-12: the +// widest page at 19,114 and the fully-wired belt's tool block at 34,027 over +// twenty-three tools, weighed as the widest machine pays for it ([widestBelt]). +// There is no rounding in it and no headroom on it. +const fixedPrefixBudget = fixedPrefixTarget + fixedPrefixWaiver + +// fixedPrefixTarget is what the shipped prefix is SUPPOSED to be: the figure the +// last diet aimed at and the one [fixedPrefixBudget] held until the right belt +// was weighed. +// +// THE CAP IS THIS PLUS A WAIVER, AND THE WAIVER ONLY SHRINKS. See +// [prefixWaivers] for what each arm owes today and why a number that is only +// printed is a number nobody ever pays. +const fixedPrefixTarget = 48_000 + +// prefixWaivers is what each arm is over its target by, dated, in ONE PLACE. +// +// ── THE RULE, WHICH IS `.github/known-red.txt`'S RULE ──────────────────────── +// +// A waiver only ever SHRINKS. A lane that takes bytes out lowers the figure in +// the same commit; a lane that needs more takes it out of something that is +// already being said twice. Raising one is a decision with somebody's name on it +// in a diff, which is the entire difference between a debt and a floor. +// +// 2026-09-12, #996. The lean figure shrank from 13,489 to 13,420 in its own +// first commit, which is the discipline working rather than an edit: fixing the +// page's working directory and giving the lean arm its OWN shelf sentence took +// sixty-nine bytes off a number that had been measured on a hybrid. Both arms +// were weighing a conversation nobody has — no +// memory store, no accounts hub, no standing items, no saved programs — so both +// waivers are the first honest measurement of a prefix that had never been +// weighed, and neither is a wave's overspend. The bill is not spread thin: +// `stand` is 9,607 bytes, more than a quarter of the full tool block and, on a +// sixteen-thousand-token window, roughly one token in six of everything that +// person has before they have said anything. +const ( + fixedPrefixWaiver = 5_141 + leanPrefixWaiver = 13_416 +) // THE LEAN PROFILE GETS A BUDGET OF ITS OWN (2026-09-10, the prompt diet's lane // G). promptprofile.go added a second shape of prefix for a model with a small @@ -391,7 +495,39 @@ const fixedPrefixBudget = 48_000 // THEREAFTER IT ONLY EVER RATCHETS DOWN, in the ledger discipline the full // budget above is kept under: a lane that takes bytes out lowers it in the same // commit, and nothing ever raises it again. -const leanPrefixBudget = 31_500 +// +// ── AND THEN IT TURNED OUT TO BE WEIGHING A CONVERSATION NOBODY HAS ───────── +// +// Every figure above is real and every one of them was measured against +// [leanShapedAgent], which built a conversation with NO memory store, NO +// accounts hub, NO standing items and NO saved programs — sixteen tools. A +// person on a small window who has finished setting aforge up carries `stand` +// (9,607 bytes by itself), `remember` and `search_conversations`, none of which +// [Config.leanCapabilityGroups] shelves, and their prefix is 44,989 bytes. +// +// So the arm that exists to protect the person with the LEAST room to spare was +// out by 13,489 bytes, which is more than the whole budget it was enforcing. +// That is #576's shape and it is the same hole the full arm above had, found in +// the same review: a gate pointed at something nobody runs passes without having +// tested anything, every day, in both directions. +// +// THE FIGURE IS THE MEASUREMENT NOW, with no headroom — the argument for slack +// was written when this number came off a diet that had just been paid for, and +// a number that has never been honestly weighed has not earned any. What the +// slack bought (a shared page that can still move by a couple of hundred bytes) +// is now bought by the full arm's 53,141, which both arms read the page through. +// +// WHAT IS OWED IS THE SAME BILL THE FULL ARM OWES, and it falls harder here: +// `stand` alone is 9,607 bytes on a sixteen-thousand-token window, which is +// roughly one token in six of everything that person has, spent before they have +// said anything. [leanPrefixTarget] is what this has to come back to and the +// test prints the shortfall on every green run. +const leanPrefixBudget = leanPrefixTarget + leanPrefixWaiver + +// leanPrefixTarget is what the lean prefix is SUPPOSED to be: the figure +// [leanPrefixBudget] held until the right belt was weighed. The cap is this plus +// [leanPrefixWaiver], which only ever shrinks. +const leanPrefixTarget = 31_500 // leanWindow is the window the lean budget is weighed at. Sixteen thousand // tokens is the shape the profile was written for — a local open-weight model — @@ -433,35 +569,187 @@ func widestPage() string { return page } -// TestTheFixedPrefixStaysUnderItsBudget weighs what every request carries before -// anybody has said anything. -func TestTheFixedPrefixStaysUnderItsBudget(t *testing.T) { - agent := v3ShapedAgent(t) +// atAFixedPlace is a config whose WORKING DIRECTORY is a constant. +// +// THE PAGE INTERPOLATES WHERE YOU ARE (prompt.go's `- Working directory: %s`), +// and a fixture's workspace is a temp directory named after the test and the +// platform's temp root: `/tmp/TestX123/001` on Linux and +// `/var/folders/9k/…/T/TestX456/001` on macOS. So this arm's number moved by +// forty-five bytes between two machines and failed on the second — the same +// class of defect as `grep`'s two sentences and the capability shelf, found the +// same way, in the gate whose entire purpose is to be one number for everyone. +// +// The page also carries `- Workstation: %s/%s` from runtime.GOOS/GOARCH, which +// is a byte of difference between darwin and linux and is left alone: it is a +// FACT ABOUT THE MACHINE the model is told on purpose, it cannot be made +// constant without lying to the fixture, and a byte is inside nobody's decision. +// The working directory is not that — it is a fixture artefact, and it is tens +// of bytes. +// The full arm does not need it: [widestPage] weighs `systemPrompt` with its +// belt-fact tokens substituted and never renders the machine lines at all, so +// that number is already the same everywhere — and it is an UNDER-count of the +// shipped page by those few lines, which is recorded here rather than fixed +// because moving it would move a ratchet for a reason that is not growth. +func pageAsWeighed(config Config, now time.Time) int { + config.Workspace = "/w" + page := renderSystemAt(config, now) + // AND THE WORKSTATION LINE IS NORMALISED. `- Workstation: %s/%s` is + // runtime.GOOS/GOARCH, which is `darwin/arm64` on one machine and + // `linux/arm64` on another — ONE BYTE, and a ratchet with no headroom fails + // on one byte. It is a fact about the machine the model is told on purpose + // and cannot be made constant in the product, so it is made constant HERE, + // at the only place that needs it to be. + return len(strings.Replace(page, runtime.GOOS+"/"+runtime.GOARCH, "os/arch", 1)) +} - definitions := agent.beltDefinitions() +// widestBelt is the shipped belt weighed as the WIDEST MACHINE pays for it. +// +// A TOOL WHOSE DESCRIPTION DEPENDS ON WHAT IS INSTALLED still sends those bytes +// on every request, and this gate runs on machines of both kinds: `grep` says +// one sentence where ripgrep is present and a longer one where it is not +// (bare's [bare.WidestGrepDescription]), and the difference measured 107 bytes +// of prefix — enough that the same commit passed on the laptop it was written on +// and failed on the runner that proved it. So the number below is one number +// everywhere, and it is the larger one. +func widestBelt(t *testing.T, agent *Agent) []ai.ToolDefinition { + t.Helper() + definitions := append([]ai.ToolDefinition(nil), agent.beltDefinitions()...) if len(definitions) == 0 { t.Fatal("the belt is empty, so this test would pass on nothing") } - block, err := json.Marshal(definitions) + widest := map[string]string{ + // `grep` says a longer sentence about itself where ripgrep is absent. + "grep": bare.WidestGrepDescription(agent.resultCaps()), + // `load_capability` NAMES THE GROUPS THIS BUILD HAS, and a group whose + // every member was gated off is not named at all — `edit_video` is built + // only where ffmpeg is on PATH, so a machine that can edit video pays for + // one more clause than one that cannot (tools_capabilities.go). + // + // IT IS THIS SHAPE'S OWN SHELF AND NOT THE FULL ONE. A lean belt pre-arms + // `questions` instead of shelving it (promptprofile.go), so pasting the + // full shape's sentence into the lean arm would have made that number a + // hybrid of two belts — a figure neither of them pays. + "load_capability": widestLoadCapability(agent.config), + } + swapped := 0 + for index := range definitions { + longest, varies := widest[definitions[index].Function.Name] + if !varies { + continue + } + swapped++ + // ASSIGNED, NOT COMPARED. A swap that only fired when it made the number + // bigger would be a swap that silently stopped firing the day the other + // spelling grew — which is the same defeat, one level up. `widest` is + // widest by construction and this takes it at its word. + definitions[index].Function.Description = longest + } + // AND THE SWAP CANNOT SILENTLY STOP APPLYING. A tool that leaves the belt, or + // is renamed, would take its machine-variance off this number without anybody + // noticing the budget had quietly got easier. + if swapped != len(widest) { + t.Fatalf("%d of the %d tools that vary by machine are on the belt", swapped, len(widest)) + } + return definitions +} + +// widestLoadCapability is the loader's sentence for ONE SHAPE at the widest +// machine: every group that shape shelves rather than carries, each with every +// member the table declares — which is what it says where all the programs its +// tools shell out to are installed. +// +// It is built through the product's own [loadCapabilityDescription], so the +// three sentences after the group list are stated once and cannot drift out of +// this number. +// +// THE ONLY THING TAKEN OUT IS WHAT THIS SHAPE PRE-ARMS, because that is the one +// difference between the two belts' shelves that is a property of the SHAPE +// rather than of the machine. A group gated off by config would be +// over-counted here; that is the safe direction for a budget and it is the same +// answer everywhere, which is the whole point. +func widestLoadCapability(config Config) string { + prearmed := map[string]bool{} + for _, group := range config.prearmedGroups() { + prearmed[group] = true + } + order := make([]string, 0, len(capabilityGroups)) + members := make(map[string][]string, len(capabilityGroups)) + for _, group := range capabilityGroups { + if prearmed[group.name] { + continue + } + order = append(order, group.name) + members[group.name] = group.members + } + return loadCapabilityDescription(func(group string) []string { return members[group] }, order) +} + +// TestTheFixedPrefixStaysUnderItsBudget weighs what every request carries before +// anybody has said anything. +func TestTheFixedPrefixStaysUnderItsBudget(t *testing.T) { + reportPrefix(t, prefixArm{ + what: "the fixed prefix", + definitions: widestBelt(t, shippedShapeAgent(t)), + page: len(widestPage()), + budget: fixedPrefixBudget, + target: fixedPrefixTarget, + owed: "see this file's head for the bill and who owes it", + howToPay: "every byte here is sent again on every request of every turn, so take the " + + "addition back out of something that already says it rather than raising the budget", + }) +} + +// prefixArm is one weighing: what to call it, what it carries, and the two +// figures it is judged against. +type prefixArm struct { + what string + definitions []ai.ToolDefinition + page int + budget int + target int + owed string + howToPay string +} + +// reportPrefix weighs one arm, says what is still owed on a green run, and names +// the bill biggest-first when the ratchet is broken. +// +// IT IS ONE FUNCTION BECAUSE THE TWO ARMS ARE ONE TEST with two sets of +// constants. They were forty lines each, written twice, and the copies had +// already drifted: only one of them printed its shortfall against the target, +// so the arm carrying the larger debt was the arm that never mentioned it. +func reportPrefix(t *testing.T, arm prefixArm) { + t.Helper() + block, err := json.Marshal(arm.definitions) if err != nil { t.Fatal(err) } - tools, prompt := len(block), len(widestPage()) - total := tools + prompt - t.Logf("the fixed prefix is %d bytes (~%d tokens): prompt %d + tools %d", total, total/4, prompt, tools) - - if total <= fixedPrefixBudget { + tools := len(block) + total := tools + arm.page + t.Logf("%s is %d bytes (~%d tokens): prompt %d + tools %d over %d tools", + arm.what, total, total/4, arm.page, tools, len(arm.definitions)) + // AND THE DEBT IS ENFORCED, NOT LOGGED. It was a t.Logf, which is invisible: + // nothing in the Makefile passes -v, so the one line saying this prefix is + // thousands of bytes over what it is supposed to be was printed where no + // human and no CI log would ever show it. A debt nobody is reminded of is a + // debt that has quietly become the floor. + // + // So the TARGET is the cap, and the overage is a WAIVER: one dated figure per + // arm, in one place, which may only ever shrink ([prefixWaivers]). That is + // the known-red discipline — `.github/known-red.txt` and its ratchet — applied + // to bytes. A wave that grows the prefix has to raise a waiver, in a diff, with + // its name on it; nothing can drift. + if total <= arm.budget { return } - // THE FAILURE NAMES THE BILL, BIGGEST FIRST. A lane reading this has just // grown one of these lines and has no other way to see which. type weighed struct { name string bytes int } - heaviest := make([]weighed, 0, len(definitions)) - for _, definition := range definitions { + heaviest := make([]weighed, 0, len(arm.definitions)) + for _, definition := range arm.definitions { encoded, err := json.Marshal(definition) if err != nil { t.Fatal(err) @@ -469,42 +757,46 @@ func TestTheFixedPrefixStaysUnderItsBudget(t *testing.T) { heaviest = append(heaviest, weighed{definition.Function.Name, len(encoded)}) } sort.Slice(heaviest, func(i, j int) bool { return heaviest[i].bytes > heaviest[j].bytes }) - - report := fmt.Sprintf("the fixed prefix is %d bytes (~%d tokens), over its %d budget by %d\n"+ - " prompts/system.md %6d\n"+ + report := fmt.Sprintf("%s is %d bytes (~%d tokens), over its %d budget by %d\n"+ + " the page %6d\n"+ " the tool block %6d over %d tools\n"+ "the heaviest tools:\n", - total, total/4, fixedPrefixBudget, total-fixedPrefixBudget, prompt, tools, len(definitions)) + arm.what, total, total/4, arm.budget, total-arm.budget, arm.page, tools, len(arm.definitions)) for index, tool := range heaviest { if index == 8 { break } report += fmt.Sprintf(" %-20s%6d\n", tool.name, tool.bytes) } - t.Fatalf("%severy byte here is sent again on every request of every turn, so take the "+ - "addition back out of something that already says it rather than raising the budget", report) + t.Fatalf("%sthe cap is the %d target plus a waiver of %d, and a waiver only ever shrinks "+ + "([prefixWaivers]). %s", report, arm.target, arm.budget-arm.target, arm.howToPay) } // ── the lean arm ──────────────────────────────────────────────────────────── -// leanShapedAgent is the shipping conversation door on a small window: the same -// config [v3ShapedAgent] builds, with the window a local open-weight model -// actually has. Everything else about the shape is deliberately identical, so -// the difference between the two numbers below is the profile and nothing else. +// leanShapedAgent is the SHIPPING CONVERSATION on a small window: the shape +// [shippedShapeAgent] builds, with the one thing that differs — the window a +// local open-weight model actually has — changed and nothing else. The +// difference between the two numbers below is therefore the profile, and only +// the profile. +// +// ── IT WEIGHED A THINNER AGENT THAN ANYBODY RUNS, AND THIS IS THAT REPAIR ─── +// +// It used to build its own config: no memory store, no accounts hub, no standing +// items, no saved programs. That is #576's shape again and the same one the full +// arm was repaired for in this PR — a gate pointed at something nobody runs +// passes without having tested anything. A real lean conversation carries +// `stand` (9.6 KB on its own), `remember` and `search_conversations`, and +// [Config.leanCapabilityGroups] shelves none of the three, so every one of those +// bytes is bought on every request of every turn by exactly the person whose +// window has the least room for them. func leanShapedAgent(t *testing.T) *Agent { t.Helper() + shape := beltShapeNamed(t, shippedBeltShape) agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { config.System = "" + shape.build(t, config) config.ContextWindow = leanWindow - config.AskConsent = true - config.BashBackgroundAfterSeconds = configpkg.DefaultBashBackgroundAfter - config.HarnessStore = subharness.At(t.TempDir()) - config.RunHarness = func(context.Context, string, string, string, func(subharness.Trail)) (string, subharness.Usage, error) { - return "", subharness.Usage{}, nil - } - config.OrchestrateRunner = func(context.Context, string, string, float64) (string, error) { - return "", nil - } }) return agent } @@ -522,59 +814,23 @@ func TestTheLeanPrefixStaysUnderItsBudget(t *testing.T) { if !agent.config.promptProfile().lean() { t.Fatalf("a %d-token window did not resolve to the lean profile, so this test is weighing the wrong arm", leanWindow) } - - definitions := agent.beltDefinitions() - if len(definitions) == 0 { - t.Fatal("the belt is empty, so this test would pass on nothing") - } - block, err := json.Marshal(definitions) - if err != nil { - t.Fatal(err) - } // AND `ask` IS IN THE BLOCK, because it is pre-armed rather than shelved - // (promptprofile.go's [Config.prearmedGroups]). A lean belt that had to load - // its way to a question would be measured lighter here and be unable to ask - // one, which is the one regression this number could hide. + // ([Config.prearmedGroups]). A lean belt that had to load its way to a + // question would be measured lighter here and be unable to ask one, which is + // the one regression this number could hide. if !agent.hasTool("ask") { t.Fatal("`ask` is not carried on a lean belt: a one-call-per-message model cannot load-then-ask") } - prompt := len(renderSystemAt(agent.config, time.Date(2026, 9, 2, 10, 0, 0, 0, time.UTC))) - tools := len(block) - total := tools + prompt - t.Logf("the lean prefix is %d bytes (~%d tokens): prompt %d + tools %d over %d tools", - total, total/4, prompt, tools, len(definitions)) - - if total <= leanPrefixBudget { - return - } - heaviest := make([]struct { - name string - bytes int - }, 0, len(definitions)) - for _, definition := range definitions { - encoded, err := json.Marshal(definition) - if err != nil { - t.Fatal(err) - } - heaviest = append(heaviest, struct { - name string - bytes int - }{definition.Function.Name, len(encoded)}) - } - sort.Slice(heaviest, func(i, j int) bool { return heaviest[i].bytes > heaviest[j].bytes }) - report := fmt.Sprintf("the lean prefix is %d bytes (~%d tokens), over its %d budget by %d\n"+ - " the page %6d\n"+ - " the tool block %6d over %d tools\n"+ - "the heaviest tools:\n", - total, total/4, leanPrefixBudget, total-leanPrefixBudget, prompt, tools, len(definitions)) - for index, tool := range heaviest { - if index == 8 { - break - } - report += fmt.Sprintf(" %-20s%6d\n", tool.name, tool.bytes) - } - t.Fatalf("%sthis arm's number only ever comes down: shelve the verb, cut the law that is stated twice, "+ - "or leave it — never raise the budget", report) + reportPrefix(t, prefixArm{ + what: "the lean prefix", + definitions: widestBelt(t, agent), + page: pageAsWeighed(agent.config, time.Date(2026, 9, 2, 10, 0, 0, 0, time.UTC)), + budget: leanPrefixBudget, + target: leanPrefixTarget, + owed: "see [leanPrefixBudget] for the bill and who owes it", + howToPay: "this arm's number only ever comes down: shelve the verb, cut the law that is " + + "stated twice, or leave it — never raise the budget", + }) } // TestAFrontierShapeIsUntouchedByTheProfile is the other half of the deal. diff --git a/internal/session/prompt_belt_test.go b/internal/session/prompt_belt_test.go index 4b0762f590..e93ee7ce6e 100644 --- a/internal/session/prompt_belt_test.go +++ b/internal/session/prompt_belt_test.go @@ -62,6 +62,32 @@ type mintedShape struct { page string } +// buildShippedConversation is the shipping conversation, fully wired, and it is +// A NAMED FUNCTION so that more than one row of [beltShapes] can be it. The lean +// row is this shape with the window changed and nothing else; it used to build a +// bare config of its own, which meant two rows of one table disagreed about what +// "the shipping door" means and only one of them was a conversation anybody has. +func buildShippedConversation(t *testing.T, config *Config) { + config.Memory = openTestBrain(t) + config.connectHub = &fakeHub{connected: true, account: "you@example.test"} + config.HarnessStore = subharness.At(t.TempDir()) + config.RunHarness = func(context.Context, string, string, string, func(subharness.Trail)) (string, subharness.Usage, error) { + return "", subharness.Usage{}, nil + } + config.OrchestrateRunner = func(context.Context, string, string, float64) (string, error) { return "", nil } + config.BashBackgroundAfterSeconds = configpkg.DefaultBashBackgroundAfter + // AND THE TWO SEAMS THAT ARE THE REST OF THE UNIVERSE. The big machines + // and `stand` are conditional on somebody being there to answer a card + // and on there being a store to arm one in (tools.go), and a universe + // built without them would be a universe that could not tell a tool + // nobody named from a tool nobody has. + config.AskConsent = true + config.Standing = &Standing{} + config.standingItems = &fakeStanding{} + config.Subharnesses = registryWith(t, &fakeGeneralist{}, &fakeRunner{manifest: theProgram()}) + config.HarnessCards = true +} + // beltShapes is every shape, and each is built the way its own door builds it — // task_run.go for a node, standing_run.go for a check — so that a door that // changes what it hands down changes this test's answer too. @@ -69,28 +95,8 @@ var beltShapes = []beltShape{{ // The shipping conversation, fully wired: a store behind memory, an // accounts hub, the harness machines. This is the fullest belt there is and // it is what the universe of tool names is built from. - name: "a conversation that remembers", - build: func(t *testing.T, config *Config) { - config.Memory = openTestBrain(t) - config.connectHub = &fakeHub{connected: true, account: "you@example.test"} - config.HarnessStore = subharness.At(t.TempDir()) - config.RunHarness = func(context.Context, string, string, string, func(subharness.Trail)) (string, subharness.Usage, error) { - return "", subharness.Usage{}, nil - } - config.OrchestrateRunner = func(context.Context, string, string, float64) (string, error) { return "", nil } - config.BashBackgroundAfterSeconds = configpkg.DefaultBashBackgroundAfter - // AND THE TWO SEAMS THAT ARE THE REST OF THE UNIVERSE. The big machines - // and `stand` are conditional on somebody being there to answer a card - // and on there being a store to arm one in (tools.go), and a universe - // built without them would be a universe that could not tell a tool - // nobody named from a tool nobody has. - config.AskConsent = true - config.Standing = &Standing{} - config.standingItems = &fakeStanding{} - config.Subharnesses = registryWith(t, &fakeGeneralist{}, &fakeRunner{manifest: theProgram()}) - config.HarnessCards = true - }, -}, { + name: "a conversation that remembers", + build: buildShippedConversation}, { // The same door with memory off, which is what --once and a session opened // against no store get: no brain, so no `remember` and no // `search_conversations`. @@ -141,7 +147,15 @@ var beltShapes = []beltShape{{ // exactly what a second partition can break. name: "a lean conversation on a small window", build: func(t *testing.T, config *Config) { - config.ContextWindow = 16_000 + // IT IS THE SHIPPED SHAPE WITH THE WINDOW CHANGED, and one row of this + // table is not allowed to disagree with another about what "the shipping + // door" means. This built a bare config until #996 — no store, no hub, no + // standing items, no saved programs — while the gate that bounds the lean + // prefix built the full one, so the two were measuring different + // conversations under the same name and only one of them was a + // conversation anybody has. + buildShippedConversation(t, config) + config.ContextWindow = leanWindow }, }, { // A standing check's probe (standing_run.go): the parent's config with the diff --git a/internal/session/recovery_law_test.go b/internal/session/recovery_law_test.go index 2803768d9c..012c307f83 100644 --- a/internal/session/recovery_law_test.go +++ b/internal/session/recovery_law_test.go @@ -188,6 +188,14 @@ func TestEveryAllowedCountIsStillThere(t *testing.T) { } } +// sessionFset is the position table every law in this package reads through, so +// a law that wants to NAME A LINE — and a failure a lane has to go and look at +// always does — can, without parsing the tree a second time of its own. +var sessionFset = token.NewFileSet() + +// sessionLine is where a node is, for a failure message. +func sessionLine(node ast.Node) int { return sessionFset.Position(node.Pos()).Line } + // sessionSources parses every non-test file of this package, keyed by base name. func sessionSources(t *testing.T) map[string]*ast.File { t.Helper() @@ -195,14 +203,14 @@ func sessionSources(t *testing.T) map[string]*ast.File { if err != nil { t.Fatal(err) } - fset := token.NewFileSet() + fset := sessionFset files := map[string]*ast.File{} for _, entry := range entries { name := entry.Name() if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { continue } - parsed, err := parser.ParseFile(fset, filepath.Join(".", name), nil, parser.ParseComments) + parsed, err := parser.ParseFile(fset, filepath.Join(".", name), nil, parser.ParseComments|parser.SkipObjectResolution) if err != nil { t.Fatalf("parse %s: %v", name, err) } diff --git a/internal/session/repair_role.go b/internal/session/repair_role.go index 1a44ffcffe..77b4634e7d 100644 --- a/internal/session/repair_role.go +++ b/internal/session/repair_role.go @@ -51,25 +51,23 @@ import ( "github.com/Agent-Field/aforge-v2/internal/roles" ) -// roleRepair is the hands that fix what the cheap hands got wrong. +// The hands that fix what the cheap hands got wrong are [roles.RoleRepair] — +// declared there with every other role's word, which carries the reasoning for +// the tier — and REGISTERED HERE, where the call is. // -// It is DECLARED HERE rather than in internal/roles, which is the arrangement -// that package's own header asks for: the registry is open, and a package that -// adds an auxiliary call declares its role in the file that owns the call. The -// tier is HIGH and not the mastermind's, for [roles.RoleCareful]'s reason — a -// repair round is many turns of ordinary work done by a model that can be -// trusted with something subtle, not one answer that decides what every other -// call does. -const roleRepair = roles.Role("repair") - +// THE WORD AND THE REGISTRATION ARE DIFFERENT THINGS and this file used to +// conflate them. The registry is open: a package that adds an auxiliary call +// says so at the call, which is why the Register lives here and why the +// description is passed from here — internal/roles cannot write a line for a +// role it does not know about, and a settings row with a blank under its name is +// a call somebody is paying for and cannot read. But the WORD is vocabulary, and +// everything that asks what the roles are reads one file. Spelled here as +// `roles.Role("repair")` it was invisible to all of them. func init() { - // The description is passed here because internal/roles cannot write a line - // for a role it does not know about, and a settings row with a blank under - // its name is a call somebody is paying for and cannot read. - roles.Register(roleRepair, roles.TierHigh, "the second go at work a check found gaps in") + roles.Register(roles.RoleRepair, roles.TierHigh, "the second go at work a check found gaps in") } -// repairModel is which hands one repair round runs on: [roleRepair]'s answer, +// repairModel is which hands one repair round runs on: [roles.RoleRepair]'s answer, // with the model the node is already on as the floor. // // THE FLOOR IS THE WHOLE OF THE ALL-FLASH CASE. A failure to resolve is the same @@ -95,7 +93,7 @@ func (a *Agent) repairModel(node *TaskNode) string { // never an override. return floor } - model, err := roles.Resolve(roles.Source(source), roleRepair, floor) + model, err := roles.Resolve(roles.Source(source), roles.RoleRepair, floor) if err != nil || strings.TrimSpace(model) == "" { return floor } diff --git a/internal/session/route_checks_test.go b/internal/session/route_checks_test.go index 8c164bd4f4..b0e04a2775 100644 --- a/internal/session/route_checks_test.go +++ b/internal/session/route_checks_test.go @@ -32,6 +32,7 @@ func TestAutomaticTaskChecksReachTheRealChecker(t *testing.T) { agent, _, _ = routeAgent(t, completer) } collect(t, mustSubmit(t, agent, asked)) + routeSettled(t, agent) node := agent.graph().node(1) if node == nil { t.Fatal("no task was admitted") diff --git a/internal/session/route_judge.go b/internal/session/route_judge.go index 5ef69e6ad3..93cce412e5 100644 --- a/internal/session/route_judge.go +++ b/internal/session/route_judge.go @@ -482,6 +482,16 @@ func (a *Agent) applyRouteJudge(hub *eventHub, ruling judgeRuling) { ruling.release() return } + // A NIL HUB IS THE ORDINARY CASE ON THIS ROAD and it is not an absence. This + // ruling is spent when it LANDS, which is sometimes after the turn that bought + // it has sealed and closed its hub, so the told-after line goes through the + // door that says it on whichever stream is live and holds it for the next one + // otherwise ([launchRouteTask], memory.go's [Agent.sayLate]). + // + // The task itself needs none of that. It is on the rail the moment it is + // admitted, through the standing lane every node's news goes out on + // ([Agent.emitTaskUpdate]), which a turn ending has never closed. + // THE GAP IS SPENT BY A START AND BY NOTHING ELSE, which is why this line // stands here rather than beside the confirm. The gap is a person's // patience: it exists because work appearing over the top of a conversation @@ -612,6 +622,11 @@ func (a *Agent) confirmRouteWork(ctx context.Context, model, asked, answered str // really over, and lets it go where it has not ([judgeRace.end]). type judgeRace struct { reading *sidecar[judgeRuling] + // done releases this reading's place on the session's lifetime. It is called + // once, by whichever of [judgeRace.spendWhenItLands] or [judgeRace.end] + // finishes with the reading — never by both, because the reading is taken by + // exactly one of them. + done func() } // judgeAhead starts the post-turn READING. Everything gated — a node, a session @@ -620,24 +635,60 @@ type judgeRace struct { // // NOTHING UNDER HERE HAS AN EFFECT. What comes back is a [judgeRuling], and the // turn is what spends it ([Agent.applyRouteJudge]). +// +// ── AND IT DOES NOT RUN ON THE TURN'S CONTEXT ─────────────────────────────── +// +// This is the ONE reading in the package that is deliberately not tied to the +// work it was read for, and the reason is what its answer DOES. Every other +// reading beside a turn is applied to that turn — to this step if it lands in +// time, to the next one if it does not — so cancelling it with the turn loses +// nothing that had anywhere to go. A yes here starts a TASK, which is an effect +// that lands entirely outside the turn and is just as good a minute after it. +// +// Tied to the turn's context, the only way to have that answer at all was to +// hold the turn open until it arrived — and that is exactly what the loop did, +// past the last word of the answer, under a phase line reading "whether that +// should be work". So the reading runs on the session's own lifetime +// (sidecar.go's [afterTurn]) and the turn stops waiting for it. A session that +// is closing starts none, which is the nil handle again. func (a *Agent) judgeAhead(ctx context.Context, user userMessage, usedTools bool, answer string) *judgeRace { if usedTools || !a.config.AskConsent || a.config.InTask { return nil } - return &judgeRace{reading: readBeside(ctx, func(judgeCtx context.Context) judgeRuling { + lifetime, done, ok := a.after.begin() + if !ok { + return nil + } + return &judgeRace{done: done, reading: readBeside(lifetime, func(judgeCtx context.Context) judgeRuling { return a.readRouteJudge(judgeCtx, user, usedTools, answer) }, nil)} } -// takeAtTheEnd waits for the ruling. It is bounded by the two windows the calls -// under it carry and by the turn's own context, so it is not a wait that can -// outlast either — and it is a WAIT, which is only honest because the turn that -// calls it has already been told it is over (sidecar.go's own warning). -func (r *judgeRace) takeAtTheEnd() (judgeRuling, bool) { +// spendWhenItLands hands the ruling to `apply` the moment it arrives, and the +// turn walks away. +// +// IT REPLACES A WAIT AND THE NAME IS THE WHOLE DIFFERENCE. What stood here was +// `takeAtTheEnd`, which blocked the turn between the model's last word and the +// seal — measured at up to half a minute on an ordinary tool-less answer. The +// turn's job was never to know WHETHER the ruling had arrived; it was to say +// when the ruling may be SPENT, which is after the reader that could re-open the +// turn has said it will not. It still says exactly that, and it no longer waits +// to be told the answer is ready (sidecar.go's [sidecar.spendWhenItLands]). +func (r *judgeRace) spendWhenItLands(apply func(judgeRuling)) { if r == nil { - return judgeRuling{}, false + return + } + // AND THE PLACE ON THE SESSION'S LIFETIME IS RELEASED ON BOTH ROADS. The + // reading is taken by exactly one of this and [judgeRace.end], so exactly one + // of them calls `done` — but a sidecar that refuses to be taken would + // otherwise leave this reading counted for ever, and a closing session would + // wait out its whole grace for work that had already finished. + if !r.reading.spendWhenItLands(func(ruling judgeRuling) { + defer r.done() + apply(ruling) + }) { + r.done() } - return r.reading.takeAtTheEnd() } // end lets the reading go without waiting for it, for [routeRace.end]'s reason. @@ -655,6 +706,7 @@ func (r *judgeRace) end() { if ruling, ok := r.reading.take(); ok { ruling.release() } + r.done() } // routeWidth is what TWO READINGS OF ONE REQUEST come to on breadth: armed if @@ -1202,7 +1254,23 @@ func (a *Agent) launchRouteTask(hub *eventHub, verdict routeVerdict, title strin // the rest, on the rail. said := "this looked like work, so task " + strconv.FormatUint(id, 10) + " " + word + ": " + spec.title - hub.send(Event{Kind: EventNotice, Text: said}) + // AND IT IS SAID ON WHATEVER STREAM IS LIVE, OR HELD FOR THE NEXT ONE. + // + // This line opens by saying why work began that nobody asked for, so it is the + // one thing here a person cannot do without — and the ruling that starts this + // work is spent WHENEVER IT LANDS now, which is sometimes after the turn that + // bought it has sealed and closed its hub ([judgeRace.spendWhenItLands]). A + // send to that hub would be a line nobody reads, and a task appearing on the + // rail with no word about where it came from is exactly the surprise this + // sentence exists to prevent. The caller with a live hub of its own still + // passes one — the checkpoint road is inside its turn — and everything else + // goes through the door that holds a line until there is somewhere to say it + // (memory.go's [Agent.sayLate]). + if hub != nil { + hub.send(Event{Kind: EventNotice, Text: said}) + } else { + a.sayLate(said) + } return said, id } diff --git a/internal/session/route_judge_test.go b/internal/session/route_judge_test.go index 824cac0b30..c059a8b332 100644 --- a/internal/session/route_judge_test.go +++ b/internal/session/route_judge_test.go @@ -420,15 +420,78 @@ func noCard(t *testing.T, collected []Event) { // lines that stand above it — the ceiling's and the race's — both say "a task" // themselves, and a helper that answered with the reason instead of the start // would make every "nothing was started" assertion on this page read backwards. -func routeNotice(collected []Event) string { +// routeNotice is the told-after line WHEREVER IT LANDED, asked once and now. +// +// THE TURN DOES NOT WAIT FOR THE RULING ANY MORE (loop.go), so the line is on +// this turn's stream when the ruling beat the seal and on the session's held +// queue when it did not — and a reader that looked only at the stream would +// read a line that was said as a line that was never said (memory.go's +// [Agent.sayLate]). This is the reader for a NEGATIVE: nothing was said +// anywhere, right now. +func routeNotice(agent *Agent, collected []Event) string { for _, event := range collected { - if event.Kind == EventNotice && strings.HasPrefix(event.Text, "this looked like work, so task ") { + if event.Kind == EventNotice && routeToldAfter(event.Text) { return event.Text } } + return routeHeld(agent) +} + +// routeHeld is the told-after line waiting for the next turn's stream, or "". +func routeHeld(agent *Agent) string { + if agent == nil { + return "" + } + agent.mu.Lock() + defer agent.mu.Unlock() + for _, line := range agent.heldLines { + if routeToldAfter(line) { + return line + } + } return "" } +func routeToldAfter(text string) bool { + return strings.HasPrefix(text, "this looked like work, so task ") +} + +// routeSaid is [routeNotice] for a POSITIVE: it WAITS for the line, because a +// ruling that starts work is spent whenever it lands and "not yet" is not the +// same answer as "never". +func routeSaid(t *testing.T, agent *Agent, collected []Event) string { + t.Helper() + var line string + waitFor(t, "the told-after line the started work owes the person", func() bool { + line = routeNotice(agent, collected) + return line != "" + }) + return line +} + +// routeSettled waits for everything the turn set going BESIDE itself. +// +// THE TURN NO LONGER WAITS FOR THE ROUTE JUDGE (loop.go), so "was the judge +// asked", "did work start" and "was the person told" are claims about the +// SESSION and not about the instant the turn sealed. This waits for exactly what +// [Agent.Close] waits for — the after-turn lifetime's own running work +// (sidecar.go's [afterTurn]) — which is registered while the turn is still +// going, so there is no window where this returns before the reading it is +// waiting for has begun. +func routeSettled(t *testing.T, agent *Agent) { + t.Helper() + quiet := make(chan struct{}) + go func() { + defer close(quiet) + agent.after.running.Wait() + }() + select { + case <-quiet: + case <-time.After(10 * time.Second): + t.Fatal("the work the turn set going beside itself never finished") + } +} + const routeYes = `{"work": true, "goal": "audit every package's pricing code and report what is wrong", "why": "research across every package"}` // A WORDY TURN ABOUT REAL WORK: the judge is asked, the task STARTS, and the @@ -443,6 +506,7 @@ func TestTheJudgeStartsWorkAfterAToolLessTurn(t *testing.T) { t.Fatalf("submit: %v", err) } collected := collect(t, events) + routeSettled(t, agent) if completer.asked() != 1 { t.Fatalf("the judge was asked %d times", completer.asked()) @@ -474,9 +538,9 @@ func TestTheJudgeStartsWorkAfterAToolLessTurn(t *testing.T) { } // AND THEY ARE TOLD, in one line that says why work began that they did not // ask for. - notice := routeNotice(collected) + notice := routeSaid(t, agent, collected) if !strings.Contains(notice, "this looked like work") || !strings.Contains(notice, "task 1 started") { - t.Fatalf("the turn said %q about the work it started", notice) + t.Fatalf("the person was told %q about the work it started", notice) } if started := runs.started(); len(started) != 0 { t.Fatalf("the judge reached a planner: %v", started) @@ -583,6 +647,7 @@ func TestAJudgeThatCannotAnswerIsSilent(t *testing.T) { t.Fatalf("submit: %v", err) } collected := collect(t, events) + routeSettled(t, agent) if completer.asked() != 1 { t.Fatalf("the judge was asked %d times", completer.asked()) } @@ -604,26 +669,43 @@ func TestAJudgeThatCannotAnswerIsSilent(t *testing.T) { // a keypress between a judge that likes every turn and a rail full of work. func TestTheRateLimitHolds(t *testing.T) { completer := &routeCompleter{answer: "Here is what I would look at.", verdict: routeYes} - agent, _, _ := routeAgent(t, completer) + agent, _, nodes := routeAgent(t, completer) - starts := 0 + // The reading is what was ADMITTED, turn by turn, and it is neither the line + // that announces the work nor the run that carries it out. The told-after line + // rides whichever stream is live when the ruling lands ([routeNotice]), so + // counting lines would charge turn 1's announcement to turn 2 and read the gap + // as broken; the RUN starts on a goroutine of its own after admission, so + // counting runs reads the gap as broken whenever the runner is scheduled late. + // Admission is the judge's own act, inside the ruling this turn waited for + // ([routeSettled]), and it is the thing the gap is a rule about. + admitted := func() int { + count := 0 + for agent.graph().node(uint64(count)+1) != nil { + count++ + } + return count + } + started := 0 for turn := 1; turn <= 4; turn++ { events, err := agent.Submit(context.Background(), routeAsk) if err != nil { t.Fatalf("submit %d: %v", turn, err) } collected := collect(t, events) + routeSettled(t, agent) noCard(t, collected) - if routeNotice(collected) != "" { - starts++ + if admitted() > started { + started = admitted() if turn != 1 && turn != routeJudgeGap+1 { t.Fatalf("work started on turn %d, inside the gap", turn) } } } - if starts != 2 { - t.Fatalf("%d tasks over four turns, want one on turn 1 and one on turn %d", starts, routeJudgeGap+1) + if started != 2 { + t.Fatalf("%d tasks over four turns, want one on turn 1 and one on turn %d", started, routeJudgeGap+1) } + waitFor(t, "both tasks the judge started", func() bool { return nodes.count() == 2 }) } // THE JUDGE'S OWN WIDE VERDICT ARMS THE TASK IT STARTS. This was the one @@ -651,6 +733,7 @@ func TestTheJudgesWideVerdictArmsTheTaskItStarts(t *testing.T) { t.Fatalf("submit: %v", err) } collect(t, events) + routeSettled(t, agent) node := agent.graph().node(1) if node == nil { @@ -678,6 +761,7 @@ func TestARouteYesWithoutWidthArmsNothing(t *testing.T) { t.Fatalf("submit: %v", err) } collect(t, events) + routeSettled(t, agent) node := agent.graph().node(1) if node == nil { @@ -708,6 +792,7 @@ func TestTheJudgesDoneConditionIsWhatTheWorkIsFinishedAgainst(t *testing.T) { t.Fatalf("submit: %v", err) } collect(t, events) + routeSettled(t, agent) node := agent.graph().node(1) if node == nil { @@ -746,6 +831,7 @@ func TestAnAutoStartedTaskWithNoDoneConditionIsFinishedAgainstThePersonsWords(t t.Fatalf("submit: %v", err) } collect(t, events) + routeSettled(t, agent) node := agent.graph().node(1) if node == nil { @@ -856,6 +942,7 @@ func TestBothJudgesMustAgreeBeforeWorkStarts(t *testing.T) { agent, _, nodes := routeAgent(t, completer) collected := collect(t, mustSubmit(t, agent, routeAsk)) + routeSettled(t, agent) if completer.asked() != 1 || completer.confirms() != 1 { t.Fatalf("the screen was asked %d times and the confirm %d, want one each", @@ -872,8 +959,8 @@ func TestBothJudgesMustAgreeBeforeWorkStarts(t *testing.T) { if agent.graph().node(2) != nil { t.Fatal("one turn admitted two tasks") } - if notice := routeNotice(collected); !strings.Contains(notice, "task 1 started") { - t.Fatalf("the turn said %q about the work it started", notice) + if notice := routeSaid(t, agent, collected); !strings.Contains(notice, "task 1 started") { + t.Fatalf("the person was told %q about the work it started", notice) } } @@ -885,6 +972,7 @@ func TestAYesTheConfirmRefusesStartsNothingAndSaysNothing(t *testing.T) { agent, _, nodes := routeAgent(t, completer) collected := collect(t, mustSubmit(t, agent, routeAsk)) + routeSettled(t, agent) if completer.asked() != 1 || completer.confirms() != 1 { t.Fatalf("the screen was asked %d times and the confirm %d, want one each", @@ -894,7 +982,7 @@ func TestAYesTheConfirmRefusesStartsNothingAndSaysNothing(t *testing.T) { t.Fatal("a refused yes started work anyway") } noCard(t, collected) - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a refused yes said %q out loud", notice) } for _, event := range collected { @@ -922,6 +1010,7 @@ func TestAConfirmThatAnswersProseStartsNothing(t *testing.T) { agent, _, nodes := routeAgent(t, completer) collected := collect(t, mustSubmit(t, agent, routeAsk)) + routeSettled(t, agent) if completer.confirms() != 1 { t.Fatalf("the confirm was asked %d times, want once and never repaired", completer.confirms()) @@ -930,7 +1019,7 @@ func TestAConfirmThatAnswersProseStartsNothing(t *testing.T) { t.Fatal("a yes nobody could confirm started work") } noCard(t, collected) - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a yes nobody could confirm said %q", notice) } } @@ -943,6 +1032,7 @@ func TestTheConfirmIsNeverAskedAboutANo(t *testing.T) { agent, _, nodes := routeAgent(t, completer) collect(t, mustSubmit(t, agent, routeAsk)) + routeSettled(t, agent) if completer.asked() != 1 { t.Fatalf("the screen was asked %d times", completer.asked()) @@ -967,13 +1057,15 @@ func TestAConfirmedNoDoesNotSpendTheGap(t *testing.T) { completer := &routeCompleter{answer: "Here is what I would look at.", verdict: routeYes, confirm: routeConfirmNo} agent, _, nodes := routeAgent(t, completer) - if notice := routeNotice(collect(t, mustSubmit(t, agent, routeAsk))); notice != "" { + refused := collect(t, mustSubmit(t, agent, routeAsk)) + routeSettled(t, agent) + if notice := routeNotice(agent, refused); notice != "" { t.Fatalf("the refused turn said %q", notice) } completer.answerConfirmWith(routeYes) // THE VERY NEXT TURN, which is inside the gap a start would have opened. collected := collect(t, mustSubmit(t, agent, routeAsk)) - if notice := routeNotice(collected); !strings.Contains(notice, "task 1 started") { + if notice := routeSaid(t, agent, collected); !strings.Contains(notice, "task 1 started") { t.Fatalf("the turn after a refused yes said %q, want the work it agreed to", notice) } waitFor(t, "the task the second turn started", func() bool { return nodes.count() == 1 }) @@ -1125,7 +1217,7 @@ func TestARacedYesIsLookedAtEarlyAndTheLookConvertsTheTurn(t *testing.T) { if !saidSomething(said, checkpointCeilingNote) { t.Fatalf("the conversion never said its line; notices were %q", said) } - notice := routeNotice(collected) + notice := routeSaid(t, agent, collected) if !strings.Contains(notice, "this looked like work") || !strings.Contains(notice, "task 1 started") { t.Fatalf("the turn said %q about the work it started", notice) } @@ -1196,7 +1288,7 @@ func TestARacedYesOnlyMakesTheSidecarLookSooner(t *testing.T) { if said := noticeTexts(collected); saidSomething(said, checkpointCeilingNote) { t.Fatalf("a raced yes said something to the person: %q", said) } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a raced yes announced %q", notice) } // AND THE TURN RAN TO ITS OWN END, on its own answer. @@ -1369,7 +1461,7 @@ func TestAVerdictThatLandsAfterTheTurnIsDropped(t *testing.T) { if saidSomething(noticeTexts(collected), checkpointCeilingNote) { t.Fatalf("a dropped verdict said its line out loud: %q", noticeTexts(collected)) } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a dropped verdict said %q", notice) } // AND THE TURN ENDED AS ITS OWN ANSWER, which is the whole of what the person @@ -1408,7 +1500,7 @@ func TestAnInterruptDiscardsTheRacedYes(t *testing.T) { if saidSomething(noticeTexts(collected), checkpointCeilingNote) { t.Fatalf("an interrupted turn drew the conversion line: %q", noticeTexts(collected)) } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("an interrupted turn said %q", notice) } } @@ -1486,7 +1578,7 @@ func TestARacedYesIsDroppedWhenTheModelSaysNothingIsLeft(t *testing.T) { t.Fatalf("the person was told their answer was being moved and then watched it finish where "+ "it was; notices were %q", noticeTexts(collected)) } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a task was announced over a finished turn: %q", notice) } if nodes.count() != 0 || agent.graph().node(1) != nil { @@ -1520,10 +1612,7 @@ func TestAConvertedTaskIsNamedFromTheAskAndNotFromTheDowry(t *testing.T) { collected := collect(t, mustSubmit(t, agent, routeEnumerated)) waitFor(t, "the task the race converted the turn into", func() bool { return nodes.count() == 1 }) - notice := routeNotice(collected) - if notice == "" { - t.Fatalf("no task was announced; notices were %q", noticeTexts(collected)) - } + notice := routeSaid(t, agent, collected) if strings.Contains(notice, "DSML") { t.Fatalf("the sentinel became the task's name: %q", notice) } @@ -1633,10 +1722,14 @@ func TestTheRateLimitIsSharedByBothAsks(t *testing.T) { // TURN ONE starts work the POST-turn way: the pre-turn screen says no (its // default), the answer comes back in words, and the judge behind it starts a // task. - first := collect(t, mustSubmit(t, agent, routeAsk)) - if notice := routeNotice(first); !strings.Contains(notice, "task 1 started") { - t.Fatalf("the first turn said %q, want the work the post-turn read started", notice) - } + collect(t, mustSubmit(t, agent, routeAsk)) + // THE TASK IS THE FACT AND THE LINE IS THE FACT'S ANNOUNCEMENT, and they no + // longer land in the same breath. The turn seals when the answer is done and + // the ruling is spent whenever it lands (loop.go), so the start is waited on + // here and the told-after line rides whichever stream is live when it + // arrives — this turn's if it is still open, the next one's otherwise + // (memory.go's [Agent.sayLate]). Asserting it onto THIS turn's events would + // be asserting an ordering the turn no longer promises. waitFor(t, "the task the post-turn read started", func() bool { return nodes.count() == 1 }) // TURN TWO would be a yes at the FRONT of the turn — and it is never asked, @@ -1648,7 +1741,12 @@ func TestTheRateLimitIsSharedByBothAsks(t *testing.T) { if completer.preAsked() != before { t.Fatalf("the pre-turn screen was asked again inside the gap (%d then %d)", before, completer.preAsked()) } - if notice := routeNotice(second); notice != "" { + // A LINE ON THIS TURN IS NOT A START ON THIS TURN. The first turn's + // told-after sentence rides whichever stream is live when its ruling lands, + // and that is sometimes this one (memory.go's [Agent.sayLate]) — so what + // says nothing started here is the NUMBER in the line, and the graph beside + // it. + if notice := routeNotice(agent, second); strings.Contains(notice, "task 2") { t.Fatalf("a second task started inside the gap: %q", notice) } if agent.graph().node(2) != nil { @@ -1687,7 +1785,7 @@ func TestARacedYesTheConfirmRefusesLetsTheTurnRunToItsEnd(t *testing.T) { if nodes.count() != 0 || agent.graph().node(1) != nil { t.Fatal("a refused pre-turn yes started work anyway") } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a refused pre-turn yes said %q out loud", notice) } for _, event := range collected { @@ -1701,6 +1799,11 @@ func TestARacedYesTheConfirmRefusesLetsTheTurnRunToItsEnd(t *testing.T) { if completer.answered() != 1 { t.Fatalf("the conversation was asked %d times, want the ordinary turn", completer.answered()) } + // AND IT IS WAITED FOR RATHER THAN COUNTED ON THE SPOT. The turn does not + // hold its seal for the post-turn read any more (loop.go), so the read is + // still in flight when the turn's stream closes; what this test is about is + // that the read HAPPENS, which is a fact to wait on. + waitFor(t, "the post-turn judge's one look", func() bool { return completer.asked() == 1 }) if completer.asked() != 1 { t.Fatalf("the post-turn judge was asked %d times, want its usual one look", completer.asked()) } @@ -1867,7 +1970,7 @@ func TestASlowScreenNeverDelaysTheTurnsFirstRequest(t *testing.T) { if answers != 1 { t.Fatalf("the conversation was asked %d times, want the ordinary turn", answers) } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a judge that never answered said %q", notice) } for _, event := range collected { @@ -1922,10 +2025,15 @@ func TestAJudgesRulingIsSpentByTheTurnAndNotByTheReading(t *testing.T) { agent, _, nodes := routeAgent(t, completer) asked := userMessage{message: textMessage("user", routeAsk)} - // A READING THAT SAYS YES, taken in full. + // A READING THAT SAYS YES, taken in full. The turn does not WAIT for it — it + // says when the ruling may be spent and walks away — so the test collects it + // through the same door and then waits on the fact, which is the only thing + // it may wait on ([judgeRace.spendWhenItLands]). judge := agent.judgeAhead(context.Background(), asked, false, "Here is what I would look at.") - ruling, ok := judge.takeAtTheEnd() - if !ok || !ruling.start { + rulings := make(chan judgeRuling, 1) + judge.spendWhenItLands(func(ruling judgeRuling) { rulings <- ruling }) + ruling := <-rulings + if !ruling.start { t.Fatalf("the judge answered %+v, want a ruling that asks for a start", ruling) } if completer.asked() == 0 || completer.confirms() == 0 { @@ -1942,8 +2050,12 @@ func TestAJudgesRulingIsSpentByTheTurnAndNotByTheReading(t *testing.T) { } // A TURN THAT RE-OPENS LETS THE RULING GO, and lets go of the name it was - // holding with it. - judge.end() + // holding with it. This one was taken above, so letting it go is the ordinary + // no-op every already-spent reading gets; the road that matters is the one + // below it, where a reading is let go of having never been taken. + ruling.release() + dropped := agent.judgeAhead(context.Background(), asked, false, "Here is what I would look at.") + dropped.end() if count := nodes.count(); count != 0 { t.Fatalf("%d tasks survived a turn that let the ruling go", count) } @@ -1953,8 +2065,10 @@ func TestAJudgesRulingIsSpentByTheTurnAndNotByTheReading(t *testing.T) { // not lose it — that is the other half of loop.go's law. hub := newEventHub() second := agent.judgeAhead(context.Background(), asked, false, "Here is what I would look at.") - landed, ok := second.takeAtTheEnd() - if !ok || !landed.start { + seconds := make(chan judgeRuling, 1) + second.spendWhenItLands(func(ruling judgeRuling) { seconds <- ruling }) + landed := <-seconds + if !landed.start { t.Fatalf("the second reading answered %+v, want a ruling that asks for a start", landed) } agent.applyRouteJudge(hub, landed) diff --git a/internal/session/session.go b/internal/session/session.go index 794e88ebd4..c5432a8b4c 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -3057,6 +3057,18 @@ type Agent struct { titleCtx context.Context titleStop context.CancelFunc titleJobs sync.WaitGroup + + // after is the lifetime of every reading a turn bought that OUTLIVES it, and + // today that is the post-turn judge alone (sidecar.go's [afterTurn] states + // the bargain and names the two older spellings of it above). It is minted + // for every session, because the first turn may buy one. + after *afterTurn + + // heldLines are the dim lines this session owes the person and has had no + // stream to say them on (memory.go's [Agent.sayLate]). A reading that + // outlives its turn — the post-turn memory pass, the route judge's ruling — + // decides after the hub is closed, and the next turn is where the line lands. + heldLines []string // titleWatchers is the standing subscription to the name this session gives // itself, and it exists because THE NAME NOW ARRIVES AFTER THE TURN THAT // BOUGHT IT MAY HAVE ENDED. It is [Agent.harnessWatchers]' shape exactly diff --git a/internal/session/sessionfile.go b/internal/session/sessionfile.go index 0a22878dfc..45bc5a3495 100644 --- a/internal/session/sessionfile.go +++ b/internal/session/sessionfile.go @@ -891,11 +891,24 @@ type journalCaption struct { // from a fast turn that asked several things concurrently — the difference // between the law being kept and the readings having been deleted. type journalPace struct { - SendMS int64 `json:"sendMs"` - FirstWordMS int64 `json:"firstWordMs,omitempty"` - StepGapMS int64 `json:"stepGapMs,omitempty"` - Steps int `json:"steps,omitempty"` - Aside []string `json:"aside,omitempty"` + SendMS int64 `json:"sendMs"` + FirstWordMS int64 `json:"firstWordMs,omitempty"` + StepGapMS int64 `json:"stepGapMs,omitempty"` + // SealMS is the model's last word to the turn being sealed — the END of the + // turn said as a number, which is the third place a reading can stand in + // front of a person and the one this row could not see. The post-turn judge + // stood there and held it for as long as a cheap screen and a mastermind + // confirm took, with the answer already fully written on the screen. + SealMS int64 `json:"sealMs,omitempty"` + // ReaskMS is how long the request the recall THREW AWAY was on the wire: the + // first send to the cut. It is the price of the one move this law buys with a + // second request, and without it `recall:reasked` is a name for something + // nobody can weigh — the row's own SendMS says 1 on a turn where the person + // waited seven seconds (memory.go's [recallAside.cutAt] states the whole of + // why). Absent from every turn that did not re-ask. + ReaskMS int64 `json:"reaskMs,omitempty"` + Steps int `json:"steps,omitempty"` + Aside []string `json:"aside,omitempty"` } type journalTook struct { diff --git a/internal/session/sidecar.go b/internal/session/sidecar.go index c7d8bb1041..eb179b02d7 100644 --- a/internal/session/sidecar.go +++ b/internal/session/sidecar.go @@ -64,6 +64,7 @@ import ( "context" "sync" "sync/atomic" + "time" "github.com/Agent-Field/aforge-v2/internal/guard" ) @@ -159,6 +160,104 @@ func readBeside[T any](ctx context.Context, ask func(context.Context) T, act fun return side } +// ── AND THE LIFETIME A READING GETS WHEN THE TURN IS NOT ONE ──────────────── +// +// [readBeside] ties a reading to the WORK'S context, which is right for every +// reading that has work to run beside: a turn that ends has nothing left for its +// recall or its mark to be applied to, so cancelling them with it is the honest +// thing. +// +// THE END-OF-TURN READINGS ARE THE EXCEPTION AND THEY ARE WHY THIS EXISTS. The +// post-turn judge is asked whether the answer should have been WORK, and a yes +// starts a task — an effect that is about the turn and lands entirely outside it. +// Tied to the turn's context it could only be had by holding the turn open until +// it answered, which is the wait this whole file exists to remove. Tied to the +// SESSION it is what it always was: a reading nobody is waiting for, whose answer +// is spent when it arrives ([sidecar.spendWhenItLands]). +// +// THE BARGAIN IS THE [jobRegistry]'S, IN MINIATURE: the work is tracked, so a +// session that is closing can wait for it, and it is cancelled, so the wait is +// short. A session that has closed starts nothing, and that test is taken under +// this type's own lock so that "closed, therefore no new work" is one atomic +// fact rather than two. +// +// TWO OLDER SPELLINGS OF THIS SHAPE ARE STILL IN THE PACKAGE and are named here +// rather than left to be rediscovered: the namer's (session.go's `titleCtx`, +// `titleStop`, `titleJobs`) and the memory pass's (`memoryCtx`, `memoryStop`, +// `memoryJobs`). They are this type written out by hand, twice. They are not +// folded in here because their shutdowns genuinely differ — the namer is +// cancelled and then joined, the memory pass is waited for and THEN cancelled, +// because a pass two seconds from keeping something a person said is worth two +// seconds of a quit and a half-written name is not — and a fold that flattened +// that difference would be this type deciding something neither of them asked it +// to. Folding them wants that difference carried as a named property. +type afterTurn struct { + mu sync.Mutex + ctx context.Context + stop context.CancelFunc + closed bool + running sync.WaitGroup +} + +// newAfterTurn mints the lifetime. It is called once per session, at +// construction, because work bought by the FIRST turn has to have somewhere to +// be cancelled from. +func newAfterTurn() *afterTurn { + ctx, stop := context.WithCancel(context.Background()) + return &afterTurn{ctx: ctx, stop: stop} +} + +// begin registers one piece of work and answers the context it runs under. It +// answers false once the session is closing, which is the caller's whole +// instruction: do not start. +// +// THE CALLER OWNS THE `done` IT IS HANDED and must call it on every way out, or +// a closing session waits the whole grace for work that has already finished. +func (t *afterTurn) begin() (context.Context, func(), bool) { + if t == nil { + return nil, func() {}, false + } + t.mu.Lock() + defer t.mu.Unlock() + if t.closed { + return nil, func() {}, false + } + t.running.Add(1) + return t.ctx, t.running.Done, true +} + +// settle ends the lifetime: nothing new starts, whatever is in flight is +// cancelled, and the caller waits a bounded moment for it to unwind. +// +// IT CANCELS BEFORE IT WAITS, which is the namer's order rather than the memory +// pass's, and for the namer's reason: nothing under here owes a store a write. +// What the judge owes is a decision, and a decision nobody is going to read is +// worth none of a person's quit. +func (t *afterTurn) settle(grace time.Duration) { + if t == nil { + return + } + t.mu.Lock() + already := t.closed + t.closed = true + t.mu.Unlock() + if already { + return + } + t.stop() + settled := make(chan struct{}) + go func() { + t.running.Wait() + close(settled) + }() + timer := time.NewTimer(grace) + defer timer.Stop() + select { + case <-settled: + case <-timer.C: + } +} + // ── AND HOW A LANDING IS SEEN WITHOUT RACING IT ───────────────────────────── // // A reading lands whenever the scheduler lets it, and nothing in this package @@ -307,6 +406,67 @@ func (s *sidecar[T]) takeAtTheEnd() (T, bool) { return s.answer, true } +// spendWhenItLands hands the answer to `spend` AT THE MOMENT IT LANDS, and the +// caller does not wait for it. +// +// IT IS THE FOURTH VERB AND IT IS WHAT THE END OF A TURN ACTUALLY NEEDS. +// [sidecar.takeAtTheEnd] above is a WAIT, and the only thing that ever made it +// honest was that the caller had already been told the turn was over — which is +// true of the caller and NOT of the person, who is sitting in front of a +// finished answer while a judge decides something about it. A turn that waited +// there kept the seal, the transcript, the next Submit and the follow-up drain +// all parked behind a reading nobody asked for: measured at up to half a minute +// after the last word was written, under a phase line reading "whether that +// should be work". +// +// So the turn says WHEN the answer may be spent, and stops saying WHETHER it has +// arrived. `spend` runs on the reading's own goroutine the instant it lands, and +// on the CALLER'S goroutine when it has landed already — which is the ordinary +// fast case and costs exactly what taking it did. +// +// THE CALLER MUST HAVE DECIDED THE ANSWER IS SPENDABLE, exactly as [sidecar.take] +// requires: this is TAKE with the waiting removed, not a licence to act earlier. +// Everything [sidecar.take] does for the one-answer rule is done here — the +// sidecar is marked taken at once, so a second caller gets nothing, and the +// interruption is claimed so a cut nobody needs is not raised. +// +// AND THE READING MUST OUTLIVE THE TURN FOR THIS TO MEAN ANYTHING. A sidecar +// started on the turn's own context is cancelled the moment the turn returns, so +// arming this on one would be a promise to spend an answer that is about to be +// cancelled. A caller that uses this verb starts its reading on the session's own +// lifetime instead ([Agent.afterTurn]). +// It reports whether it TOOK the reading, which is the one-answer rule read from +// the caller's side: a sidecar that was already spent hands `spend` nothing, and +// a caller holding something on the reading's behalf — a place on a lifetime, a +// name being made — has to be told so it can let go of it. +func (s *sidecar[T]) spendWhenItLands(spend func(T)) bool { + if s == nil || s.taken || spend == nil { + return false + } + s.taken = true + select { + case <-s.settled: + // It is already here, so this is [sidecar.take] with the select already + // answered, and it runs on the caller's goroutine exactly as that did. + s.claim() + s.stop() + spend(s.answer) + default: + // AND THE HANDOFF IS ITS OWN GOROUTINE because the whole point is that + // this caller walks away. It is bounded by the reading's own windows and + // by the lifetime the reading was started on, the same two bounds + // [sidecar.takeAtTheEnd] was bounded by — the difference is only who is + // standing in front of them. + guard.Go("reading spent after the turn", func() { + <-s.settled + s.claim() + s.stop() + spend(s.answer) + }) + } + return true +} + // pending reports that a reading is in flight or landed and not yet spent. It is // what a caller asks before starting a second one: two readers shown the same // question a step apart answer it twice and the turn pays for both. diff --git a/internal/session/spawnfloor_test.go b/internal/session/spawnfloor_test.go index c1a1fb8a7e..3762d5a450 100644 --- a/internal/session/spawnfloor_test.go +++ b/internal/session/spawnfloor_test.go @@ -44,8 +44,12 @@ func TestAGenuineMultiPartAskCanStillBecomeATask(t *testing.T) { if agent.graph().node(1) == nil { t.Fatal("a genuine multi-part ask started nothing") } - if notice := routeNotice(collected); !strings.Contains(notice, "task 1 started") { - t.Fatalf("the turn said %q, want the work it started", notice) + // AND THE TOLD-AFTER LINE IS SAID. The turn does not wait for the ruling any + // more, so the line is not promised to THIS turn's events — but it is promised + // to the person, which is the claim worth holding ([routeSaid]). + notice := routeSaid(t, agent, collected) + if !strings.Contains(notice, "task 1 started") { + t.Fatalf("the person was told %q, want the work that started", notice) } } @@ -133,7 +137,7 @@ func assertAskNeverSpawns(t *testing.T, asked string) { if output := toolOutput(t, collected, "propose_task"); !strings.Contains(output, spawnFloorRefusal) { t.Fatalf("propose_task answered %q, want the floor's refusal", output) } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("a task was announced for %q: %q", asked, notice) } }) @@ -152,7 +156,7 @@ func assertAskNeverSpawns(t *testing.T, asked string) { if nodes.count() != 0 || agent.graph().node(1) != nil { t.Fatalf("the route judge started work on %q", asked) } - if notice := routeNotice(collected); notice != "" { + if notice := routeNotice(agent, collected); notice != "" { t.Fatalf("the route judge announced %q for %q", notice, asked) } }) diff --git a/internal/session/spellout.go b/internal/session/spellout.go index bd1c1b7706..3f2a6557c9 100644 --- a/internal/session/spellout.go +++ b/internal/session/spellout.go @@ -50,20 +50,13 @@ import ( "github.com/Agent-Field/agentfield/sdk/go/ai" ) -// spellOutRole is the expansion, registered as a ROLE so it resolves the way -// every other auxiliary call in this build resolves: the person's pin, then the -// tier, then the conversation's own model (internal/roles). -// -// IT SITS LOW, with the namer and the sentinel rather than with the shaper. The -// shaper writes the only document an autonomous worker will ever read, so a -// vague answer there costs a whole task's spend; this one writes three lines a -// person reads on screen before deciding whether to keep them, and a weak answer -// costs one esc. A person who wants it thought about harder pins it -// (`roles.spellout: <model>`). -const spellOutRole roles.Role = "spellout" - +// The expansion is [roles.RoleSpellOut] — declared there with every other role's +// word so that everything which reads the vocabulary can see it — and registered +// here, where the call is, so it resolves the way every other auxiliary call in +// this build resolves: the person's pin, then the tier, then the conversation's +// own model. func init() { - roles.Register(spellOutRole, roles.TierLow, "what a half-written request obviously means") + roles.Register(roles.RoleSpellOut, roles.TierLow, "what a half-written request obviously means") } const ( @@ -150,7 +143,7 @@ func (a *Agent) SpellOut(ctx context.Context, draft string) string { } a.mu.Lock() - call, err := roles.ResolveCall(roles.Source(a.config.RolesSource), spellOutRole, a.model) + call, err := roles.ResolveCall(roles.Source(a.config.RolesSource), roles.RoleSpellOut, a.model) closed := a.closed a.mu.Unlock() if closed || err != nil || strings.TrimSpace(call.Model) == "" { @@ -172,6 +165,7 @@ func (a *Agent) SpellOut(ctx context.Context, draft string) string { // A side errand of the box the person is typing in, named as one so it // is priced as one and never owns the clock (internal/lane's roles.go). provider.WithRole(provider.WithoutStream(ctx), lane.RoleAuxiliary), + callPurpose(roles.RoleSpellOut), []ai.Message{ textMessage("system", spellOutSystem), // THE INSTRUCTION IS LAST, after the draft rather than above it, for diff --git a/internal/session/spellout_test.go b/internal/session/spellout_test.go index cfd9009b58..eb00c6f78a 100644 --- a/internal/session/spellout_test.go +++ b/internal/session/spellout_test.go @@ -74,7 +74,7 @@ func TestALongClauseIsHeldToItsCap(t *testing.T) { // call in this build resolves. A role that stopped being registered would fall // back to the conversation's own model, which is the expensive one. func TestTheExpansionIsRegisteredOnTheLowTier(t *testing.T) { - tier, ok := roles.TierOf(spellOutRole) + tier, ok := roles.TierOf(roles.RoleSpellOut) if !ok { t.Fatal("the expansion is not a registered role") } diff --git a/internal/session/standing_run.go b/internal/session/standing_run.go index a70bef4fba..57e6ac1da8 100644 --- a/internal/session/standing_run.go +++ b/internal/session/standing_run.go @@ -98,16 +98,11 @@ const ( standingOutcomeClip = 400 ) -// standingSentinelRole is the cheap yes/no judgment, registered as a ROLE so it -// resolves the way every other auxiliary call in this build resolves: the -// person's pin, then the low tier, then the conversation's own model -// (internal/roles). It sits low for the guardian's reason — it reads a few -// kilobytes and answers one binary question, and a wrong no costs a check that -// said nothing rather than money. -const standingSentinelRole roles.Role = "sentinel" - +// The cheap yes/no judgment is [roles.RoleSentinel] — declared there with every +// other role's word, which carries the reasoning — and registered here, where +// the call is. func init() { - roles.Register(standingSentinelRole, roles.TierLow, "is this worth telling you about") + roles.Register(roles.RoleSentinel, roles.TierLow, "is this worth telling you about") } // ── the live-window registry ──────────────────────────────────────────────── @@ -1025,7 +1020,7 @@ func NewStandingSentinel(parent Config) standing.Sentinel { ) return func(ctx context.Context, judgment standing.Judgment) (bool, string, float64, error) { once.Do(func() { - model, built = roles.Resolve(roles.Source(parent.RolesSource), standingSentinelRole, parent.Model) + model, built = roles.Resolve(roles.Source(parent.RolesSource), roles.RoleSentinel, parent.Model) if built != nil { return } @@ -1063,8 +1058,15 @@ func NewStandingSentinel(parent Config) standing.Sentinel { // the phase clock share: a standing run has no one in front of it, so // its wait is worth nothing and its stream is nobody's to watch // (internal/lane's roles.go). + // + // And the purpose, because this road builds its own client and never + // passes the door: it is the call this build makes most often with + // nobody there, and until it said so it reached the log with no tag at + // all (clientdoor.go's [withPurpose]). callCtx := provider.WithRole( - provider.WithRoutingIntent(provider.WithoutStream(ctx), provider.IntentBackground), + provider.WithRoutingIntent( + provider.WithoutStream(withPurpose(ctx, purposeSentinel)), + provider.IntentBackground), lane.RoleStanding) if rung := effort.Resolve(effort.Scope{ Task: restoredRung(judgment.Item.Does.Effort), diff --git a/internal/session/steer_test.go b/internal/session/steer_test.go index d6420e36af..08c424c95c 100644 --- a/internal/session/steer_test.go +++ b/internal/session/steer_test.go @@ -23,6 +23,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "path/filepath" "strings" "sync" @@ -349,12 +350,32 @@ func TestASteerCutsEvenTheLastGenerationBeforeItCanSeal(t *testing.T) { } } +// steerGrindRounds is how many tool rounds this fixture's turn spends before it +// answers in words: the ceiling's own first mark, plus two, which is what opens +// the price gate in front of the one reading a turn still waits for. +var steerGrindRounds = checkpointMarkAt(1) + 2 + +// happyFallThroughCompleter answers an ordinary turn and PARKS AT ITS LAST +// BOUNDARY, so that a steer can be sent while the turn is provably still +// running and provably has no next step to be spliced into. +// +// IT PARKS ON THE REMAINS READER AND NOT ON THE ROUTE JUDGE, and the difference +// is the whole of what moved. The judge used to hold the turn open until it +// answered, which made it a convenient prop for "past the last boundary and +// still running" — and it does not hold it any more: the turn seals when the +// answer is done and the ruling is spent whenever it lands (loop.go). +// +// What is left is the ONE end-of-turn reading the turn still waits for: the +// reader asking what is left of the ask, which decides whether there IS a turn +// to seal. It is asked past the ceiling's price gate, so this turn grinds to it +// — which makes the window an honest property of the turn's shape rather than a +// reading that happened to be slow. type happyFallThroughCompleter struct { mu sync.Mutex conversation [][]ai.Message - judgeEntered chan struct{} - releaseJudge chan struct{} - judgeOnce sync.Once + atLastBound chan struct{} + releaseBound chan struct{} + boundOnce sync.Once } func (c *happyFallThroughCompleter) CompleteWithMessages(ctx context.Context, messages []ai.Message, _ ...ai.Option) (*ai.Response, error) { @@ -366,21 +387,21 @@ func (c *happyFallThroughCompleter) CompleteWithMessages(ctx context.Context, me return textResponse(routeConfirmNo), nil } if system == routeJudgeBrief { + return textResponse(routeConfirmNo), nil + } + if len(messages) > 0 && strings.Contains(messageText(messages[len(messages)-1]), "[still asked]") { wait := false - c.judgeOnce.Do(func() { - close(c.judgeEntered) + c.boundOnce.Do(func() { + close(c.atLastBound) wait = true }) if wait { select { - case <-c.releaseJudge: + case <-c.releaseBound: case <-ctx.Done(): return nil, ctx.Err() } } - return textResponse(routeConfirmNo), nil - } - if len(messages) > 0 && strings.Contains(messageText(messages[len(messages)-1]), "[still asked]") { return textResponse(checkpointNothingLeft), nil } if system != "SYSTEM" { @@ -389,7 +410,19 @@ func (c *happyFallThroughCompleter) CompleteWithMessages(ctx context.Context, me c.mu.Lock() snapshot := append([]ai.Message(nil), messages...) c.conversation = append(c.conversation, snapshot) + round := len(c.conversation) c.mu.Unlock() + // THE TURN GRINDS TO THE PRICE GATE and then answers in words, so the reader + // this fixture parks on is really asked. A different path every round, for + // [grindingSteps]' reason: a turn repeating one call reads as a turn going in + // circles and is handed over long before the gate. + if round <= steerGrindRounds { + arguments, _ := json.Marshal(struct { + Path string `json:"path"` + }{Path: fmt.Sprintf("./%d", round)}) + return toolResponseWithText(fmt.Sprintf("call-%d", round), "ls", string(arguments), + "Working through the next path."), nil + } return textResponse("ordinary answer"), nil } @@ -406,24 +439,29 @@ func (c *happyFallThroughCompleter) requests() [][]ai.Message { func TestASteerAfterTheLastBoundaryFallsThroughIntoOneFreshTurn(t *testing.T) { path := filepath.Join(t.TempDir(), "session.jsonl") completer := &happyFallThroughCompleter{ - judgeEntered: make(chan struct{}), - releaseJudge: make(chan struct{}), + atLastBound: make(chan struct{}), + releaseBound: make(chan struct{}), } agent, _ := newTestAgent(t, completer, func(config *Config) { config.SessionFile = path config.AskConsent = true config.RolesSource = tierSettings(map[string]string{ roles.TierKey(roles.TierLow): routeScreenModel, + // AND A MASTERMIND, because the reader this fixture parks on is one + // ([roles.RoleMarkReader]) and a session that cannot resolve it skips + // the reading entirely — which would leave this turn with no + // end-of-turn wait at all and no window to steer into. + roles.TierKey(roles.TierMastermind): routeScreenModel, }) }) turn := mustSubmit(t, agent, "explain how parser state flows across every package") select { - case <-completer.judgeEntered: + case <-completer.atLastBound: case <-time.After(10 * time.Second): t.Fatal("the successful turn never passed its last model boundary") } steered := mustSteer(t, agent, "fix the parser instead") - close(completer.releaseJudge) + close(completer.releaseBound) collect(t, turn) fromSteer := collect(t, steered) @@ -434,10 +472,11 @@ func TestASteerAfterTheLastBoundaryFallsThroughIntoOneFreshTurn(t *testing.T) { t.Fatalf("steer events = %v, want %v", got, want) } requests := completer.requests() - if len(requests) != 2 { - t.Fatalf("ordinary requests = %d, want the original and one fresh turn", len(requests)) + if want := steerGrindRounds + 2; len(requests) != want { + t.Fatalf("ordinary requests = %d, want %d — the original turn's rounds and one fresh turn", + len(requests), want) } - got := userLines(requests[1]) + got := userLines(requests[len(requests)-1]) var copies int for _, line := range got { if line == "fix the parser instead" { diff --git a/internal/session/subharness_env.go b/internal/session/subharness_env.go index 5d57673628..2c300e4d91 100644 --- a/internal/session/subharness_env.go +++ b/internal/session/subharness_env.go @@ -203,7 +203,7 @@ func (e *subharnessEnv) AI(ctx context.Context, promptRef string, input any, opt if !opts.Schema.Empty() { options = append(options, ai.WithSchema(json.RawMessage(opts.Schema))) } - response, err := e.agent.completeWithModel(call, []ai.Message{ + response, err := e.agent.completeWithModel(call, purposeSubharness, []ai.Message{ textMessage("system", prompt), // THE INPUT GOES LAST, as its own message and as JSON. The prompt is the // asset under review and the input is the material; two messages keep diff --git a/internal/session/subharness_intake.go b/internal/session/subharness_intake.go index 987fe814db..844e727e46 100644 --- a/internal/session/subharness_intake.go +++ b/internal/session/subharness_intake.go @@ -167,6 +167,7 @@ func (a *Agent) fillSubharnessCard(card *SubharnessCard) { // (internal/lane's roles.go). response, err := a.completeWithModel( provider.WithRole(provider.WithoutStream(ctx), lane.RoleAuxiliary), + callPurpose(roles.RoleIntake), []ai.Message{ textMessage("system", intakeSystem), textMessage("user", "The form:\n"+intakeFields(*card)+ diff --git a/internal/session/task_cascade_test.go b/internal/session/task_cascade_test.go index daf7d1060a..925756dc84 100644 --- a/internal/session/task_cascade_test.go +++ b/internal/session/task_cascade_test.go @@ -3,7 +3,7 @@ package session // THE CASCADE: WHERE A CHEAP CREW BUYS THE CAREFUL MODEL, AND WHERE IT DOES NOT. // // One escalation exists in this build (repair_role.go): work a checker sent back -// is handed to [roleRepair], which sits on the careful tier. Four things have to +// is handed to [roles.RoleRepair], which sits on the careful tier. Four things have to // hold for that to be an economy rather than a leak, and each of them is a test // here: // @@ -44,18 +44,18 @@ const carefulTier = "test/careful-model" // be paying a thinking model's price for many turns of ordinary editing, which // is the split [roles.TierMastermind] exists to keep. func TestTheRepairRoleSitsOnTheCarefulTier(t *testing.T) { - tier, ok := roles.TierOf(roleRepair) + tier, ok := roles.TierOf(roles.RoleRepair) if !ok { - t.Fatalf("%q is not a registered role, so it resolves to nothing", roleRepair) + t.Fatalf("%q is not a registered role, so it resolves to nothing", roles.RoleRepair) } if tier != roles.TierHigh { - t.Fatalf("%q resolves on %q, want the careful tier", roleRepair, tier) + t.Fatalf("%q resolves on %q, want the careful tier", roles.RoleRepair, tier) } // A ROW A PERSON READS. The settings sheet lists every registered role, and a // role with no line under its name is a call somebody pays for and cannot // read (internal/roles' Describe states the emptiness law about it). - if strings.TrimSpace(roles.Describe(roleRepair)) == "" { - t.Fatalf("%q has no description, so its settings row says only its own name", roleRepair) + if strings.TrimSpace(roles.Describe(roles.RoleRepair)) == "" { + t.Fatalf("%q has no description, so its settings row says only its own name", roles.RoleRepair) } } @@ -91,12 +91,12 @@ func TestARepairRoundResolvesThroughTheRepairRolesTier(t *testing.T) { // role alone moves the round and nothing else in the crew. pinned, _ := cascadeAgent(t, map[string]string{ - roles.TierKey(roles.TierHigh): carefulTier, - roles.PinKey(roleRepair): "vendor/mender", + roles.TierKey(roles.TierHigh): carefulTier, + roles.PinKey(roles.RoleRepair): "vendor/mender", }, taskSpec{title: "the greeting", model: "vendor/flash"}) if got := pinned.repairModel(node); got != "vendor/mender" { - t.Fatalf("a pin on %q resolved to %q", roleRepair, got) + t.Fatalf("a pin on %q resolved to %q", roles.RoleRepair, got) } } diff --git a/internal/session/task_run.go b/internal/session/task_run.go index 506ca62ff3..d828bfadd2 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -5264,8 +5264,8 @@ func (a *Agent) workTaskNode(ctx context.Context, node *TaskNode, listed *job) T } // THE NODE'S SPEND IS THE PERSON'S, so it is folded into the session's - // auxiliary usage — the pocket the title and the compaction summary come out - // of — rather than charged to whichever turn happened to propose it. It is + // auxiliary usage — the pocket the title and the memory reflex come out of — + // rather than charged to whichever turn happened to propose it. It is // kept ON THE NODE as well, in the same call, because the node outlives its // child: a surface asking a landed node what it cost has nobody else to ask // (see [TaskNode.spend]). A node that was re-modelled has TWO children, and diff --git a/internal/session/task_shape.go b/internal/session/task_shape.go index 3b88dc9b99..16d321799f 100644 --- a/internal/session/task_shape.go +++ b/internal/session/task_shape.go @@ -179,7 +179,7 @@ func (a *Agent) shapeBrief(ctx context.Context, request string) (shapedBrief, bo ctx = provider.WithRole(provider.WithoutStream(ctx), lane.RoleAuxiliary) messages := []ai.Message{textMessage("system", shapePrompt), textMessage("user", request)} for attempt := 0; attempt < 2; attempt++ { - response, callErr := a.completeWithModel(ctx, messages, call.Model) + response, callErr := a.completeWithModel(ctx, callPurpose(roles.RoleShaper), messages, call.Model) if callErr != nil || response == nil { return shapedBrief{}, false } diff --git a/internal/session/taxonomy_boundary.go b/internal/session/taxonomy_boundary.go index 616e7f8ca4..8a4b967acd 100644 --- a/internal/session/taxonomy_boundary.go +++ b/internal/session/taxonomy_boundary.go @@ -618,7 +618,7 @@ func (a *Agent) readFinding(node *TaskNode, log io.Writer) taxonomy.Verdict { // under it reads exactly like a finding with none. evidence.TransportSeen = 1 } - verdict := a.classify(evidence, node.runModel(), string(roleRepair)) + verdict := a.classify(evidence, node.runModel(), string(roles.RoleRepair)) switch verdict.Action { case taxonomy.ActionEscalate: // THE TALLY IS NOT TOLD YET. A lift is recorded at the moment it is @@ -646,7 +646,7 @@ func (a *Agent) readPass(node *TaskNode, log io.Writer) taxonomy.Verdict { tally := a.tallyFor(node) evidence := tally.Evidence() evidence.Passed = true - verdict := a.classify(evidence, node.runModel(), string(roleRepair)) + verdict := a.classify(evidence, node.runModel(), string(roles.RoleRepair)) if verdict.Action == taxonomy.ActionDeescalate { tally.Deescalate() fmt.Fprintf(log, "the check passed: the work goes back to %s\n", node.runModel()) diff --git a/internal/session/title_test.go b/internal/session/title_test.go index ad40eac90a..223519fcd4 100644 --- a/internal/session/title_test.go +++ b/internal/session/title_test.go @@ -340,9 +340,9 @@ func TestTitleModelFollowsTheRolesLadder(t *testing.T) { {"nothing set", nil, "test/model"}, {"the tier", map[string]string{roles.TierKey(roles.TierLow): "cheap/model"}, "cheap/model"}, {"the pin beats the tier", map[string]string{ - roles.TierKey(roles.TierLow): "cheap/model", - roles.PinKey(roles.RoleTitle): "pinned/model", - roles.PinKey(roles.RoleCompaction): "other/model", + roles.TierKey(roles.TierLow): "cheap/model", + roles.PinKey(roles.RoleTitle): "pinned/model", + roles.PinKey(roles.RoleAuditor): "other/model", }, "pinned/model"}, } { t.Run(testCase.name, func(t *testing.T) { diff --git a/internal/session/toolask.go b/internal/session/toolask.go index 3e5ea0b331..7691b55fff 100644 --- a/internal/session/toolask.go +++ b/internal/session/toolask.go @@ -118,7 +118,6 @@ func (a *Agent) askModel(ctx context.Context, question toolAsk) (string, error) // the client bounded in total rather than on the stream client, which by // design carries no total deadline at all (internal/provider's transport.go). asked := provider.WithRole(provider.WithoutStream(ctx), lane.RoleTool) - asked = provider.WithCallTag(asked, toolCallTag(question.tool)) if a.config.taskID != 0 { asked = provider.WithCallNode(asked, strconv.FormatUint(a.config.taskID, 10)) } @@ -126,7 +125,9 @@ func (a *Agent) askModel(ctx context.Context, question toolAsk) (string, error) defer stop() // NO OUTPUT CEILING TRAVELS WITH IT ([toolAskSendsNoOutputCeiling]). - response, err := a.completeWithModel(asked, []ai.Message{{ + // THE TOOL IS THE PURPOSE, carried to the door rather than stamped here, so + // that this package has one place that spells a tag (clientdoor.go). + response, err := a.completeWithModel(asked, callPurpose(toolCallTag(question.tool)), []ai.Message{{ Role: "user", Content: []ai.ContentPart{{Type: "text", Text: question.prompt}, question.part}, }}, question.model) diff --git a/internal/session/tools_capabilities.go b/internal/session/tools_capabilities.go index 5e738cc564..02bc618af2 100644 --- a/internal/session/tools_capabilities.go +++ b/internal/session/tools_capabilities.go @@ -267,9 +267,10 @@ func (a *Agent) shelvedTools() []bare.Tool { // meets an unfamiliar name in its own history find the group it is in. func (a *Agent) loadCapabilityTool(shelf map[string][]bare.Tool, order []string) bare.Tool { return bare.Tool{ - Name: loadCapabilityToolName, - Description: loadCapabilityDescription(shelf, order), - Schema: json.RawMessage(loadCapabilitySchema(order)), + Name: loadCapabilityToolName, + Description: loadCapabilityDescription( + func(group string) []string { return toolNames(shelf[group]) }, order), + Schema: json.RawMessage(loadCapabilitySchema(order)), Execute: func(ctx context.Context, args json.RawMessage) (string, bool, error) { var parsed struct { Group string `json:"group"` @@ -287,10 +288,17 @@ func (a *Agent) loadCapabilityTool(shelf map[string][]bare.Tool, order []string) // sentence in it is a law the model cannot get elsewhere: what the verb does, // WHEN the tools become usable, how long they last, and that it is not // permission. -func loadCapabilityDescription(shelf map[string][]bare.Tool, order []string) string { +// IT TAKES THE MEMBERS AS A QUESTION rather than the shelf itself, because what +// is on the shelf depends on the MACHINE — `edit_video` is built only where +// ffmpeg is on PATH, and a group whose every member was gated off is not named +// at all — while the bytes this sentence costs are spent on every request +// everywhere. The one gate that bounds the fixed prefix has to be able to ask +// for the widest answer rather than this machine's (prefixbudget_test.go's +// widestBelt), and a function is the seam that lets it. +func loadCapabilityDescription(members func(group string) []string, order []string) string { var lines strings.Builder for _, name := range order { - lines.WriteString(" " + name + ": " + strings.Join(toolNames(shelf[name]), ", ") + ".") + lines.WriteString(" " + name + ": " + strings.Join(members(name), ", ") + ".") } return "Load one additional tool group. Available groups:" + lines.String() + " Full schemas arrive on the next model request; continue in this same turn. " + diff --git a/internal/session/tools_doc.go b/internal/session/tools_doc.go index d62ccd311d..92208bff19 100644 --- a/internal/session/tools_doc.go +++ b/internal/session/tools_doc.go @@ -338,7 +338,12 @@ func (a *Agent) readDocument(ctx context.Context, path, question string, offset, if rung == provider.DocumentParseNative { asked = question } - response, err := client.ParseDocument(ctx, provider.DocumentRequest{ + // THE PURPOSE, because this road builds its own client and never passes + // the door (clientdoor.go's [withPurpose]). A rung of the document reader + // is the model's own eyes on a file, billed to the session and asked for + // by a tool call rather than by a turn, and it reached the call log with + // no tag at all until it said so. + response, err := client.ParseDocument(withPurpose(ctx, purposeDocument), provider.DocumentRequest{ Model: model, Filename: filename, MediaType: entry.mediaType, @@ -349,8 +354,8 @@ func (a *Agent) readDocument(ctx context.Context, path, question string, offset, // Accounted BEFORE the answer is judged, and folded into the SESSION // total rather than the turn's ([Agent.addAuxiliaryUsage]): a rung that // billed for an unusable answer still billed, and no turn of the - // person's ran on that endpoint (the same treatment the title, the - // compaction summary and a generated picture get). + // person's ran on that endpoint (the same treatment the title and a + // generated picture get). if response != nil { a.addAuxiliaryUsage(&ai.Response{Usage: response.Usage}, model, 1) } diff --git a/internal/session/tools_image.go b/internal/session/tools_image.go index 08a3741954..2120e8482e 100644 --- a/internal/session/tools_image.go +++ b/internal/session/tools_image.go @@ -159,8 +159,8 @@ func (a *Agent) generateImageTool(client MediaGenerator, defaultModel string) ba } // The picture is paid for whether or not it is any good, so the // accounting lands before the write can fail. It folds into the - // SESSION total and not the turn's, exactly as the title and the - // compaction summary do: no turn asked for a picture at this price, + // SESSION total and not the turn's, exactly as the title call does: + // no turn asked for a picture at this price, // and charging one turn for it would make an ordinary question read // as the cost of a rendering (see [Agent.addAuxiliaryUsage]). a.addAuxiliaryUsage(&ai.Response{Usage: response.Usage}, model, 1) diff --git a/internal/tui3/settings.go b/internal/tui3/settings.go index bf67d2e0a3..9e84e717b6 100644 --- a/internal/tui3/settings.go +++ b/internal/tui3/settings.go @@ -380,7 +380,7 @@ var settingUI = map[string]settingMeta{ }, config.KeyTierHighModel: { tab: tabProviders, label: "careful work", widget: widgetSelect, - about: "careful · checks what must not be wrong — audits, compaction, vision", + about: "careful · checks what must not be wrong — audits, briefs, vision", }, // The fourth class is the one whose value may name a LEVEL as well as a // model, so it is a TEXT box and not a picker: the picker returns an id, and @@ -1746,6 +1746,14 @@ func (a *app) applyRolePin(row config.Setting, role roles.Role, model string) { a.sheet.msg = err.Error() return } + // AND A PIN THIS BUILD NO LONGER ACTS ON IS SAID OUT LOUD to the person who + // is about to lose it. The row already DRAWS only the live pins + // (config.LivePinsAt) and this write will not put the dead one back, so + // without the line the row would quietly get shorter under their hands. It + // reads the raw stored text, which is the only place the dead word still is. + if note := config.RetiredPinNote(config.ModelRolesAt(a.profileDir)); note != "" { + a.sheet.msg = note + } if model = strings.TrimSpace(model); model == "" { delete(pins, string(role)) } else { diff --git a/internal/tui3/settingsroles_test.go b/internal/tui3/settingsroles_test.go index 176a46b61d..bbe4b52f59 100644 --- a/internal/tui3/settingsroles_test.go +++ b/internal/tui3/settingsroles_test.go @@ -105,7 +105,7 @@ func TestTheRolesSectionSaysWhatAnswersEachRole(t *testing.T) { // The worker is the seat that does the work, and sits apart from the // small calls beside it. {roles.RoleWorker, "worker", "test/worker-model"}, - {roles.RoleCompaction, "careful work", "test/careful-model"}, + {roles.RoleAuditor, "careful work", "test/careful-model"}, {roles.RoleTitle, "small work", "test/cheap-model"}, } { row := roleItem(t, a, c.role) @@ -198,7 +198,7 @@ func TestARoleFollowsItsShippedClassAndThenTheConversation(t *testing.T) { {roles.RolePlanner, config.DefaultMastermindModel}, {roles.RoleWorker, config.DefaultWorkerModel}, {roles.RoleTitle, config.DefaultLowModel}, - {roles.RoleCompaction, config.DefaultHighModel}, + {roles.RoleAuditor, config.DefaultHighModel}, {roles.RoleReflex, config.DefaultReflexModel}, } { if got := roleItem(t, a, c.role).model; got != c.want {