From 392759b5cd5aba071be3f2632dd7ac5313c055da Mon Sep 17 00:00:00 2001 From: Yogthos Date: Mon, 28 Sep 2026 16:12:21 -0400 Subject: [PATCH] Keep tests and acceptance apart, ask RFC designs for the RFC, excuse blocked checks The rerun on endless-flight shipped all three tasks with the suite green and was sent back on one acceptance criterion; the route note recorded that as tests failing, so the supervisor's trigger for green work sent back never fired and every pass it made concluded nothing was wrong. The route now records the tests and acceptance separately and the brief names the criterion that refused the round. plan's signature did not declare rfc, so native-tool providers could not send the RFC an RFC design is read from, and the re-prompt asked for the files and goal the branch had already given. Both fixed. An acceptance check the host refused (no window server) is tried again and then counted as undecided rather than failed, so a round is not sent back to repair the environment. --- resources/cells/board.clj | 13 +++++++----- resources/cells/feature.clj | 20 ++++++++++++++++--- resources/cells/oversight.clj | 5 +++++ resources/gates.edn | 1 + resources/prompts/design-no-plan.md | 1 + resources/prompts/system-tools.md | 5 +++-- resources/userspace.edn | 1 + resources/wordlists.edn | 8 ++++++++ src/samizdat/agent/acceptance.clj | 29 ++++++++++++++++++++------- src/samizdat/agent/telemetry.clj | 8 +++++++- test/samizdat/acceptance_test.clj | 20 +++++++++++++++++++ test/samizdat/board_test.clj | 31 +++++++++++++++++++++++++++++ test/samizdat/feature_test.clj | 7 ++++++- test/samizdat/oversight_test.clj | 19 ++++++++++++++++++ 14 files changed, 149 insertions(+), 19 deletions(-) create mode 100644 resources/prompts/design-no-plan.md diff --git a/resources/cells/board.clj b/resources/cells/board.clj index 4afdd9a9..edff36bf 100644 --- a/resources/cells/board.clj +++ b/resources/cells/board.clj @@ -682,7 +682,8 @@ :requires [:conn :run-id] :input [:map [:board/task {:optional true} :any] [:board/plan-text {:optional true} :any] - [:board/plan-attempts {:optional true} :int]] + [:board/plan-attempts {:optional true} :int] + [:board/plan-decision {:optional true} :any]] :output [:map [:board/design-decision :keyword] [:board/design-findings {:optional true} :any]]} (fn [{:keys [conn run-id] :as ctx} {:keys [board/task board/plan-text] :as data}] @@ -729,10 +730,12 @@ :else :revise) findings (cond (and no-plan? (= :revise decision)) - (str "You did not produce a plan. End your planning step " - "with a `plan` call that names the files this change " - "touches, the tests that pin it, and a one- or " - "two-sentence goal stating how it meets the WHOLE ask.") + ;; Named by the path: an RFC task's plan-text is the + ;; plan's `rfc`, and asking it for files, tests and a + ;; goal it had already given told it nothing + ;; (karamazov-na2k.22). + (prompt/render "design-no-plan" + {:rfc (= :rfc (:board/plan-decision data))}) (= :revise decision) (:findings reviewed) :else nil)] (journal/note! conn run-id :design-review diff --git a/resources/cells/feature.clj b/resources/cells/feature.clj index a75a5aa9..975afcd6 100644 --- a/resources/cells/feature.clj +++ b/resources/cells/feature.clj @@ -517,7 +517,9 @@ :input [:map [:branch :map] [:review/decision :keyword] [:critic/decision :keyword]] ;; Both keys on every branch of the cond, the note carrying WHY on the ;; paths where nothing ran. - :output [:map [:verify/passed? :boolean] [:verify/note :any]]} + :output [:map [:verify/passed? :boolean] [:verify/note :any] + [:verify/tests-passed? {:optional true} :any] + [:verify/accepted? {:optional true} :any]]} (fn [{:keys [conn run-id root config] :as ctx} data] (let [cmd (get-in config [:run :verify-cmd]) criteria? (seq (get-in config [:run :acceptance]))] @@ -565,7 +567,10 @@ {:data {:passed passed? :exit (:exit r) :timeout (:timeout r) :tests-passed tests-passed? :accepted accepted? :note (judge/for-the-record :reply-chars note)}}) - (assoc data :verify/passed? passed? :verify/note note)))))) + ;; The two halves as well as the verdict, so the route note can + ;; say which one refused (karamazov-na2k.23). + (assoc data :verify/passed? passed? :verify/note note + :verify/tests-passed? tests-passed? :verify/accepted? accepted?)))))) (cell/defcell :feature/supervise {:doc "Where the supervisor's directives about the OUTER loop land. @@ -687,7 +692,16 @@ {:data {:decision decision :revision rev :soft-cap soft-cap :hard-cap hard-cap :hollow hollow :strategy strategy :auto-switch auto-next - :tests-passed (:verify/passed? data) + ;; THE TESTS, not gate 2 as a whole: a green + ;; suite refused on an acceptance criterion + ;; read as a red one, so the supervisor's + ;; green-but-sent-back trigger never fired + ;; (karamazov-na2k.23). Where verify did not run + ;; the halves, the verdict stands in. + :tests-passed (if (contains? data :verify/tests-passed?) + (:verify/tests-passed? data) + (:verify/passed? data)) + :accepted (:verify/accepted? data) :gave-up (boolean give-up?) :runaway runaway?}}) (case decision :ship diff --git a/resources/cells/oversight.clj b/resources/cells/oversight.clj index 2b7e1f04..540b6287 100644 --- a/resources/cells/oversight.clj +++ b/resources/cells/oversight.clj @@ -424,6 +424,11 @@ :at-cap? (at-cap? round) :hollow? (:hollow round) :tests-passed? (:tests-passed round) + ;; Whether the operator's criteria held, + ;; and gate 2's own words for why not + ;; (karamazov-na2k.23). + :accepted? (:accepted round) + :verify-note (:note (journal/last-note conn run-id :verify)) :review (some-> (journal/last-note conn run-id :review) :decision keyword) :critic (some-> (journal/last-note conn run-id :critique) diff --git a/resources/gates.edn b/resources/gates.edn index 56a0e04b..f2c25c3c 100644 --- a/resources/gates.edn +++ b/resources/gates.edn @@ -2228,6 +2228,7 @@ :nobody-shipped "NO IMPLEMENTOR SHIPPED — the implement round produced nothing verified" :tests-failing "TESTS FAILING — {{detail}}" :tests-failing-fallback "the tests did not pass" + :acceptance-failing "ACCEPTANCE NOT MET, TESTS GREEN — the round was sent back on the operator's criteria, not on its tests: {{detail}}" :revision-cap "REVISION CAP REACHED — {{revision}} revisions (soft cap {{soft-cap}}) with no verified solution. This is a soft stop for YOU to decide: keep solving with a new approach, or STOP if it is a genuine dead end." :thrash "THRASH — a branch spent a third+ of its turns on empty/mis-parsed calls" :reviewer-bounced "REVIEWER BOUNCED — the reviewer sent the work back" diff --git a/resources/prompts/design-no-plan.md b/resources/prompts/design-no-plan.md new file mode 100644 index 00000000..02f84f88 --- /dev/null +++ b/resources/prompts/design-no-plan.md @@ -0,0 +1 @@ +{% if rfc %}Your plan call did not carry the RFC. This change is designed as an RFC, and the `rfc` field of `plan` is what the reviewer reads: call `plan` again with the whole document in `rfc`, beside the files, the tests and the goal.{% else %}You did not produce a plan. End your planning step with a `plan` call that names the files this change touches, the tests that pin it, and a one- or two-sentence goal stating how it meets the WHOLE ask.{% endif %} diff --git a/resources/prompts/system-tools.md b/resources/prompts/system-tools.md index 67fa982d..b1813219 100644 --- a/resources/prompts/system-tools.md +++ b/resources/prompts/system-tools.md @@ -23,9 +23,10 @@ give_up({reason}) ### Developing at the REPL ``` -plan({files, tests?, goal?}) +plan({files, tests?, goal?, rfc?}) Say which files you are about to create or edit, which tests you will - write, and why — one line. Every entry in files and tests is a bare + write, and why — one line. When the design step asks for an RFC, `rfc` + carries the whole document; it is what the reviewer reads. Every entry in files and tests is a bare relative path such as test/flight/ghost_test.clj, nothing else: a path with a description after it is refused, because a declared file is what you are held to and a sentence can never be written. REQUIRED before eval: the REPL stays closed diff --git a/resources/userspace.edn b/resources/userspace.edn index e94225fc..1e8f1ab4 100644 --- a/resources/userspace.edn +++ b/resources/userspace.edn @@ -191,6 +191,7 @@ :system-structure "prompts/system-structure.md" :system-tools "prompts/system-tools.md" :system-turn "prompts/system-turn.md" + :design-no-plan "prompts/design-no-plan.md" :experiment-reviews "prompts/experiment-reviews.md" :pruning "prompts/pruning.md" :reply-format-dropped "prompts/reply-format-dropped.md" diff --git a/resources/wordlists.edn b/resources/wordlists.edn index ce847823..658f446a 100644 --- a/resources/wordlists.edn +++ b/resources/wordlists.edn @@ -378,6 +378,14 @@ ;; refuses refinements is worse than no detector. #{"not" "never" "fails" "failed" "cannot" "can't" "no" "without" "refuses"} + :environment-blocked + ;; Output that says the HOST refused an acceptance :check, not the work + ;; (karamazov-na2k.24): raylib with no window server. A check that prints one + ;; of these is tried once more and then counted as undecided rather than + ;; failed. Narrow on purpose: every phrase here is a way for a real failure + ;; to be excused, so add one only for an output that measured as the host's. + #{"Failed to initialize platform"} + :completeness-blocked ;; A stated LIMIT, which is a result rather than a plan. Same first-person ;; voice and the same work verb as an abandoned intention — "I have not been diff --git a/src/samizdat/agent/acceptance.clj b/src/samizdat/agent/acceptance.clj index d5be26f9..58af9830 100644 --- a/src/samizdat/agent/acceptance.clj +++ b/src/samizdat/agent/acceptance.clj @@ -40,6 +40,7 @@ policy already refuse to let a run write (karamazov-kvw, files/run-config?). A spec the run could edit would be Gate 2 all over again." (:require [clojure.string :as str] + [samizdat.lexicon :as lexicon] [samizdat.prompt :as prompt])) ;; --- the spec --------------------------------------------------------------- @@ -91,13 +92,27 @@ (assoc base :passed? nil :output "not run") (= :check kind) - (let [{:keys [green? timeout? output]} - (try (run-check text) - (catch Throwable e {:green? false :output (str "check failed to run: " (ex-message e))}))] - (assoc base :passed? (boolean green?) - :output (if timeout? - (str "timed out" (when-not (str/blank? (str output)) (str "\n" output))) - (str output)))) + (let [attempt (fn [] (try (run-check text) + (catch Throwable e {:green? false :output (str "check failed to run: " (ex-message e))}))) + blocked? (fn [{:keys [green? output]}] + (and (not green?) + (some #(str/includes? (str output) %) + (lexicon/wordlist :environment-blocked)))) + ;; A check the ENVIRONMENT refused — no window server, say — is + ;; tried once more, and if it is still refused it is undecided, + ;; not failed: the work was not what stopped it, and sending the + ;; round back to fix it asks an owner to repair the host + ;; (run eb64f59d, karamazov-na2k.24). + first-try (attempt) + r (if (blocked? first-try) (attempt) first-try) + {:keys [green? timeout? output]} r] + (if (blocked? r) + (assoc base :passed? nil :blocked true + :output (str "blocked by the environment\n" output)) + (assoc base :passed? (boolean green?) + :output (if timeout? + (str "timed out" (when-not (str/blank? (str output)) (str "\n" output))) + (str output))))) :else (let [{:keys [yes? reply]} diff --git a/src/samizdat/agent/telemetry.clj b/src/samizdat/agent/telemetry.clj index 0db07a42..651cf07a 100644 --- a/src/samizdat/agent/telemetry.clj +++ b/src/samizdat/agent/telemetry.clj @@ -282,7 +282,7 @@ deserve the supervisor's attention is mechanism — it is the same question whatever the project builds — while the words that carry them, and the two numbers behind the thrash judgement, are `gates.edn :run-health`." - [{:keys [results review revision errors hollow? tests-passed? verify-note + [{:keys [results review revision errors hollow? tests-passed? accepted? verify-note at-cap? soft-cap self-graded]} health] (let [total (count results) shipped (count (filter #(= :done (:status %)) results)) @@ -316,6 +316,12 @@ (conj (signal :tests-failing {:detail (or verify-note (signal :tests-failing-fallback))})) + ;; Green, and refused on the operator's criteria: its own signal, with + ;; gate 2's words, because 'the tests did not pass' is the wrong story + ;; and the supervisor acted on it (karamazov-na2k.23). + (and (false? accepted?) (not (false? tests-passed?))) + (conj (signal :acceptance-failing {:detail (or verify-note "")})) + at-cap? (conj (signal :revision-cap {:revision revision :soft-cap soft-cap})) diff --git a/test/samizdat/acceptance_test.clj b/test/samizdat/acceptance_test.clj index 0f185a0c..a74dc63c 100644 --- a/test/samizdat/acceptance_test.clj +++ b/test/samizdat/acceptance_test.clj @@ -261,3 +261,23 @@ :roles {:default :local} :run {:acceptance [{:check "no name"}]}}))) (is (not (system/started?)))) + +(deftest a-check-the-environment-blocked-is-undecided-not-failed + ;; karamazov-na2k.24, run eb64f59d: three hours in, the game-run check + ;; failed with raylib's "Failed to initialize platform" — no window server + ;; at that moment; the same check passed earlier in the run — and the round + ;; was sent back to fix work that was not broken. + (let [calls (atom 0) + blocked {:green? false :output "WARNING: SYSTEM: Failed to initialize platform"} + [r] (acceptance/check [{:name "the game runs" :kind :check :text "run-game"}] + {:kinds #{:check} + :run-check (fn [_] (swap! calls inc) blocked)})] + (is (nil? (:passed? r)) "undecided, like a judge that could not answer") + (is (true? (:blocked r))) + (is (= 2 @calls) "it was tried again once before being called blocked") + (is (acceptance/all-passed? [r]) "so it does not send the round back on its own")) + (testing "a check that failed for any other reason still fails" + (let [[r] (acceptance/check [{:name "suite" :kind :check :text "t"}] + {:kinds #{:check} + :run-check (fn [_] {:green? false :output "2 failures"})})] + (is (false? (:passed? r)))))) diff --git a/test/samizdat/board_test.clj b/test/samizdat/board_test.clj index 96040b9a..b67bb35b 100644 --- a/test/samizdat/board_test.clj +++ b/test/samizdat/board_test.clj @@ -18,6 +18,7 @@ [samizdat.agent.gitdiff :as gitdiff] [samizdat.agent.judge :as judge] [samizdat.cells :as cells] + [mycelium.cell :as cell] [samizdat.llm.client :as llm] [samizdat.store.db :as db] [samizdat.store.journal :as journal] @@ -332,6 +333,36 @@ (testing "fail-open held: construction still ran and the task closed" (is (= "done" (:status (first (db/fetch conn ["SELECT status FROM tasks"])))))))))) +(deftest an-rfc-design-with-no-rfc-is-asked-for-the-rfc + ;; karamazov-na2k.22, runs 390dcd17 and eb64f59d: an RFC-triaged design + ;; called plan with files, tests and a goal but no rfc — plan's signature + ;; did not declare one — and was sent back with "you did not produce a + ;; plan ... name the files, the tests and a goal", which it had just done. + (cells/load-cells!) + (let [conn (db/open! ":memory:") + rid (runs/start-run! conn {:problem "p"}) + t (tasks/create! conn {:title "fade the terrain" :body "b"}) + review (fn [decision] + ((:handler (cell/get-cell! :board/design-review)) + {:conn conn :run-id rid :config {}} + {:board/task t :board/plan-text nil :board/plan-attempts 1 + :board/plan-decision decision}))] + (let [r (review :rfc)] + (is (= :revise (:board/design-decision r))) + (is (str/includes? (str (:board/design-findings r)) "`rfc`") + "the re-prompt names the field the RFC path reads")) + (let [r (review :plan)] + (is (not (str/includes? (str (:board/design-findings r)) "`rfc`")) + "and a plain plan is not asked for one")))) + +(deftest the-plan-tool-declares-its-rfc-argument + ;; Native tool specs are derived from the signature lines; an argument the + ;; signature leaves out is not in the schema a native-tool provider sends. + (let [specs (into {} (map (juxt :name identity)) + ((requiring-resolve 'samizdat.llm.toolspec/signatures) + (slurp (clojure.java.io/resource "prompts/system-tools.md"))))] + (is (contains? (get-in specs ["plan" :parameters :properties]) :rfc)))) + (deftest triage-is-three-way-by-size ;; karamazov-dq1r: trivial skips, a multi-part task gets an RFC, an ordinary ;; single-change task gets the lightweight plan in between. diff --git a/test/samizdat/feature_test.clj b/test/samizdat/feature_test.clj index aade7f13..8b1c60a0 100644 --- a/test/samizdat/feature_test.clj +++ b/test/samizdat/feature_test.clj @@ -460,7 +460,12 @@ (is (seq at-verify) "the verdicts are on the journal from the verify stage") (is (= [true false] (mapv :passed? (:results (first at-verify)))) "the check passed and the judge failed, each on its own line")) - (is (some #(false? (:tests-passed %)) route) "route saw gate 2 red") + ;; karamazov-na2k.23: the two halves stay apart on the route note. It + ;; used to say :tests-passed false here — the suite was green — and so + ;; the supervisor's green-but-sent-back trigger never fired and its + ;; brief said the tests were failing. + (is (some #(and (true? (:tests-passed %)) (false? (:accepted %))) route) + "route saw gate 2 red on acceptance, with the tests green") (is (some #(str/includes? (str (:note %)) "says what it saw") verify) "and the verify note names the criterion, in the failure's own words"))) (testing "the same run with a YES completes" diff --git a/test/samizdat/oversight_test.clj b/test/samizdat/oversight_test.clj index 3e152c88..9e60b313 100644 --- a/test/samizdat/oversight_test.clj +++ b/test/samizdat/oversight_test.clj @@ -938,6 +938,25 @@ "the finding reaches the brief beside the change it was measured under"))) (finally (session/reset!)))) +(deftest a-green-round-refused-on-acceptance-wakes-the-supervisor-and-says-why + ;; karamazov-na2k.23, run eb64f59d: every task shipped, the suite was green, + ;; one acceptance judge said no, and three supervisor passes concluded + ;; 'shipped clean, nothing wrong'. The brief has to say which criterion. + (let [conn (db/open! ":memory:") + rid (runs/start-run! conn {:problem "p"})] + (journal/note! conn rid :verify + {:data {:passed false :tests-passed true :accepted false + :note "tests passed\nacceptance criteria not met:\nFAIL the game was exercised, not only unit-tested"}}) + (journal/note! conn rid :route + {:data {:decision "revise" :revision 0 :soft-cap 6 :strategy "board" + :tests-passed true :accepted false}}) + (let [{:keys [gather prob]} (reasoning-over conn rid)] + (is (true? (:oversight/worth-a-look? gather)) "green work sent back is worth a look") + (is (str/includes? (str prob) "the game was exercised, not only unit-tested") + "and the brief names the criterion that refused it") + (is (not (str/includes? (str prob) "the tests did not pass")) + "rather than saying the tests failed")))) + (deftest a-refused-mutation-is-worth-a-model-call ;; A refusal means the supervisor tried to change the harness and the ;; protocol said no. That is precisely a moment where the next pass will