scrum2 cleanup: JSON-marshal in stringifyValue, drop dead detectCycle, name SourceWorkflow
5 small fixes from the §3.8 scrum2 review wave:
- workflow.stringifyValue now JSON-marshals maps/slices instead of
fmt.Sprint %v (Opus+Kimi convergent: LLM modes were getting Go's
map[k:v] syntax, which is unparseable as JSON context).
- workflow.detectCycle removed — duplicate of topoSort that discarded
the useful node ID. Validate() now calls topoSort directly and
returns its wrapped ErrCycle.
- observer.SourceWorkflow named constant — was an implicit string
cast (observer.Source("workflow")) at the cmd/observerd handler.
- Unused context imports + dead silencer comments removed across
workflow/modes.go and observerd/main.go.
- Unused store parameter dropped from registerBuiltinModes (reserved
comment removed; can be re-added when a mode actually needs it).
just verify still PASS — these are pure cleanup, no behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c41698acae
commit
8278eb9a87
@ -18,7 +18,6 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"flag"
|
"flag"
|
||||||
@ -75,7 +74,7 @@ func main() {
|
|||||||
// to gateway's matrixd_url so a single-toml deploy works without
|
// to gateway's matrixd_url so a single-toml deploy works without
|
||||||
// duplicating the address.
|
// duplicating the address.
|
||||||
matrixdURL := cfg.Gateway.MatrixdURL
|
matrixdURL := cfg.Gateway.MatrixdURL
|
||||||
registerBuiltinModes(runner, store, matrixdURL)
|
registerBuiltinModes(runner, matrixdURL)
|
||||||
|
|
||||||
h := &handlers{store: store, runner: runner}
|
h := &handlers{store: store, runner: runner}
|
||||||
if err := shared.Run("observerd", cfg.Observerd.Bind, h.register, cfg.Auth); err != nil {
|
if err := shared.Run("observerd", cfg.Observerd.Bind, h.register, cfg.Auth); err != nil {
|
||||||
@ -145,7 +144,7 @@ func (h *handlers) handleWorkflowRun(r http.ResponseWriter, req *http.Request) {
|
|||||||
Success: n.Error == "",
|
Success: n.Error == "",
|
||||||
DurationMs: n.DurationMs,
|
DurationMs: n.DurationMs,
|
||||||
OutputSummary: summarizeOutput(n.Output),
|
OutputSummary: summarizeOutput(n.Output),
|
||||||
Source: observer.Source("workflow"),
|
Source: observer.SourceWorkflow,
|
||||||
Error: n.Error,
|
Error: n.Error,
|
||||||
Timestamp: n.StartedAt.UTC().Format(time.RFC3339Nano),
|
Timestamp: n.StartedAt.UTC().Format(time.RFC3339Nano),
|
||||||
}
|
}
|
||||||
@ -205,7 +204,7 @@ func summarizeOutput(output map[string]any) string {
|
|||||||
// - playbook.record (HTTP to matrixd)
|
// - playbook.record (HTTP to matrixd)
|
||||||
// - playbook.lookup (HTTP to matrixd)
|
// - playbook.lookup (HTTP to matrixd)
|
||||||
// - llm.chat (HTTP to gateway /v1/chat)
|
// - llm.chat (HTTP to gateway /v1/chat)
|
||||||
func registerBuiltinModes(r *workflow.Runner, store *observer.Store, matrixdURL string) {
|
func registerBuiltinModes(r *workflow.Runner, matrixdURL string) {
|
||||||
// Fixture modes for runner mechanics smokes.
|
// Fixture modes for runner mechanics smokes.
|
||||||
r.RegisterMode("fixture.echo", func(_ workflow.Context, input map[string]any) (map[string]any, error) {
|
r.RegisterMode("fixture.echo", func(_ workflow.Context, input map[string]any) (map[string]any, error) {
|
||||||
out := make(map[string]any, len(input))
|
out := make(map[string]any, len(input))
|
||||||
@ -232,13 +231,8 @@ func registerBuiltinModes(r *workflow.Runner, store *observer.Store, matrixdURL
|
|||||||
hc := &http.Client{Timeout: 30 * time.Second}
|
hc := &http.Client{Timeout: 30 * time.Second}
|
||||||
r.RegisterMode("matrix.search", workflow.MatrixSearch(matrixdURL, hc))
|
r.RegisterMode("matrix.search", workflow.MatrixSearch(matrixdURL, hc))
|
||||||
}
|
}
|
||||||
|
|
||||||
_ = store // reserved for future modes that need self-provenance
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// context still used in decodeJSON via http.Request.Context().
|
|
||||||
var _ = context.Background
|
|
||||||
|
|
||||||
func decodeJSON(w http.ResponseWriter, r *http.Request, v any) bool {
|
func decodeJSON(w http.ResponseWriter, r *http.Request, v any) bool {
|
||||||
defer r.Body.Close()
|
defer r.Body.Close()
|
||||||
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBytes)
|
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBytes)
|
||||||
|
|||||||
@ -32,10 +32,15 @@ import (
|
|||||||
type Source string
|
type Source string
|
||||||
|
|
||||||
const (
|
const (
|
||||||
SourceMCP Source = "mcp"
|
SourceMCP Source = "mcp"
|
||||||
SourceScenario Source = "scenario"
|
SourceScenario Source = "scenario"
|
||||||
SourceLangfuse Source = "langfuse"
|
SourceLangfuse Source = "langfuse"
|
||||||
SourceOverseerCorrection Source = "overseer_correction"
|
SourceOverseerCorrection Source = "overseer_correction"
|
||||||
|
// SourceWorkflow tags ObservedOps emitted by the workflow runner
|
||||||
|
// (one per node execution). Added 2026-04-29 scrum2 (Opus BLOCK):
|
||||||
|
// the workflow handler was casting a string literal to Source,
|
||||||
|
// which worked coincidentally but left the taxonomy implicit.
|
||||||
|
SourceWorkflow Source = "workflow"
|
||||||
)
|
)
|
||||||
|
|
||||||
// ObservedOp is one entry in the observer's ring buffer (and JSONL
|
// ObservedOp is one entry in the observer's ring buffer (and JSONL
|
||||||
|
|||||||
@ -17,7 +17,6 @@ package workflow
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
@ -209,6 +208,3 @@ func remarshalInput(input map[string]any, target any) error {
|
|||||||
return json.Unmarshal(bs, target)
|
return json.Unmarshal(bs, target)
|
||||||
}
|
}
|
||||||
|
|
||||||
// silence "imported and not used" if context isn't referenced after
|
|
||||||
// the MatrixSearch factory is used. Compiler will catch the real case.
|
|
||||||
var _ = context.Background
|
|
||||||
|
|||||||
@ -2,6 +2,7 @@ package workflow
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
@ -308,10 +309,13 @@ func walkPath(output map[string]any, path string) (any, error) {
|
|||||||
return cur, nil
|
return cur, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// stringifyValue renders a value as a string. For JSON-shaped values
|
// stringifyValue renders a value as a string for prompt substitution.
|
||||||
// (maps, slices, complex types), uses fmt.Sprintf %v which is
|
// Strings pass through; nil → empty string; everything else is JSON-
|
||||||
// adequate for prompt-substitution. JSON marshaling would be cleaner
|
// marshaled (so maps/slices produce valid JSON, not Go's %v syntax
|
||||||
// for complex types but adds a dep cycle for v0.
|
// like `map[k:v]`). JSON failure falls back to fmt.Sprint so the
|
||||||
|
// substitution can never panic on weird inputs. Per 2026-04-29
|
||||||
|
// scrum2 (Opus + Kimi convergent): %v output was confusing for LLM
|
||||||
|
// modes that expected JSON-shaped context.
|
||||||
func stringifyValue(v any) string {
|
func stringifyValue(v any) string {
|
||||||
switch x := v.(type) {
|
switch x := v.(type) {
|
||||||
case string:
|
case string:
|
||||||
@ -319,6 +323,9 @@ func stringifyValue(v any) string {
|
|||||||
case nil:
|
case nil:
|
||||||
return ""
|
return ""
|
||||||
default:
|
default:
|
||||||
|
if bs, err := json.Marshal(x); err == nil {
|
||||||
|
return string(bs)
|
||||||
|
}
|
||||||
return fmt.Sprint(x)
|
return fmt.Sprint(x)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -373,17 +380,3 @@ func topoSort(nodes []Node) ([]string, error) {
|
|||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// detectCycle is the predicate-only variant called from Validate;
|
|
||||||
// returns the offending node ID + true if a cycle exists.
|
|
||||||
func detectCycle(nodes []Node) (string, bool) {
|
|
||||||
_, err := topoSort(nodes)
|
|
||||||
if err == nil {
|
|
||||||
return "", false
|
|
||||||
}
|
|
||||||
// Best-effort extract — topoSort wraps the cycle-starting ID in
|
|
||||||
// the error message; for v0 just signal "yes, somewhere."
|
|
||||||
for _, n := range nodes {
|
|
||||||
_ = n
|
|
||||||
}
|
|
||||||
return "(see runner error for details)", true
|
|
||||||
}
|
|
||||||
|
|||||||
@ -165,8 +165,12 @@ func (w Workflow) Validate() error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if cyclicID, ok := detectCycle(w.Nodes); ok {
|
// Cycle detection: topoSort returns a wrapped ErrCycle on any
|
||||||
return fmt.Errorf("%w: starting at node %q", ErrCycle, cyclicID)
|
// cycle, including the offending node ID. Removed the separate
|
||||||
|
// detectCycle helper after 2026-04-29 scrum2 flagged it as dead
|
||||||
|
// code (it called topoSort then discarded the useful node ID).
|
||||||
|
if _, err := topoSort(w.Nodes); err != nil {
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user