Commit Graph
5 Commits
Author SHA1 Message Date
Vasanth T 378d538e24 perf(agent): execution profiles with one-shot posture escalation (PR10b+PR10c) (#740)
* perf(agent): execution profiles, --exec-profile, bench passthrough, TUI /profile (PR10b+10c)

* fix(execprofile): harden profiles against review findings

- fast escalates on critical-risk mutations, not high: the sandbox marks
  every shell command high before command analysis, so a high trigger
  spent the one-shot on the first go test of any task
- escalation can now restore the provider-default effort a profile
  displaced (PostureEscalation.RestoreDefaultEffort; a plain empty
  target means leave untouched)
- TUI /turns while a profile is active pins the budget: disarms the
  escalation turn target (mirroring exec's explicit --max-turns rule)
  and profile reverts use touched bits instead of value equality, so
  explicit choices survive even when they coincide with profile values
- spec-draft runs get the same escalation policy as the main run
  instead of the displaced budget with no safety net
- bench validates and canonicalizes --exec-profile before spawning:
  an unknown name previously produced an exit-0 report full of
  millisecond usage-error walls
- turn summary prints exec-profile alongside mode; zero exec rejects
  an inline-empty --exec-profile=; balanced trace label documented and
  pinned as deliberate attribution

* fix(execprofile): explicit effort choices disarm the escalation effort restore

An explicit effort set AFTER profile selection could be cleared by a
mid-run escalation (the effort analog of the /turns pin, introduced by
RestoreDefaultEffort): TUI /effort and Ctrl+T now disarm the restore on
a policy clone, and the spec-draft path only arms it when no explicit
--spec-reasoning-effort governs the draft.

* fix(execprofile): address review findings on the profile surfaces

- --use-spec projects away a profile's self-correct knob instead of
  silently arming an option the draft runner ignores (the explicit
  --self-correct --use-spec combination is already a parse-time error)
- RunTurnBench canonicalizes and validates ExecProfile at the library
  boundary, so a direct caller's casing/whitespace cannot fork report
  labels and an unknown name fails before anything runs
- reverting a profile whose displaced budget was zero clears
  ZERO_MAX_TURNS (SetMaxTurnsEnv ignores zero, so sub-agents would have
  kept the removed profile's budget)
- the TUI effort-disarm test fails instead of skipping when the catalog
  stops filling low effort

* fix(tui): reconcile an active profile's effort fill on model switch

The fill is per-model, not a session preference: fast selected on a
model without low fills nothing, and switching to a model that supports
it should behave like selecting the profile there; the reverse switch
must not keep sending a level the destination does not support. Both
switch paths (the /model command and the cross-provider picker) now
re-derive the fill, leaving explicitly touched efforts alone and
keeping the escalation's effort restore in step. Also: the revert-env
test asserts key removal via LookupEnv, and applyProfileTurnBudget's
pinned-budget parameter is named and documented as covering mode
presets, not just the flag.

* fix(tui): void profile effort bookkeeping when the model-switch drop erases it

The /model path's pre-existing unsupported-effort drop ran before the
profile reconciliation and left contradictory state behind: a dropped
touched override kept blocking the profile from governing, and a
dropped profile fill left a stale applied value with the effort restore
still armed. The drop itself is load-bearing (the TUI run path forwards
the effort to the provider unfiltered), so instead of preserving a
known-unsupported level, the voided choice now resets the profile's
effort bookkeeping and the reconcile re-derives from scratch. The whole
sequence lives in one ring-parameterized helper driven by the switch
target's authoritative ring, unit-tested against synthetic rings for
both the void-then-refill arc and the supported-override survival.

* fix(tui): only drop an explicit effort when the target ring is authoritative

A nil ring meant both a catalog model with no effort controls and a
live-discovered custom model the catalog cannot vouch for. The drop now
requires an authoritative ring (a resolved catalog entry, whose ring may
legitimately be empty); an unknown ring preserves an explicit preference,
matching the cross-provider picker's behavior, while the profile's own
fill still applies only where support is known.
2026-07-19 00:53:17 +05:30
Vasanth T dbd9443014 fix(perfbench): absolutize the bench binary and make errored tasks first-class (#730)
* fix(perfbench): absolutize the bench binary and make errored tasks first-class

* fix(perfbench): state that errored tasks count as pass-rate failures

* fix(perfbench): align errored-task wording with the actual tier accounting

* chore(perfbench): never commit generated baseline reports
2026-07-18 21:39:52 +05:30
Vasanth T abe24bcb10 Add per-turn tracing and a turn-benchmark harness (Phase 0 baseline) (#700)
* Add per-turn tracing and a turn-benchmark harness (Phase 0 baseline)

Introduce an opt-in `internal/trace` package that attributes a run's wall
time to named spans — prompt build, provider connect/queue, generation,
tool queue/execution, permission wait, verification, compaction,
persistence — plus counters for model requests, tool calls, retries,
reconnects, compactions, completion nudges, acceptance checks, model
switches, and input/cached/output tokens. The recorder is nil-safe (a nil
*Recorder is a no-op), so every stamp site can call it unguarded; an
untraced run is byte-identical to today.

The recorder threads into agent.Run via a new `Options.Trace` field and a
single `trace.WithContext` assignment, so the shared providerio seam
(connect around client.Do, queue around OAuth resolve, first token at the
first non-keepalive SSE payload) reaches it through `trace.FromContext`
without any provider-interface change. All instrumentation is opt-in: when
`Options.Trace` is nil the hot path is untouched.

Headless `zero exec` gains `--trace <path>` (and `ZERO_TRACE=<path>`),
writing an agenteval-contract-compatible NDJSON trace (`-` for stderr).
The trace is pure observation; enabling it does not change behavior.

A new per-turn benchmark harness extends `internal/perfbench` (reusing the
existing TaskSet/NumericStats/subprocess runner shapes): `RunTurnBench`
aggregates per-span median/P95 across iterations, ranks the top three
controllable latency sources by share of attributed time, rolls up per
task class, and totals tokens/counts. A checked-in 42-task manifest across
seven classes (nav, edit, fix, refactor, longproc, longctx, parallel) and
local offline fixtures back it, and `make baseline` runs it against the
built `zero`, writing `internal/perfbench/reports/baseline.json`. The
report is machine-specific and regenerated, not hand-edited.

The manifest is JSON rather than YAML to avoid adding a dependency;
per-task verification targets the specific test (`go test -run <name>`)
so fix tasks are independent. Each task is a fresh `zero exec` process, so
iterations are cold-start samples; a warm in-process path is left for
follow-up.

Verified: `go build ./...`, `go vet ./...`, `gofmt -l` clean on touched
files; `go test -race` green for internal/trace, internal/agent,
internal/perfbench, and cmd/zero-perf-bench.

* test(trace): make TestAttributionRatio clock-independent

The test recorded two synthetic 10ms spans with no real delay and asserted
the attribution ratio stayed <= 1.0. On a precise clock (Linux/macOS CI) the
wall is microseconds while attributed is 20ms, so attributed/wall blows past
1.0 and the test failed — even though the ratio legitimately exceeds 1.0
when spans overlap (parallel tool execution). On Windows the coarse timer
masked it, so the failure only surfaced in CI.

Assert the contract instead: attributed is the deterministic sum of span
durations, and the ratio equals attributed/wall (0 when wall is zero). Add a
zero-wall case for the divide-by-zero guard.

* Address review feedback on the per-turn tracer + benchmark harness

Benchmark runner (internal/perfbench):
- A nonzero agent exit code no longer counts as Passed. The runner set
  VerifyErr but then fell through to outcome.Passed = true, so a crashed
  run could be recorded as a pass. Return before the verification block
  when VerifyErr is already set.
- A task now passes only when every iteration passes. The previous
  "any iteration passes" rule let a flaky fail hide behind one good run.
- Each invocation copies its workspace fixture to a fresh temp dir and
  runs the agent + verifier there, so mutating tasks (edit/fix/refactor)
  can't dirty the checked-in fixtures or bleed into the next iteration.
- Trace load failures (missing/malformed trace file) are surfaced as
  result warnings via a new TraceIssue field instead of being swallowed.

Tracer (internal/trace):
- Finish freezes the recorder: Counter, addSpan, and the first-event
  stamps are dropped after Finish returns, so a snapshot captures exactly
  one turn and late stamps can't mutate it.
- WriteText propagates the first write error instead of silently
  swallowing sink failures.

Provider seam (internal/providers/providerio):
- FirstTokenAt is stamped only after a payload is accepted as real model
  output, not before handle() decides to drop it as an error payload.

Agent loop (internal/agent):
- The compaction counter increments only when the history actually
  shrinks, in both the proactive and reactive paths — paid no-ops no
  longer inflate the compaction total.
- Document on Options.Trace that a fresh Recorder is required per Run.

CLI (internal/cli/exec):
- Finish the trace at the agent.Run boundary and defer only the
  serialization, so the snapshot covers exactly the turn.
- Reject --trace / ZERO_TRACE for spec-draft runs with a clear error
  instead of silently accepting and writing nothing.

Manifest + fixtures:
- Strengthen the edit verifiers: rename and value-bump tasks now assert
  the old symbol is gone, and the getter task greps the method signature.
- Add the missing testdata/edit fixture (10 edit tasks referenced a
  directory that was never committed, so the whole edit class errored
  out at run time).
- The manifest test now stats each fixture so a missing dir fails at
  load time instead of at every run.
- longproc benchmark keeps Process live across the loop; README code
  fence gets a language tag and documents the per-invocation fixture copy.
- Add tests for the tracing wrapper (nil vs wired recorder, OnUsage
  forwarding, token aggregation) and the frozen-recorder contract.

* Address review feedback: exclusive-time attribution + wired spans + fix-suite compile

Rewrite the trace attribution model so concurrent/nested spans no longer
double-count. Spans record wall intervals; the recorder derives a parent for
each by interval containment and computes exclusive time (duration minus the
union of its children). AttributedDuration sums only top-level spans; Coverage
is the union of intervals over wall, capped at 1, the honest ">=95%" metric.
Top-latency ranking uses exclusive time so shares sum to ~1 instead of being
inflated by overlapping provider_connect/generation and nested
permission_wait/tool_execution.

Wire the spans that were named but never stamped: verification around the
self-correct verify pass, and StampFirstVisibleEvent on the first forwarded
text event. Drop the tool_queue/process_wait/persistence constants that had no
stamps, and rename the misnamed near-noise prompt_build span to tool_partition.
Trim RequiredEventKeys to events a healthy short run actually emits; move
conditional phases (tools, permission, compaction, verification) to
OptionalEventKeys so a short trace does not false-fail.

Make ReadNDJSON fail loudly on a corrupt file: require a type:trace header,
error on span/counter lines that appear before it, and error when a header
yields no spans or counters.

Fix the blocking fix-suite compile error: Label's %d format verb on a string
tripped go test's vet before any fix task could verify. Make it a runtime
wrong-prefix bug ("admin-" vs "user-") that still type-checks, so each fix
task is independent again.

Manifest: correct refactor-04's stats.go to main.go, reword fix-07 to the
wrong-prefix bug, add a latency-only description (read-only tasks carry no
verification oracle by design), and frame the report README on exclusive time
and coverage.

Tests updated for the occurrence/exclusive/coverage model; trace, perfbench,
and agent green under -race.

* Fix exclusive-time round-trip, identical-interval nesting, and empty-trace parse

Three correctness bugs in the attribution model found by an adversarial review
of the prior commit, all undermining the no-double-count guarantee:

1. exclusive_ms:0 was overwritten with the inclusive Duration on re-parse. A
   parent whose children tile its interval legitimately has exclusive time 0;
   WriteNDJSON emits exclusive_ms:0, but ReadNDJSON's "<= 0 means missing"
   fallback turned that back into the full duration, so every re-parsed trace
   re-introduced the double-counting the model exists to prevent (the benchmark
   harness, which ranks re-parsed traces, inherits it). Gate the fallback on
   key ABSENCE, so a written 0 is preserved.

2. deriveNesting formed a 2-cycle for two spans with identical [start, end]
   intervals: the symmetric containment check made each parent the other,
   dropping both from top-level and zeroing AttributedDuration. Break the tie
   deterministically — strict containment is an unambiguous parent; for
   identical intervals, only the lower-indexed span may parent the other, so
   one stays top-level.

3. ReadNDJSON returned a valid empty TurnTrace for empty/blank-only input, so a
   run that crashed before emitting a trace (or where --trace was not honored)
   masqueraded as a clean zero-attribution sample. Empty input now errors, so
   the harness records a TraceIssue.

Also: decode counters via json.Number so int64 values above 2^53 round-trip
exactly instead of losing precision through float64; and drop the harness's
own "exclusive <= 0 -> Duration" ranking fallback, which was the same
double-count on the re-parsed path. Added regression tests covering the
zero-exclusive round-trip, the identical-interval nesting, empty-input
rejection, and counter precision.

* perf(turn-bench): split pass/fail into oracle tiers so the pass rate can't be misread as correctness

The turn benchmark counted any exit-0 task as passed, including the 24
read-only tasks (nav/longproc/longctx/parallel) that carry no verification
oracle — so the JSON tasksPassed / "N/M passed" headline could read as a
correctness verdict it never was. Split pass/fail into three oracle tiers:

- correctness (edit grep + fix go test): the only pass rate that can move
  with model quality (tasksVerified / tasksPassed / correctnessPassRate)
- build-only (refactor go build): a non-positive oracle, reported as
  buildPassRate and excluded from correctnessPassRate; the manifest declares
  buildOnlyClasses: ["refactor"]
- latency-only (no oracle): counted in latencyOnlyTasks, never in any pass
  rate

NewTurnExecRunner no longer sets Passed=true for tasks with no
verificationCommand, and the summary headline names each tier explicitly so
an exit-0 read-only run cannot inflate a correctness number. Bumps
TurnSchemaVersion to 2. Class breakdown and tier contract documented in
internal/perfbench/MANIFEST.md and reports/README.md.

Also install the trace first-token/visible-event forwarding handlers in
agent.Run whenever options.Trace is set, not only when a UI OnText /
OnReasoning callback is set, so a headless traced run (zero exec --trace)
still captures FirstTokenAt. forwardedVisibleText stays tied to the user
callback so stall-retry semantics are unchanged. Adds a regression
assertion to the existing traced-run test.

* Address CodeRabbit review on 2bfb3406: omitempty + crash visibility

Three findings from CodeRabbit's review of the 3-tier pass-rate fix:

- BuildPassRate dropped its omitempty so an explicit 0.0 (all refactor
  tasks fail to compile) is no longer indistinguishable from 'no build
  tasks ran' in the JSON. Matches CorrectnessPassRate's always-serialize
  behavior, which is the whole point of the tier split: the report
  cannot be misread.
- A crashed sample (outcome.Err) now emits a Warning mirroring the
  TraceIssue path, so a run that died every iteration can't look like a
  normal benchmark with fewer measurements instead of a failure.
- cmd.Run()'s returned error is preserved on the no-run_end path: when
  stderr is empty and Run failed (binary missing, exec permission
  denied), the actual OS error is reported instead of collapsing to the
  generic 'missing terminal run_end event'.

go build/vet/gofmt clean; go test -race green for perfbench/agent/trace.

* Address CodeRabbit re-review: anti-averaging README + fixture-isolation test

Re-review at d0615249 asked for the pass-rate contract to be loud to
downstream consumers and for the fixture-isolation property to be asserted,
not just read from code.

- reports/README.md gains a prominent 'do not average the tier pass rates /
  do not report a single pass rate' warning. The schema offers no headline
  pass rate on purpose; a consumer must name the tier it is reporting. This
  is the durability fix for the misread the tier split exists to prevent.
- turn_bench_test.go gains TestCopyFixtureIsolatesSourceFromMutation: copies
  a fixture, mutates the copy, and asserts the source is byte-identical
  (and that a file added to the copy never appears in the source). Proves the
  'fixtures stay clean across runs' property without needing the binary.

Items the re-review also raised that were already satisfied: the
oracle-presence-over-declared-list precedence is documented on the
BuildOnlyClasses field (taskbench.go), and the PR body already says
'48-task manifest'. The Share->exclusiveShare rename is tracked in #701.

go build/vet/gofmt clean; go test -race green for perfbench.
2026-07-16 21:58:20 +08:00
KRATOS 47c3cc8937 wedge stage 13: launch readiness (first-run + benchmark) (#186)
* stage 13: launch readiness (first-run + benchmark)

First-run, model-agnostic-first:
- provideronboarding: detect local OpenAI-compatible runtimes (Ollama/LM Studio)
  on their default ports and offer them with no API key required
- provideronboarding: ClassifySetupProbe maps a live connectivity probe to a
  specific, fixable error class (bad key, wrong base URL, model not found,
  rate limit, config) instead of a stack trace; never panics
- zero setup --verify: probe the provider after saving and report the fixable
  error on failure; print a concrete 'try this' headless example on success

zero doctor hardening:
- new sandbox.backend check (bubblewrap/sandbox-exec) with a platform-specific
  install remedy; warn (not fail) on policy-only fallback
- new lsp.servers check listing missing language servers with per-binary install
  remedies; injectable GOOS/LookupExecutable for deterministic tests

Self-correct surface + benchmark:
- zero exec --self-correct wires the post-edit verify-and-correct loop via the
  project verifier; nil corrector when off keeps the agent loop byte-identical
- perfbench task harness: reproducible Terminal-Bench-style runner over headless
  zero exec + stream-json; records model + commit + self-correct flag + date
- zero-perf-bench tasks subcommand (with --dry-run) and a sample task set
- docs/BENCHMARK.md: the number, the exact command, the model, and the
  with/without-self-correct methodology

* stage 13: address review (verify-state, fixture paths, run_end exit, 404 classification)

* stage 13: verifier inherits process environment (PATH/HOME/toolchain)
2026-06-13 11:10:37 +05:30
Anandan 503b6c5177 Move performance benchmark to Go (#87)
* Move performance benchmark to Go

* Address perf benchmark review feedback

* Fix perf benchmark pipe drain order
2026-06-06 12:59:01 +05:30