From 4ddd4fc88ec79cb3a909df233d90cbb031d6fb32 Mon Sep 17 00:00:00 2001 From: Smana Date: Mon, 24 Aug 2026 14:37:02 +0200 Subject: [PATCH 1/3] fix(investigate): make a submitted finding account for its own verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two live cards shipped with the header promising something the body did not contain, and both payloads were legal: 2026-08-24 02:18 โ€” "Action suggested", no next-steps section at all. suggested_action and actions are both optional, and the verdict badge is drawn from the verdict alone. 2026-08-22 22:53 โ€” "High confidence ยท 85%" over a Why paragraph with no evidence bullets. evidence was optional too, so the badge was backed by the model's say-so and the verify pass had nothing to trace. Require evidence (minItems 1) on a root cause and say what it is for. Say that suggested_action is required when the verdict claims an action. Add actionWithoutRemedy and unevidencedConclusion next to unaccountedInconclusive โ€” pure predicates, logged at the source, not errors: the analysis is still worth delivering and re-asking would spend a model call to re-answer a question the model just answered badly. The renderer stops hiding the first case: a verdict claiming an action with no remedy now says so on the card instead of dropping the section, so the reader can tell an absent remedy from a failed render. Also close the other half of the inconclusive rule. The schema said a recurrence you DID identify is not inconclusive; it never said the converse, and the 02:18 card reported 85% "high confidence" on a finding whose actual content was that it could not identify the failing resource. Confidence measures support for the cause you stated. --- .../investigate/findings_contract_test.go | 85 +++++++++++++++++++ internal/investigate/loop.go | 8 ++ internal/investigate/tools.go | 54 +++++++++++- internal/notify/slack.go | 25 +++++- internal/notify/slack_test.go | 42 +++++++++ 5 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 internal/investigate/findings_contract_test.go diff --git a/internal/investigate/findings_contract_test.go b/internal/investigate/findings_contract_test.go new file mode 100644 index 00000000..005dfb44 --- /dev/null +++ b/internal/investigate/findings_contract_test.go @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: Apache-2.0 + +package investigate + +import ( + "testing" + + "github.com/Smana/runlore/internal/providers" +) + +// TestActionWithoutRemedy pins the other half of the shape contract: a verdict that +// tells the on-call to act must ship something to act on. +// +// Live card, 2026-08-24 02:18: header "๐Ÿ›  Action suggested", body ending on +// "โ€ฆ1 more hypothesis below", and no next-steps section at all โ€” because +// suggested_action and actions are both optional in the schema, so the payload was +// legal. A card that promises an action and has none costs more trust than a card +// that says inconclusive. +func TestActionWithoutRemedy(t *testing.T) { + cause := func(action string) []providers.Hypothesis { + return []providers.Hypothesis{{Summary: "s", SuggestedAction: action}} + } + cases := []struct { + name string + inv providers.Investigation + want bool + }{ + {"action_suggested with no remedy anywhere", + providers.Investigation{Verdict: providers.VerdictActionSuggested, RootCauses: cause("")}, true}, + {"action_required with no remedy anywhere", + providers.Investigation{Verdict: providers.VerdictActionRequired, RootCauses: cause("")}, true}, + {"a suggested_action on the root cause accounts for it", + providers.Investigation{Verdict: providers.VerdictActionSuggested, RootCauses: cause("delete the stale Job")}, false}, + {"a proposed action accounts for it", + providers.Investigation{Verdict: providers.VerdictActionRequired, RootCauses: cause(""), + Actions: []providers.Action{{Description: "restore Drive access"}}}, false}, + {"no_action is not claiming an action", + providers.Investigation{Verdict: providers.VerdictNoAction, RootCauses: cause("")}, false}, + {"inconclusive is the other predicate's business", + providers.Investigation{Verdict: providers.VerdictInconclusive, RootCauses: cause("")}, false}, + {"an omitted verdict is a parse concern, not this one", + providers.Investigation{RootCauses: cause("")}, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := actionWithoutRemedy(c.inv); got != c.want { + t.Fatalf("actionWithoutRemedy = %v, want %v", got, c.want) + } + }) + } +} + +// TestUnevidencedConclusion: a conclusive verdict whose leading root cause cites no +// evidence at all. Live card, 2026-08-22 22:53 โ€” "High confidence ยท 85%" with a Why +// paragraph and not one bullet under it. The prose may still be right, but nothing +// in the card lets a human check it, and nothing lets the verify pass trace it. +func TestUnevidencedConclusion(t *testing.T) { + cases := []struct { + name string + inv providers.Investigation + want bool + }{ + {"conclusive, leading cause cites nothing", + providers.Investigation{Verdict: providers.VerdictActionSuggested, + RootCauses: []providers.Hypothesis{{Summary: "stale Job never deleted"}}}, true}, + {"evidence on the leading cause accounts for it", + providers.Investigation{Verdict: providers.VerdictActionSuggested, + RootCauses: []providers.Hypothesis{{Summary: "s", Evidence: []string{"kube_job_failed=1"}}}}, false}, + {"no_action still has to show its work", + providers.Investigation{Verdict: providers.VerdictNoAction, + RootCauses: []providers.Hypothesis{{Summary: "self-healed"}}}, true}, + {"inconclusive is allowed to have nothing", + providers.Investigation{Verdict: providers.VerdictInconclusive, + RootCauses: []providers.Hypothesis{{Summary: "s"}}}, false}, + {"no root cause at all is the inconclusive predicate's business", + providers.Investigation{Verdict: providers.VerdictActionSuggested}, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := unevidencedConclusion(c.inv); got != c.want { + t.Fatalf("unevidencedConclusion = %v, want %v", got, c.want) + } + }) + } +} diff --git a/internal/investigate/loop.go b/internal/investigate/loop.go index bce23181..f451a3ef 100644 --- a/internal/investigate/loop.go +++ b/internal/investigate/loop.go @@ -757,6 +757,14 @@ func (li *LoopInvestigator) Investigate(ctx context.Context, req Request) error li.Log.Warn("submit_findings: verdict=inconclusive with no cause, no open question and no data gap โ€” the delivered card will have no Why and no next steps", "title", inv.Title, "trigger_key", req.TriggerKey, "confidence", inv.Confidence, "tools_used", used) } + if actionWithoutRemedy(inv) { + li.Log.Warn("submit_findings: verdict claims an action but no suggested_action or action was supplied โ€” the card's header promises a remedy its body does not carry", + "title", inv.Title, "trigger_key", req.TriggerKey, "verdict", inv.Verdict, "confidence", inv.Confidence, "tools_used", used) + } + if unevidencedConclusion(inv) { + li.Log.Warn("submit_findings: conclusive verdict whose leading root cause cites no evidence โ€” the confidence badge is backed by nothing the reader or the verify pass can trace", + "title", inv.Title, "trigger_key", req.TriggerKey, "verdict", inv.Verdict, "confidence", inv.Confidence, "tools_used", used) + } if li.Metrics != nil { // Usage-anchored when the provider reported usage; heuristic otherwise. li.Metrics.InvestigationTokens.Record(ctx, int64(calib.estimate(sys, messages, specs))) diff --git a/internal/investigate/tools.go b/internal/investigate/tools.go index dc4e5a4e..5138ab59 100644 --- a/internal/investigate/tools.go +++ b/internal/investigate/tools.go @@ -76,11 +76,11 @@ func submitFindingsSpec() providers.ToolSpec { "confidence":{"type":"number"}, "affected_resource":{"type":"object","description":"the workload your investigation identified as the failing/affected resource","properties":{"kind":{"type":"string"},"name":{"type":"string"},"namespace":{"type":"string"}}}, "root_causes":{"type":"array","items":{"type":"object","properties":{ -"summary":{"type":"string"},"confidence":{"type":"number","description":"how strongly the evidence supports THIS root cause, 0-1. Required: an omitted confidence is delivered as 0%, which reads to the on-call as 'no confidence' and buries a sound finding under a red badge"},"change_ref":{"type":"string"}, -"evidence":{"type":"array","items":{"type":"string"}},"suggested_action":{"type":"string"},"reversible":{"type":"boolean"}}, -"required":["summary","confidence"]}}, +"summary":{"type":"string"},"confidence":{"type":"number","description":"how strongly the evidence below supports THIS root cause, 0-1. It measures support for the cause you STATED - not how sure you are of the narrative around it, and not how confident you are that you could not find something. Required: an omitted confidence is delivered as 0%, which reads to the on-call as 'no confidence' and buries a sound finding under a red badge"},"change_ref":{"type":"string"}, +"evidence":{"type":"array","minItems":1,"items":{"type":"string"},"description":"REQUIRED, at least one: the specific tool results that support this cause - name the tool and quote the value, error or log line it returned. This is what lets a human check the cause and what the verify pass traces. A cause asserted with no evidence is delivered as a bare paragraph under a confidence badge nothing backs"},"suggested_action":{"type":"string","description":"what the human should DO about this cause. Required whenever the verdict is action_suggested or action_required, unless a top-level actions entry already covers it - a card whose verdict promises an action and then offers none is worse than one that admits it is inconclusive"},"reversible":{"type":"boolean"}}, +"required":["summary","confidence","evidence"]}}, "unresolved":{"type":"array","items":{"type":"string"},"description":"genuine open questions only a human can answer - put tool or data limitations in data_gaps instead"}, -"verdict":{"type":"string","enum":["no_action","action_suggested","action_required","inconclusive"],"description":"actionability for the on-call: no_action (benign/self-healed/synthetic), action_suggested (a human should follow the next steps), action_required (live impact needing prompt action), inconclusive (you genuinely could not determine the cause, and data_gaps/unresolved say what blocked you). A recurrence of a fault you DID identify is NOT inconclusive - naming a known cause again is still a conclusion, so restate it with the actionability verdict it deserves"}, +"verdict":{"type":"string","enum":["no_action","action_suggested","action_required","inconclusive"],"description":"actionability for the on-call: no_action (benign/self-healed/synthetic), action_suggested (a human should follow the next steps), action_required (live impact needing prompt action), inconclusive (you genuinely could not determine the cause, and data_gaps/unresolved say what blocked you). A recurrence of a fault you DID identify is NOT inconclusive - naming a known cause again is still a conclusion, so restate it with the actionability verdict it deserves. The converse also holds: if you could not name the failing resource or the cause THIS time, the verdict is inconclusive however sure you are of everything around it - do not report a high-confidence action_suggested whose actual finding is that you could not identify what failed"}, "ruled_out":{"type":"array","items":{"type":"string"},"description":"hypotheses you considered and REJECTED, one line each naming the disproving evidence"}, "data_gaps":{"type":"array","items":{"type":"string"},"description":"signals you could not obtain (tool errors, RBAC denials, truncated output) that limited the investigation - data limitations, NOT questions for a human. State the tool you called and the ACTUAL error it returned. Do NOT speculate about a cause you did not verify: if a metrics query returned nothing, call discover_metrics before claiming a metric or its name is absent; if a logs query returned nothing, call discover_log_fields; if a tool was refused, report the refusal rather than guessing why. An empty result is not evidence that the data does not exist."}, "actions":{"type":"array","description":"proposed remediations; prefer reversible, low-blast-radius","items":{"type":"object","properties":{ @@ -210,6 +210,52 @@ func unaccountedInconclusive(inv providers.Investigation) bool { return inv.UnaccountedInconclusive() } +// actionWithoutRemedy reports whether a submitted finding tells the on-call to act +// and then gives them nothing to do: a verdict of action_suggested/action_required +// with no suggested_action on any root cause and no proposed action. +// +// Both fields are optional in the schema, so this payload is legal, and it shipped +// live on 2026-08-24 โ€” a card headed "Action suggested" whose next-steps section was +// simply absent. The verdict badge is rendered from the verdict alone, so the header +// promises a remedy the body never carries; a card that does that costs more trust +// than one that says inconclusive. +// +// Deliberately NOT an error, for the same reason as unaccountedInconclusive: the +// analysis is still worth delivering, and re-asking costs a model call to re-answer +// a question the model just answered badly. The renderer says so on the card instead. +func actionWithoutRemedy(inv providers.Investigation) bool { + if inv.Verdict != providers.VerdictActionSuggested && inv.Verdict != providers.VerdictActionRequired { + return false + } + for _, rc := range inv.RootCauses { + if strings.TrimSpace(rc.SuggestedAction) != "" { + return false + } + } + for _, a := range inv.Actions { + if strings.TrimSpace(a.Description) != "" { + return false + } + } + return true +} + +// unevidencedConclusion reports whether a finding reaches a conclusive verdict while +// its leading root cause cites no evidence at all. +// +// Live on 2026-08-22: "High confidence ยท 85%" over a Why paragraph with not one +// bullet beneath it. The prose may well have been right, but nothing on the card let +// a human check it and nothing let the verify pass trace it โ€” the confidence badge +// was backed by the model's say-so alone. `inconclusive` is exempt: having nothing +// to show is what that verdict is for. So is an empty root-cause list, which is +// unaccountedInconclusive's business. +func unevidencedConclusion(inv providers.Investigation) bool { + if inv.Verdict == providers.VerdictInconclusive || len(inv.RootCauses) == 0 { + return false + } + return len(inv.RootCauses[0].Evidence) == 0 +} + // buildInvestigation maps the parsed findings shape onto a providers.Investigation, // clamping confidences. Shared by the direct and the tolerant parse paths. func buildInvestigation(f findings) providers.Investigation { diff --git a/internal/notify/slack.go b/internal/notify/slack.go index 402a02c3..0599a440 100644 --- a/internal/notify/slack.go +++ b/internal/notify/slack.go @@ -670,7 +670,23 @@ func summaryBlocks(inv providers.Investigation) []map[string]any { // 6. Suggested next steps โ€” the resolution guide (per-root-cause suggestions + // policy actions, de-duplicated, reversibility-flagged), capped at three. - if steps := nextSteps(inv); len(steps) > 0 { + // + // When the verdict claims an action and there is no remedy to show, the section + // is rendered anyway, saying that. suggested_action and actions are both optional + // in submit_findings, so the payload is legal, and it shipped live on 2026-08-24: + // a card headed "Action suggested" whose next-steps section was simply missing, + // leaving the reader unable to tell a failed render from an absent remedy. The + // verdict badge (2, above) is drawn from the verdict alone and would otherwise go + // on promising something the body never carries. + steps := nextSteps(inv) + if len(steps) == 0 && claimsAction(inv.Verdict) { + blocks = append(blocks, + map[string]any{"type": "divider"}, + map[string]any{"type": "section", "text": map[string]any{"type": "mrkdwn", + "text": "*๐Ÿ›  No next steps proposed* โ€” the verdict says a human should act, but the investigation " + + "did not supply a remedy. Work from the Why above and any data gaps in the thread."}}) + } + if len(steps) > 0 { var s strings.Builder s.WriteString("*๐Ÿ›  Suggested next steps* _(read-only โ€” RunLore won't apply these)_") // Own loop, not appendCappedBullets: nextSteps has already escaped these @@ -1029,6 +1045,13 @@ func nextSteps(inv providers.Investigation) []string { return steps } +// claimsAction reports whether a verdict tells the on-call to do something, and so +// obliges the card to show what. no_action and inconclusive make no such promise, +// and an absent verdict is a parse concern rather than a contract breach. +func claimsAction(v providers.Verdict) bool { + return v == providers.VerdictActionSuggested || v == providers.VerdictActionRequired +} + // truncate caps a string to n runes, appending an ellipsis when cut (Slack section // text is limited to 3000 chars). func truncate(s string, n int) string { diff --git a/internal/notify/slack_test.go b/internal/notify/slack_test.go index 9b7e3198..61f43c09 100644 --- a/internal/notify/slack_test.go +++ b/internal/notify/slack_test.go @@ -1752,3 +1752,45 @@ func TestMultiDeliverKBUpdateNilAndEmptyAreNoOps(t *testing.T) { t.Fatalf("empty Multi: got %v, want nil", err) } } + +// TestSlackActionVerdictWithoutStepsSaysSo: when the verdict tells the on-call to +// act but the investigation supplied no remedy, the card must say that outright +// rather than silently dropping the section. +// +// Live card, 2026-08-24 02:18: header "๐Ÿ›  Action suggested", and the next-steps +// section simply absent โ€” the reader is left to wonder whether the steps failed to +// render or were never produced. Naming the gap is the honest option, and it keeps +// the actionability signal (a human should still look) without promising a remedy +// that does not exist. +func TestSlackActionVerdictWithoutStepsSaysSo(t *testing.T) { + inv := providers.Investigation{ + Title: "BackupJobFailed โ€” failing instance unidentified", + Verdict: providers.VerdictActionSuggested, + RootCauses: []providers.Hypothesis{{Summary: "the failing RDS resource could not be identified", Confidence: 0.85}}, + } + txt := blocksText(t, summaryBlocks(inv)) + if !strings.Contains(txt, "No next steps") { + t.Errorf("a verdict claiming an action with no remedy must say so on the card:\n%s", txt) + } + + // Control: a card that DOES carry a remedy renders the normal section and never + // the disclaimer. + inv.RootCauses[0].SuggestedAction = "delete the stale Job object" + txt = blocksText(t, summaryBlocks(inv)) + if !strings.Contains(txt, "Suggested next steps") || !strings.Contains(txt, "delete the stale Job object") { + t.Errorf("a supplied remedy must render normally:\n%s", txt) + } + if strings.Contains(txt, "No next steps") { + t.Errorf("the disclaimer must not appear when steps exist:\n%s", txt) + } + + // Control: no_action never claims a remedy, so it never gets the disclaimer. + quiet := providers.Investigation{ + Title: "self-healed", + Verdict: providers.VerdictNoAction, + RootCauses: []providers.Hypothesis{{Summary: "transient", Confidence: 0.9}}, + } + if txt := blocksText(t, summaryBlocks(quiet)); strings.Contains(txt, "No next steps") { + t.Errorf("no_action must not carry the disclaimer:\n%s", txt) + } +} From a71c4d190618e6d9d44a61749449071563d1c03c Mon Sep 17 00:00:00 2001 From: Smaine Kahlouch Date: Mon, 24 Aug 2026 15:23:09 +0200 Subject: [PATCH 2/3] fix(notify): gate the no-remedy notice on what the card actually shows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the first revision found the notice rendering off the raw contract predicate, which is the one thing format.go says not to do: "the two must not be the same test". The contract predicate answers "did the MODEL contradict itself" and the loop logs it pre-verify; the renderer has to answer "is there anything the READER can use". The shipped golden fixtures proved it. seen_before quotes a human-reviewed resolution from the KB entry; matched_runbook carries a matched entry and its URL. Both hold an action verdict with no suggested_action, so both printed "the investigation did not supply a remedy" a few blocks below a remedy that was on screen โ€” pointing the on-call away from the answer. - ActionWithoutRemedy and Verdict.ClaimsAction move onto the contract, next to UnaccountedInconclusive, for the reason stated there: two sides consult them and must not drift. - notify.remedyMissingForReader is the reader-side counterpart, and additionally requires that the card show no other remedy. - Format renders the notice too. It feeds Matrix, the webhook text and the CLI, and its doc comment says its claims must never diverge from the card; gating only the Slack block left those channels unfixed. - nextSteps trims, so a whitespace-only suggested_action can no longer suppress the notice while rendering an empty bullet. - unevidencedConclusion tests Verdict.Conclusive() rather than "!= inconclusive", so an unparseable verdict is a parse concern rather than a warning about a badge that is never drawn, and counts non-blank evidence, so minItems:1 cannot be satisfied with "". TestSubmitFindingsRequiresEvidence guards the model-facing half, which has no compiler; mutation-tested against dropping minItems and dropping evidence from required. --- .../investigate/findings_contract_test.go | 54 ++++++++++++- internal/investigate/loop.go | 2 +- internal/investigate/tools.go | 45 ++++------- internal/notify/format.go | 42 ++++++++++ internal/notify/slack.go | 17 ++-- internal/notify/slack_test.go | 78 +++++++++++++++++++ internal/providers/providers.go | 47 +++++++++++ 7 files changed, 240 insertions(+), 45 deletions(-) diff --git a/internal/investigate/findings_contract_test.go b/internal/investigate/findings_contract_test.go index 005dfb44..22753a35 100644 --- a/internal/investigate/findings_contract_test.go +++ b/internal/investigate/findings_contract_test.go @@ -3,6 +3,9 @@ package investigate import ( + "encoding/json" + "slices" + "strings" "testing" "github.com/Smana/runlore/internal/providers" @@ -43,8 +46,8 @@ func TestActionWithoutRemedy(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - if got := actionWithoutRemedy(c.inv); got != c.want { - t.Fatalf("actionWithoutRemedy = %v, want %v", got, c.want) + if got := c.inv.ActionWithoutRemedy(); got != c.want { + t.Fatalf("ActionWithoutRemedy = %v, want %v", got, c.want) } }) } @@ -83,3 +86,50 @@ func TestUnevidencedConclusion(t *testing.T) { }) } } + +// TestSubmitFindingsRequiresEvidence guards the model-facing half of this fix, which +// has no compiler. The predicates above test Go functions; the schema is a JSON +// string literal, so a future edit that reflows or re-wraps that line can silently +// revert `evidence` to optional while every other test in this file keeps passing. +// Same reasoning as TestDataGapsForbidsSpeculation. +func TestSubmitFindingsRequiresEvidence(t *testing.T) { + var schema struct { + Properties struct { + RootCauses struct { + Items struct { + Properties map[string]struct { + MinItems int `json:"minItems"` + Description string `json:"description"` + } `json:"properties"` + Required []string `json:"required"` + } `json:"items"` + } `json:"root_causes"` + Verdict struct { + Description string `json:"description"` + } `json:"verdict"` + } `json:"properties"` + } + if err := json.Unmarshal([]byte(submitFindingsSpec().Schema), &schema); err != nil { + t.Fatalf("submit_findings schema is not valid JSON: %v", err) + } + rc := schema.Properties.RootCauses.Items + + if !slices.Contains(rc.Required, "evidence") { + t.Errorf("a root cause must REQUIRE evidence; required = %v", rc.Required) + } + if got := rc.Properties["evidence"].MinItems; got != 1 { + t.Errorf("evidence minItems = %d, want 1 โ€” an empty array satisfies a bare `required`", got) + } + // The conditional requirement that pairs a remedy with a verdict claiming one. + // ActionWithoutRemedy only WARNS, so this sentence is the only thing that stops + // the payload being produced in the first place. + if d := rc.Properties["suggested_action"].Description; !strings.Contains(d, "action_required") { + t.Errorf("suggested_action must say when it is required:\n%s", d) + } + // The converse of the inconclusive rule: high confidence in "I could not tell" + // is still inconclusive. Live 2026-08-24, an 85%% action_suggested whose finding + // was that it could not identify the failing resource. + if d := schema.Properties.Verdict.Description; !strings.Contains(d, "could not name the failing resource") { + t.Errorf("verdict must state the converse of the recurrence rule:\n%s", d) + } +} diff --git a/internal/investigate/loop.go b/internal/investigate/loop.go index f451a3ef..eb106adc 100644 --- a/internal/investigate/loop.go +++ b/internal/investigate/loop.go @@ -757,7 +757,7 @@ func (li *LoopInvestigator) Investigate(ctx context.Context, req Request) error li.Log.Warn("submit_findings: verdict=inconclusive with no cause, no open question and no data gap โ€” the delivered card will have no Why and no next steps", "title", inv.Title, "trigger_key", req.TriggerKey, "confidence", inv.Confidence, "tools_used", used) } - if actionWithoutRemedy(inv) { + if inv.ActionWithoutRemedy() { li.Log.Warn("submit_findings: verdict claims an action but no suggested_action or action was supplied โ€” the card's header promises a remedy its body does not carry", "title", inv.Title, "trigger_key", req.TriggerKey, "verdict", inv.Verdict, "confidence", inv.Confidence, "tools_used", used) } diff --git a/internal/investigate/tools.go b/internal/investigate/tools.go index 5138ab59..ef6e2605 100644 --- a/internal/investigate/tools.go +++ b/internal/investigate/tools.go @@ -210,36 +210,6 @@ func unaccountedInconclusive(inv providers.Investigation) bool { return inv.UnaccountedInconclusive() } -// actionWithoutRemedy reports whether a submitted finding tells the on-call to act -// and then gives them nothing to do: a verdict of action_suggested/action_required -// with no suggested_action on any root cause and no proposed action. -// -// Both fields are optional in the schema, so this payload is legal, and it shipped -// live on 2026-08-24 โ€” a card headed "Action suggested" whose next-steps section was -// simply absent. The verdict badge is rendered from the verdict alone, so the header -// promises a remedy the body never carries; a card that does that costs more trust -// than one that says inconclusive. -// -// Deliberately NOT an error, for the same reason as unaccountedInconclusive: the -// analysis is still worth delivering, and re-asking costs a model call to re-answer -// a question the model just answered badly. The renderer says so on the card instead. -func actionWithoutRemedy(inv providers.Investigation) bool { - if inv.Verdict != providers.VerdictActionSuggested && inv.Verdict != providers.VerdictActionRequired { - return false - } - for _, rc := range inv.RootCauses { - if strings.TrimSpace(rc.SuggestedAction) != "" { - return false - } - } - for _, a := range inv.Actions { - if strings.TrimSpace(a.Description) != "" { - return false - } - } - return true -} - // unevidencedConclusion reports whether a finding reaches a conclusive verdict while // its leading root cause cites no evidence at all. // @@ -250,10 +220,21 @@ func actionWithoutRemedy(inv providers.Investigation) bool { // to show is what that verdict is for. So is an empty root-cause list, which is // unaccountedInconclusive's business. func unevidencedConclusion(inv providers.Investigation) bool { - if inv.Verdict == providers.VerdictInconclusive || len(inv.RootCauses) == 0 { + // Conclusive() rather than "!= inconclusive": an omitted or unparseable verdict + // leaves inv.Verdict empty, which renders no badge at all, so there is no claim + // to hold to account. Testing it the other way warned about a badge never drawn. + if !inv.Verdict.Conclusive() || len(inv.RootCauses) == 0 { return false } - return len(inv.RootCauses[0].Evidence) == 0 + // Non-blank, because the schema's minItems:1 counts an empty string. Requiring a + // present-but-empty array to silence the warning would make the cheapest possible + // non-answer the way to pass, and the card then renders a bare bullet. + for _, e := range inv.RootCauses[0].Evidence { + if strings.TrimSpace(e) != "" { + return false + } + } + return true } // buildInvestigation maps the parsed findings shape onto a providers.Investigation, diff --git a/internal/notify/format.go b/internal/notify/format.go index 9341c7f2..7b38823a 100644 --- a/internal/notify/format.go +++ b/internal/notify/format.go @@ -176,6 +176,40 @@ func unaccountedForReader(inv providers.Investigation) bool { return inv.UnaccountedInconclusive() && len(inv.RuledOut) == 0 } +// noRemedyNotice is what a delivered notification says when its verdict tells the +// on-call to act and it has nothing to show them. Naming the gap keeps the +// actionability signal โ€” a human should still look โ€” without promising a remedy +// that does not exist, and lets the reader tell an ABSENT remedy from a failed +// render, which is what the live 2026-08-24 card could not do. +const noRemedyNotice = "*๐Ÿ›  No next steps proposed* โ€” the verdict says a human should act, but the " + + "investigation did not supply a remedy. Work from the analysis above and any data gaps in the thread." + +// remedyMissingForReader reports whether nothing the delivered notification can +// show accounts for a verdict that tells the on-call to act. +// +// It is the reader-side counterpart to providers.Investigation.ActionWithoutRemedy, +// split for the same reason as unaccountedForReader above: the contract predicate +// answers "did the MODEL contradict itself", which the loop logs at submit time and +// pre-verify; this answers the later, different question "is there anything on the +// delivered notification the reader can use". The two must not be the same test. +// +// The difference is load-bearing, and the shipped card fixtures prove it. Both +// `seen_before` (a human-reviewed resolution quoted from the KB entry) and +// `matched_runbook` (a matched entry with its URL) carry an action verdict and no +// suggested_action of their own โ€” and both render a remedy the reader can follow, +// several blocks ABOVE where this notice would print. Announcing that the +// investigation supplied no remedy there would point the on-call away from an +// answer that is on screen. +func remedyMissingForReader(inv providers.Investigation) bool { + if !inv.ActionWithoutRemedy() { + return false + } + if p := inv.Prior; p != nil && strings.TrimSpace(p.Resolution) != "" { + return false + } + return inv.MatchedKnowledge == nil +} + // verdictBadge maps a model verdict to its emoji + human label. Empty/unknown // verdicts return ("", "") and are rendered nowhere โ€” never invent a verdict. func verdictBadge(v providers.Verdict) (emoji, label string) { @@ -323,6 +357,14 @@ func Format(inv providers.Investigation) string { if unaccountedForReader(inv) { b.WriteString(unaccountedInconclusiveNotice + "\n") } + // The action-verdict counterpart, and it renders HERE for the reason Format's doc + // comment gives: this message feeds Matrix, the webhook text and the CLI, and its + // ordering "must never diverge" from the Slack card in what it claims. Gating the + // Slack block and not this one would have left every non-Slack channel showing the + // 2026-08-24 defect unfixed. + if remedyMissingForReader(inv) { + b.WriteString(noRemedyNotice + "\n") + } if len(inv.Unresolved) > 0 { b.WriteString("*Unresolved:*\n") for _, u := range inv.Unresolved { diff --git a/internal/notify/slack.go b/internal/notify/slack.go index 0599a440..3da659e2 100644 --- a/internal/notify/slack.go +++ b/internal/notify/slack.go @@ -679,12 +679,11 @@ func summaryBlocks(inv providers.Investigation) []map[string]any { // verdict badge (2, above) is drawn from the verdict alone and would otherwise go // on promising something the body never carries. steps := nextSteps(inv) - if len(steps) == 0 && claimsAction(inv.Verdict) { + if len(steps) == 0 && remedyMissingForReader(inv) { blocks = append(blocks, map[string]any{"type": "divider"}, map[string]any{"type": "section", "text": map[string]any{"type": "mrkdwn", - "text": "*๐Ÿ›  No next steps proposed* โ€” the verdict says a human should act, but the investigation " + - "did not supply a remedy. Work from the Why above and any data gaps in the thread."}}) + "text": noRemedyNotice}}) } if len(steps) > 0 { var s strings.Builder @@ -1026,6 +1025,11 @@ func nextSteps(inv providers.Investigation) []string { var steps []string seen := map[string]bool{} add := func(desc string, reversible bool) { + // Trimmed, so this agrees with providers.Investigation.ActionWithoutRemedy, + // which trims too. Gating on desc == "" instead let a whitespace-only + // suggested_action produce a step, suppressing the no-remedy notice while + // rendering an empty bullet โ€” the promised remedy, spelled as one space. + desc = strings.TrimSpace(desc) if desc == "" || seen[desc] { return } @@ -1045,13 +1049,6 @@ func nextSteps(inv providers.Investigation) []string { return steps } -// claimsAction reports whether a verdict tells the on-call to do something, and so -// obliges the card to show what. no_action and inconclusive make no such promise, -// and an absent verdict is a parse concern rather than a contract breach. -func claimsAction(v providers.Verdict) bool { - return v == providers.VerdictActionSuggested || v == providers.VerdictActionRequired -} - // truncate caps a string to n runes, appending an ellipsis when cut (Slack section // text is limited to 3000 chars). func truncate(s string, n int) string { diff --git a/internal/notify/slack_test.go b/internal/notify/slack_test.go index 61f43c09..01c20cd0 100644 --- a/internal/notify/slack_test.go +++ b/internal/notify/slack_test.go @@ -1794,3 +1794,81 @@ func TestSlackActionVerdictWithoutStepsSaysSo(t *testing.T) { t.Errorf("no_action must not carry the disclaimer:\n%s", txt) } } + +// TestNoRemedyNoticeRespectsWhatTheCardShows pins the split between the contract +// predicate and the reader-side one. The contract asks "did the model supply a +// remedy"; the card must ask "does the reader have one in front of them", and the +// two differ on the shapes the shipped golden fixtures already cover. +// +// Without this split the notice printed on `seen_before` and `matched_runbook` โ€” +// telling the on-call the investigation supplied no remedy directly below a +// human-reviewed resolution, or a matched runbook with its URL. +func TestNoRemedyNoticeRespectsWhatTheCardShows(t *testing.T) { + base := func() providers.Investigation { + return providers.Investigation{ + Title: "harbor-db crash-looping again", + Verdict: providers.VerdictActionSuggested, + RootCauses: []providers.Hypothesis{{Summary: "the same migration lock", Confidence: 0.6}}, + } + } + cases := []struct { + name string + mutate func(*providers.Investigation) + wantNotice bool + }{ + {"nothing to act on at all", func(*providers.Investigation) {}, true}, + {"a validated prior resolution is a remedy", func(inv *providers.Investigation) { + inv.Prior = &providers.PriorKnowledge{Resolution: "roll back to 1.14.2, clear the lock"} + }, false}, + {"a whitespace-only prior resolution is not", func(inv *providers.Investigation) { + inv.Prior = &providers.PriorKnowledge{Resolution: " "} + }, true}, + {"a matched runbook is a remedy", func(inv *providers.Investigation) { + inv.MatchedKnowledge = &providers.MatchedEntry{ + Path: "entries/x.md", Title: "Registry credentials deleted", Score: 6.4} + }, false}, + {"the model's own suggested_action is a remedy", func(inv *providers.Investigation) { + inv.RootCauses[0].SuggestedAction = "clear the migration lock" + }, false}, + {"a whitespace-only suggested_action is not, and renders no empty bullet", func(inv *providers.Investigation) { + inv.RootCauses[0].SuggestedAction = " " + }, true}, + {"no_action promises nothing", func(inv *providers.Investigation) { + inv.Verdict = providers.VerdictNoAction + }, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + inv := base() + c.mutate(&inv) + txt := blocksText(t, summaryBlocks(inv)) + if got := strings.Contains(txt, "No next steps proposed"); got != c.wantNotice { + t.Errorf("notice rendered = %v, want %v:\n%s", got, c.wantNotice, txt) + } + // Whichever way it went, the card must never show a bullet with nothing in it. + if strings.Contains(txt, "1. \n") || strings.Contains(txt, "1. _(reversible)_") { + t.Errorf("rendered an empty next-step bullet:\n%s", txt) + } + }) + } +} + +// TestFormatCarriesTheNoRemedyNotice: Format feeds Matrix, the webhook text and the +// CLI, and its doc comment says its claims "must never diverge" from the Slack card. +// Gating only the Slack block would have left every non-Slack channel showing the +// 2026-08-24 defect unfixed. +func TestFormatCarriesTheNoRemedyNotice(t *testing.T) { + inv := providers.Investigation{ + Title: "BackupJobFailed", + Verdict: providers.VerdictActionRequired, + RootCauses: []providers.Hypothesis{{Summary: "the failing RDS resource could not be identified"}}, + } + if out := Format(inv); !strings.Contains(out, "No next steps proposed") { + t.Errorf("Format must say the verdict promises a remedy it does not carry:\n%s", out) + } + // And it must not, when the reader has something to act on. + inv.RootCauses[0].SuggestedAction = "delete the stale snapshot" + if out := Format(inv); strings.Contains(out, "No next steps proposed") { + t.Errorf("Format printed the notice over a supplied remedy:\n%s", out) + } +} diff --git a/internal/providers/providers.go b/internal/providers/providers.go index ca158527..cafb08d4 100644 --- a/internal/providers/providers.go +++ b/internal/providers/providers.go @@ -1585,6 +1585,53 @@ func (inv Investigation) UnaccountedInconclusive() bool { len(inv.RootCauses) == 0 && len(inv.Unresolved) == 0 && len(inv.DataGaps) == 0 } +// ActionWithoutRemedy reports whether inv tells the on-call to act and then gives +// them nothing to do: a verdict of action_suggested/action_required with no +// suggested_action on any root cause and no proposed action. +// +// Both fields are optional in submit_findings, so this payload is legal, and it +// shipped live on 2026-08-24 โ€” a card headed "Action suggested" whose next-steps +// section was simply absent. The verdict badge is rendered from the verdict alone, +// so the header promises a remedy the body never carries; a card that does that +// costs more trust than one that says inconclusive. +// +// It lives HERE for the same reason as UnaccountedInconclusive: two sides consult +// it and must not disagree. The loop says it out loud at the source, where the +// payload is still attributable to the model call that made it; the notifier acts +// on it at delivery. One definition, so a fifth verdict is one switch to update +// rather than two adjacent ones. +// +// And, exactly as there, one definition is NOT one snapshot. The loop evaluates +// this pre-verify; the notifier evaluates it on the post-verify result, so a run +// whose reviewer rejected the one cause that carried the remedy legitimately +// disagrees between them. The notifier also does not render straight off this +// predicate โ€” see notify.remedyMissingForReader, which additionally requires that +// the card show no OTHER remedy (a validated prior resolution, a matched runbook). +func (inv Investigation) ActionWithoutRemedy() bool { + if !inv.Verdict.ClaimsAction() { + return false + } + for _, rc := range inv.RootCauses { + if strings.TrimSpace(rc.SuggestedAction) != "" { + return false + } + } + for _, a := range inv.Actions { + if strings.TrimSpace(a.Description) != "" { + return false + } + } + return true +} + +// ClaimsAction reports whether a verdict tells the on-call to do something, and so +// obliges the delivered notification to show what. no_action and inconclusive make +// no such promise, and an empty/unknown verdict is a parse concern rather than a +// contract breach โ€” it renders no badge at all, so it promises nothing. +func (v Verdict) ClaimsAction() bool { + return v == VerdictActionSuggested || v == VerdictActionRequired +} + // MatchedEntry is the strongest pre-existing catalog entry an investigation's // kb_search calls matched at clear-match strength. It closes a live visibility gap: // when a full investigation's kb_search found a known runbook and used it, the From 23c6b84a7a1a28de96c4709c05b96925feaa6a7a Mon Sep 17 00:00:00 2001 From: Smaine Kahlouch Date: Mon, 24 Aug 2026 16:00:20 +0200 Subject: [PATCH 3/3] refactor(investigate): state the shared argument once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cleanup pass on the same change. ClaimsAction is defined off Verdict.Conclusive now, for the reason ValidVerdict states directly above it: the three actionability verdicts stay enumerated once, so a fifth is one switch to update rather than two adjacent ones. The predicate's own doc claimed that property while giving it up. The Slack gate drops its `len(steps) == 0 &&` conjunct. It was implied โ€” ActionWithoutRemedy is false as soon as any root cause or action carries a non-blank remedy, and nextSteps builds its list from exactly those fields, trimmed the same way โ€” so it was either dead or a divergence from Format's gate, and a reader could not tell which. The three contract warnings shared a five-attribute tail written three times, except the first, which omitted "verdict" for no stated reason. One closure now, and a fourth predicate is a line rather than a paragraph. Comment surgery, since the diff had begun restating one argument in three files: ActionWithoutRemedy and remedyMissingForReader keep the part that explains THEM (the live payload, and the fixtures that prove the reader-side split is load-bearing) and point at UnaccountedInconclusive for the placement and pre/post-verify reasoning rather than re-deriving it. The schema's root-cause properties go one per line, matching their top-level siblings, so a two-word prompt tweak shows up in blame as one changed line instead of the whole root-cause contract. The confidence description also stops restating the verdict rule stated 3 lines below, which was ~155 bytes re-sent on every turn. Tests: the card test the table already subsumed is folded into it, carrying across its one unique assertion, and the empty-bullet guard now matches the bullets the renderer actually emits ("โ€ข ", not "1. "). --- internal/investigate/loop.go | 14 ++++--- internal/investigate/tools.go | 8 +++- internal/notify/format.go | 5 +-- internal/notify/slack.go | 15 ++++---- internal/notify/slack_test.go | 68 +++++++++------------------------ internal/providers/providers.go | 23 +++++------ 6 files changed, 49 insertions(+), 84 deletions(-) diff --git a/internal/investigate/loop.go b/internal/investigate/loop.go index eb106adc..18e92f76 100644 --- a/internal/investigate/loop.go +++ b/internal/investigate/loop.go @@ -753,17 +753,19 @@ func (li *LoopInvestigator) Investigate(ctx context.Context, req Request) error // Say it out loud when the submission contradicts itself, BEFORE verify can // rewrite it: this is the model's own payload, and the mislabel that motivated // #471 was invisible until someone read the empty card it produced. + contractWarn := func(msg string) { + li.Log.Warn("submit_findings: "+msg, + "title", inv.Title, "trigger_key", req.TriggerKey, "verdict", inv.Verdict, + "confidence", inv.Confidence, "tools_used", used) + } if unaccountedInconclusive(inv) { - li.Log.Warn("submit_findings: verdict=inconclusive with no cause, no open question and no data gap โ€” the delivered card will have no Why and no next steps", - "title", inv.Title, "trigger_key", req.TriggerKey, "confidence", inv.Confidence, "tools_used", used) + contractWarn("verdict=inconclusive with no cause, no open question and no data gap โ€” the delivered card will have no Why and no next steps") } if inv.ActionWithoutRemedy() { - li.Log.Warn("submit_findings: verdict claims an action but no suggested_action or action was supplied โ€” the card's header promises a remedy its body does not carry", - "title", inv.Title, "trigger_key", req.TriggerKey, "verdict", inv.Verdict, "confidence", inv.Confidence, "tools_used", used) + contractWarn("verdict claims an action but no suggested_action or action was supplied โ€” the card's header promises a remedy its body does not carry") } if unevidencedConclusion(inv) { - li.Log.Warn("submit_findings: conclusive verdict whose leading root cause cites no evidence โ€” the confidence badge is backed by nothing the reader or the verify pass can trace", - "title", inv.Title, "trigger_key", req.TriggerKey, "verdict", inv.Verdict, "confidence", inv.Confidence, "tools_used", used) + contractWarn("conclusive verdict whose leading root cause cites no evidence โ€” the confidence badge is backed by nothing the reader or the verify pass can trace") } if li.Metrics != nil { // Usage-anchored when the provider reported usage; heuristic otherwise. diff --git a/internal/investigate/tools.go b/internal/investigate/tools.go index ef6e2605..a89ee391 100644 --- a/internal/investigate/tools.go +++ b/internal/investigate/tools.go @@ -76,8 +76,12 @@ func submitFindingsSpec() providers.ToolSpec { "confidence":{"type":"number"}, "affected_resource":{"type":"object","description":"the workload your investigation identified as the failing/affected resource","properties":{"kind":{"type":"string"},"name":{"type":"string"},"namespace":{"type":"string"}}}, "root_causes":{"type":"array","items":{"type":"object","properties":{ -"summary":{"type":"string"},"confidence":{"type":"number","description":"how strongly the evidence below supports THIS root cause, 0-1. It measures support for the cause you STATED - not how sure you are of the narrative around it, and not how confident you are that you could not find something. Required: an omitted confidence is delivered as 0%, which reads to the on-call as 'no confidence' and buries a sound finding under a red badge"},"change_ref":{"type":"string"}, -"evidence":{"type":"array","minItems":1,"items":{"type":"string"},"description":"REQUIRED, at least one: the specific tool results that support this cause - name the tool and quote the value, error or log line it returned. This is what lets a human check the cause and what the verify pass traces. A cause asserted with no evidence is delivered as a bare paragraph under a confidence badge nothing backs"},"suggested_action":{"type":"string","description":"what the human should DO about this cause. Required whenever the verdict is action_suggested or action_required, unless a top-level actions entry already covers it - a card whose verdict promises an action and then offers none is worse than one that admits it is inconclusive"},"reversible":{"type":"boolean"}}, +"summary":{"type":"string"}, +"confidence":{"type":"number","description":"how strongly the evidence below supports THIS root cause, 0-1. It measures support for the cause you STATED, not how sure you are of the narrative around it. Required: an omitted confidence is delivered as 0%, which reads to the on-call as 'no confidence' and buries a sound finding under a red badge"}, +"change_ref":{"type":"string"}, +"evidence":{"type":"array","minItems":1,"items":{"type":"string"},"description":"REQUIRED, at least one: the specific tool results that support this cause - name the tool and quote the value, error or log line it returned. This is what lets a human check the cause and what the verify pass traces. A cause asserted with no evidence is delivered as a bare paragraph under a confidence badge nothing backs"}, +"suggested_action":{"type":"string","description":"what the human should DO about this cause. Required whenever the verdict is action_suggested or action_required, unless a top-level actions entry already covers it - a card whose verdict promises an action and then offers none is worse than one that admits it is inconclusive"}, +"reversible":{"type":"boolean"}}, "required":["summary","confidence","evidence"]}}, "unresolved":{"type":"array","items":{"type":"string"},"description":"genuine open questions only a human can answer - put tool or data limitations in data_gaps instead"}, "verdict":{"type":"string","enum":["no_action","action_suggested","action_required","inconclusive"],"description":"actionability for the on-call: no_action (benign/self-healed/synthetic), action_suggested (a human should follow the next steps), action_required (live impact needing prompt action), inconclusive (you genuinely could not determine the cause, and data_gaps/unresolved say what blocked you). A recurrence of a fault you DID identify is NOT inconclusive - naming a known cause again is still a conclusion, so restate it with the actionability verdict it deserves. The converse also holds: if you could not name the failing resource or the cause THIS time, the verdict is inconclusive however sure you are of everything around it - do not report a high-confidence action_suggested whose actual finding is that you could not identify what failed"}, diff --git a/internal/notify/format.go b/internal/notify/format.go index 7b38823a..29ac01d8 100644 --- a/internal/notify/format.go +++ b/internal/notify/format.go @@ -188,10 +188,7 @@ const noRemedyNotice = "*๐Ÿ›  No next steps proposed* โ€” the verdict says a hum // show accounts for a verdict that tells the on-call to act. // // It is the reader-side counterpart to providers.Investigation.ActionWithoutRemedy, -// split for the same reason as unaccountedForReader above: the contract predicate -// answers "did the MODEL contradict itself", which the loop logs at submit time and -// pre-verify; this answers the later, different question "is there anything on the -// delivered notification the reader can use". The two must not be the same test. +// split for the same reason as unaccountedForReader above. // // The difference is load-bearing, and the shipped card fixtures prove it. Both // `seen_before` (a human-reviewed resolution quoted from the KB entry) and diff --git a/internal/notify/slack.go b/internal/notify/slack.go index 3da659e2..14fbdd65 100644 --- a/internal/notify/slack.go +++ b/internal/notify/slack.go @@ -671,15 +671,14 @@ func summaryBlocks(inv providers.Investigation) []map[string]any { // 6. Suggested next steps โ€” the resolution guide (per-root-cause suggestions + // policy actions, de-duplicated, reversibility-flagged), capped at three. // - // When the verdict claims an action and there is no remedy to show, the section - // is rendered anyway, saying that. suggested_action and actions are both optional - // in submit_findings, so the payload is legal, and it shipped live on 2026-08-24: - // a card headed "Action suggested" whose next-steps section was simply missing, - // leaving the reader unable to tell a failed render from an absent remedy. The - // verdict badge (2, above) is drawn from the verdict alone and would otherwise go - // on promising something the body never carries. + // Rendered even with nothing to show, saying so โ€” see + // providers.Investigation.ActionWithoutRemedy for the payload that motivated it, + // and notify.remedyMissingForReader for why the gate is not that predicate + // directly. The two branches are mutually exclusive: the predicate requires that + // no root cause and no action carried a remedy, and nextSteps builds its list + // from exactly those fields, trimmed the same way. steps := nextSteps(inv) - if len(steps) == 0 && remedyMissingForReader(inv) { + if remedyMissingForReader(inv) { blocks = append(blocks, map[string]any{"type": "divider"}, map[string]any{"type": "section", "text": map[string]any{"type": "mrkdwn", diff --git a/internal/notify/slack_test.go b/internal/notify/slack_test.go index 01c20cd0..d150182b 100644 --- a/internal/notify/slack_test.go +++ b/internal/notify/slack_test.go @@ -1753,48 +1753,6 @@ func TestMultiDeliverKBUpdateNilAndEmptyAreNoOps(t *testing.T) { } } -// TestSlackActionVerdictWithoutStepsSaysSo: when the verdict tells the on-call to -// act but the investigation supplied no remedy, the card must say that outright -// rather than silently dropping the section. -// -// Live card, 2026-08-24 02:18: header "๐Ÿ›  Action suggested", and the next-steps -// section simply absent โ€” the reader is left to wonder whether the steps failed to -// render or were never produced. Naming the gap is the honest option, and it keeps -// the actionability signal (a human should still look) without promising a remedy -// that does not exist. -func TestSlackActionVerdictWithoutStepsSaysSo(t *testing.T) { - inv := providers.Investigation{ - Title: "BackupJobFailed โ€” failing instance unidentified", - Verdict: providers.VerdictActionSuggested, - RootCauses: []providers.Hypothesis{{Summary: "the failing RDS resource could not be identified", Confidence: 0.85}}, - } - txt := blocksText(t, summaryBlocks(inv)) - if !strings.Contains(txt, "No next steps") { - t.Errorf("a verdict claiming an action with no remedy must say so on the card:\n%s", txt) - } - - // Control: a card that DOES carry a remedy renders the normal section and never - // the disclaimer. - inv.RootCauses[0].SuggestedAction = "delete the stale Job object" - txt = blocksText(t, summaryBlocks(inv)) - if !strings.Contains(txt, "Suggested next steps") || !strings.Contains(txt, "delete the stale Job object") { - t.Errorf("a supplied remedy must render normally:\n%s", txt) - } - if strings.Contains(txt, "No next steps") { - t.Errorf("the disclaimer must not appear when steps exist:\n%s", txt) - } - - // Control: no_action never claims a remedy, so it never gets the disclaimer. - quiet := providers.Investigation{ - Title: "self-healed", - Verdict: providers.VerdictNoAction, - RootCauses: []providers.Hypothesis{{Summary: "transient", Confidence: 0.9}}, - } - if txt := blocksText(t, summaryBlocks(quiet)); strings.Contains(txt, "No next steps") { - t.Errorf("no_action must not carry the disclaimer:\n%s", txt) - } -} - // TestNoRemedyNoticeRespectsWhatTheCardShows pins the split between the contract // predicate and the reader-side one. The contract asks "did the model supply a // remedy"; the card must ask "does the reader have one in front of them", and the @@ -1815,27 +1773,28 @@ func TestNoRemedyNoticeRespectsWhatTheCardShows(t *testing.T) { name string mutate func(*providers.Investigation) wantNotice bool + wantSteps string // when set, the normal section must render carrying this text }{ - {"nothing to act on at all", func(*providers.Investigation) {}, true}, + {"nothing to act on at all", func(*providers.Investigation) {}, true, ""}, {"a validated prior resolution is a remedy", func(inv *providers.Investigation) { inv.Prior = &providers.PriorKnowledge{Resolution: "roll back to 1.14.2, clear the lock"} - }, false}, + }, false, ""}, {"a whitespace-only prior resolution is not", func(inv *providers.Investigation) { inv.Prior = &providers.PriorKnowledge{Resolution: " "} - }, true}, + }, true, ""}, {"a matched runbook is a remedy", func(inv *providers.Investigation) { inv.MatchedKnowledge = &providers.MatchedEntry{ Path: "entries/x.md", Title: "Registry credentials deleted", Score: 6.4} - }, false}, + }, false, ""}, {"the model's own suggested_action is a remedy", func(inv *providers.Investigation) { inv.RootCauses[0].SuggestedAction = "clear the migration lock" - }, false}, + }, false, "clear the migration lock"}, {"a whitespace-only suggested_action is not, and renders no empty bullet", func(inv *providers.Investigation) { inv.RootCauses[0].SuggestedAction = " " - }, true}, + }, true, ""}, {"no_action promises nothing", func(inv *providers.Investigation) { inv.Verdict = providers.VerdictNoAction - }, false}, + }, false, ""}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -1845,8 +1804,15 @@ func TestNoRemedyNoticeRespectsWhatTheCardShows(t *testing.T) { if got := strings.Contains(txt, "No next steps proposed"); got != c.wantNotice { t.Errorf("notice rendered = %v, want %v:\n%s", got, c.wantNotice, txt) } - // Whichever way it went, the card must never show a bullet with nothing in it. - if strings.Contains(txt, "1. \n") || strings.Contains(txt, "1. _(reversible)_") { + // A supplied remedy must render the normal section carrying its text. + if c.wantSteps != "" { + if !strings.Contains(txt, "Suggested next steps") || !strings.Contains(txt, c.wantSteps) { + t.Errorf("a supplied remedy must render normally:\n%s", txt) + } + } + // Whichever way it went, the card must never show a bullet with nothing in + // it. Next steps render as "\nโ€ข %s", with the reversible marker on the bullet. + if strings.Contains(txt, "โ€ข \n") || strings.Contains(txt, "โ€ข _(reversible)_") { t.Errorf("rendered an empty next-step bullet:\n%s", txt) } }) diff --git a/internal/providers/providers.go b/internal/providers/providers.go index cafb08d4..9f64e386 100644 --- a/internal/providers/providers.go +++ b/internal/providers/providers.go @@ -1595,18 +1595,11 @@ func (inv Investigation) UnaccountedInconclusive() bool { // so the header promises a remedy the body never carries; a card that does that // costs more trust than one that says inconclusive. // -// It lives HERE for the same reason as UnaccountedInconclusive: two sides consult -// it and must not disagree. The loop says it out loud at the source, where the -// payload is still attributable to the model call that made it; the notifier acts -// on it at delivery. One definition, so a fifth verdict is one switch to update -// rather than two adjacent ones. -// -// And, exactly as there, one definition is NOT one snapshot. The loop evaluates -// this pre-verify; the notifier evaluates it on the post-verify result, so a run -// whose reviewer rejected the one cause that carried the remedy legitimately -// disagrees between them. The notifier also does not render straight off this -// predicate โ€” see notify.remedyMissingForReader, which additionally requires that -// the card show no OTHER remedy (a validated prior resolution, a matched runbook). +// Placement, and the pre-verify/post-verify caveat, are exactly as for +// UnaccountedInconclusive above. As there, the notifier does not render straight off +// this predicate โ€” see notify.remedyMissingForReader, which additionally requires +// that the card show no OTHER remedy (a validated prior resolution, a matched +// runbook). func (inv Investigation) ActionWithoutRemedy() bool { if !inv.Verdict.ClaimsAction() { return false @@ -1628,8 +1621,12 @@ func (inv Investigation) ActionWithoutRemedy() bool { // obliges the delivered notification to show what. no_action and inconclusive make // no such promise, and an empty/unknown verdict is a parse concern rather than a // contract breach โ€” it renders no badge at all, so it promises nothing. +// +// Defined in terms of Conclusive for the reason ValidVerdict gives: the three +// actionability verdicts stay enumerated once, so a fifth is one switch to update +// rather than two adjacent ones. func (v Verdict) ClaimsAction() bool { - return v == VerdictActionSuggested || v == VerdictActionRequired + return v.Conclusive() && v != VerdictNoAction } // MatchedEntry is the strongest pre-existing catalog entry an investigation's