Skip to content

test(eval): sidecar eval harness + distributed-seams suite (V2-19)#2202

Merged
kovtcharov-amd merged 2 commits into
mainfrom
test/sidecar-eval-harness-2180
Jul 18, 2026
Merged

test(eval): sidecar eval harness + distributed-seams suite (V2-19)#2202
kovtcharov-amd merged 2 commits into
mainfrom
test/sidecar-eval-harness-2180

Conversation

@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Why this matters

Before this, nothing exercised the email agent through the v2 sidecar/daemon path — every eval ran the loop in-process — and no test guarded the daemon↔sidecar contracts as a set. The frozen §0.2 event vocabulary, the relay's crash/cancel terminal, and the auth leg were each pinned only by their own issue's tests, so a drift between seams (e.g. a front-door and the sidecar disagreeing on what "done" looks like) could slip through green.

After: an eval harness drives POST /v1/<agent>/query over REST and asserts the canonical event sequence (not a brittle final string) against baselines committed in the agent package — serial by construction per the CLAUDE.md eval rule. Plus a single distributed-seams suite that guards the contracts across seams and self-skips the ones not yet merged, with each skip keyed on the source's real presence so it flips to a live assertion the moment its issue lands.

The harness (src/gaia/eval/sidecar_harness.py) keeps its pure core (SSE parse, ordered-subsequence match tolerant of LLM-variable event counts, a cross-process SerialEvalLock) split from a thin live driver, mirroring behavior_harness. A missing/unreachable backend raises loudly — never a silent green pass.

Seams: covered vs. skipped-pending-merge

Seam Issue State
Relay ↔ translator ↔ harness vocabulary coherence V2-7 #2150 ✅ covered
Relay synthetic crash terminal parses as canonical error V2-7 #2150 ✅ covered
Daemon→sidecar bearer via private env, never argv #1980 ✅ covered
Secret-file (fd/0600) launch-secret delivery V2-3 #2149 ⏭️ skip-pending
Model-slot broker serialization V2-11 #2151 ⏭️ skip-pending
Custody /host/v1/* per-agent scoping V2-12 #2153 ⏭️ skip-pending
Migration idempotency V2-13 #2155 ⏭️ skip-pending

The on-hardware golden path (UI → daemon → sidecar → Lemonade) is an opt-in real_model test (tests/integration/eval/test_sidecar_eval.py); a new Ubuntu CI job (test_distributed_seams.yml) runs the deterministic suite on every touching PR.

Test plan

  • python -m pytest tests/unit/test_sidecar_harness.py — 27 passed (pure core: SSE parse, sequence match, serial lock, baseline load; no Lemonade)
  • python -m pytest tests/integration/test_distributed_seams.py -m distributed_seams -rs — 6 passed, 4 skipped (skips print their pending-merge reason)
  • python -m pytest tests/integration/eval/test_sidecar_eval.py — 1 skipped (loud opt-in reason; runs on hardware with GAIA_SIDECAR_EVAL_LIVE=1)
  • python util/lint.py --all --fix — black/isort/flake8 clean on the new files
  • On strix-halo: GAIA_SIDECAR_EVAL_LIVE=1 python -m pytest tests/integration/eval/test_sidecar_eval.py -m real_model (golden path through the real sidecar)

Part of #2014
Closes #2180

Before: nothing exercised the email agent through the v2 sidecar/daemon
path, and no test guarded the daemon<->sidecar contracts as a set — the
frozen §0.2 event vocabulary, the relay's crash/cancel terminal, and the
auth leg were each pinned only by their own issue's tests, with no
cross-seam coherence check. After: an eval harness drives POST
/v1/<agent>/query over REST and asserts the canonical event *sequence*
against baselines committed in the agent package (serial by construction,
per the CLAUDE.md eval rule), plus a single distributed-seams suite that
guards the contracts across seams and self-skips the ones not yet merged.

Harness (src/gaia/eval/sidecar_harness.py): pure core (SSE parse, ordered-
subsequence match tolerant of LLM-variable event counts, cross-process
SerialEvalLock) split from a thin live driver, mirroring behavior_harness.
A missing/unreachable backend raises loudly — never a silent green pass.

Seams covered now: relay<->translator<->harness vocabulary coherence; the
relay's synthetic crash terminal parses as a canonical error; the
daemon->sidecar bearer is delivered via a private env channel, never argv.
Skipped-pending-merge (each keyed on the source's real presence, so the
skip flips to a real assertion when its issue lands): secret-file delivery
(#2149), model-slot broker (#2151), custody scoping (#2153), migration
idempotency (#2155).

The on-hardware golden path (UI -> daemon -> sidecar -> Lemonade) is an
opt-in real_model test; a new Ubuntu CI job runs the deterministic suite.

Part of #2014
Closes #2180
@github-actions github-actions Bot added dependencies Dependency updates devops DevOps/infrastructure changes eval Evaluation framework changes tests Test changes performance Performance-critical changes agent::email Email agent changes labels Jul 18, 2026
@kovtcharov-amd
kovtcharov-amd marked this pull request as ready for review July 18, 2026 02:31
@github-actions

Copy link
Copy Markdown
Contributor

Verdict: Approve

This is a well-built, test-only PR: it adds a sidecar eval harness that drives the email agent's /query loop over REST and asserts the canonical event sequence (not a brittle final string), plus a consolidated distributed-seams suite that guards the daemon↔sidecar contracts and self-skips the seams not yet merged. The most important thing worth knowing: the pending-merge tests are written to flip to a hard failure the moment their source module lands — that's intentional (a built-in "now implement me" reminder), not a latent bug.

Bottom line: loud-on-absence throughout (no silent green passes), serial-by-construction via a real cross-process lock, and baselines committed next to the route they pin. I verified the always-run tests reference symbols that actually exist in the landed relay/manager/translator modules, and that requests is a core dependency so the harness import resolves in the new CI job. No blocking issues.

🔍 Technical details

Verification performed

  • Always-run seam tests resolve: gaia.daemon.relay exports TERMINAL_TYPES, _synthetic_error_frame, stream_ended_unexpectedly_detail (relay.py:71,132,142); stream_ended_unexpectedly_detail("email") contains both "ensure" (via "re-ensure") and "logs" — so test_relay_synthetic_error_detail_is_actionable passes. AgentSidecarManager._spawn_process(argv, popen_kwargs, port) and spec.token_env_var match the auth-leg test's call and assertions (manager.py:258,270). gaia_agent_email.sse_translation exports CanonicalTranslator/TERMINAL_TYPES.
  • requests import path: SidecarEvalHarness._run_locked does import requests, and the unit test_harness_unreachable_backend_raises_actionable exercises it. requests is in core install_requires (setup.py:107), so -e ".[api]" in test_distributed_seams.yml pulls it in — no ImportError masking the intended SidecarUnavailable.
  • create_app is keyword-only (token, port, pid, started_at, registry) — matches both the live fixture and the _daemon_has_route custody probe.

🟢 Minor (optional, non-blocking)

  • SerialEvalLock._pid_alive swallows except Exception: return True (sidecar_harness.py:562-572). It's documented and picks the safe loud branch (treat holder as alive → eventually SerialEvalTimeout rather than stealing the slot), so it doesn't violate the spirit of the no-silent-fallback rule. One edge worth a thought: if psutil were genuinely missing, a dead holder's stale lock could never be reclaimed and every run would time out. Since psutil is a core dep this is theoretical; consider narrowing to except ImportError so an unexpected psutil-internal error surfaces instead of being read as "alive":
          try:
              import psutil
    
              return psutil.pid_exists(pid)
          except ImportError:
              # psutil should always be present (a core dep); if it truly is not,
              # treat the holder as alive — refusing to run is the safe, loud
              # choice, never silently stealing the lock.
              return True
    
  • parse_sse treats each data: line as an independent event (sidecar_harness.py:274-300). It doesn't join multiple consecutive data: lines of one SSE event per the spec. That's fine for the current single-line data: <json>\n\n contract (relay and translator both emit that), but if a front-door ever emits a multi-line data frame it'd surface as __unparseable__. Worth a one-line comment noting the single-line-frame assumption; not a defect today.

Strengths

  • Pure-vs-live split mirrors behavior_harness: all classification logic (SSE parse, ordered-subsequence match, baseline validation, the lock) is module-level and unit-tested with no server/Lemonade — 27 pure tests including all-violations-reported and stale-lock reclamation.
  • Loud everywhere: SidecarUnavailable/SerialEvalTimeout name what failed, what to do, and where to look; absent baselines and unreachable backends can never become a green pass.
  • Skip-pending-merge keyed on real presence (_module_exists/_daemon_has_route) so the coverage table can't silently drift, and each skip auto-flips to a live assertion when its issue lands — no follow-up edit to the suite needed.
  • distributed_seams marker registered in pyproject and the CI job targets the file directly with -rs, keeping the covered-vs-pending split visible in the log and avoiding a whole-tree scan.

@kovtcharov-amd
kovtcharov-amd enabled auto-merge July 18, 2026 02:45
@kovtcharov

Copy link
Copy Markdown
Collaborator

Two harness issues worth fixing before merge — both silent-failure shaped, which matters most in an eval harness:

  1. A test that can never fail. tests/integration/test_distributed_seams.pytest_seam_coverage_manifest_reflects_reality asserts two hardcoded True literals and json.dumps-serializability; the computed probe values (covered["auth_leg2_bearer_via_env"], all four pending entries) are never asserted. Its docstring claims the covered-vs-skipped table "can never silently drift from the code," but nothing it checks can change its outcome — the manifest can drift arbitrarily and it stays green. Either assert the probe values against the documented expectations or drop the test.

  2. TOCTOU in SerialEvalLock stale-lock reclaim (src/gaia/eval/sidecar_harness.py). Two contenders that both read a dead holder pid: one unlinks and re-creates the lock with its own pid; the other, acting on its stale read, then unlinks that live lock and creates its own. Both now believe they hold the lock and evals run in parallel silently — the exact Lemonade race-evict the lock exists to prevent (and the CLAUDE.md serial-eval rule). Narrow window (needs a crashed prior eval + concurrent starts), but the failure is invisible. Fix: re-verify the file still carries the stale pid at unlink time (rename-then-remove) or use an O_EXCL reclaim token.

Lesser: the live golden-path fixture's broad except Exception → pytest.skip around registry.ensure("email", …) turns a real sidecar-startup regression into a green skip on the opt-in hardware run meant to catch it — reserve the skip for missing-deps/env.

The rest of the suite is honest and the seam coverage is real — these are the only pieces that can go green while lying.

…row the golden-path skip

Three silent-failure seams in the sidecar eval harness could all drift or
regress while the tests stayed green:

- The seam-coverage manifest test asserted two hardcoded `True` literals and
  mere JSON-serializability, never the computed probe values — so the
  covered-vs-pending split could silently diverge from reality. It now pins the
  live probes to an explicit expected map, so a merged seam flips a probe, fails
  the test, and forces the manifest + real test to be updated.

- SerialEvalLock's stale-lock reclaim was TOCTOU-racy: two contenders that both
  read the same dead holder pid could each unlink and recreate, ending with both
  acquiring — the exact parallel-eval Lemonade race-evict the lock prevents. The
  reclaim now renames the lock into a per-pid token (atomic; only one contender
  can move the file), re-verifies the stale pid, and re-contends on any
  interference. Adds a unit test simulating the losing contender.

- The live golden-path fixture wrapped `registry.ensure` in a broad
  `except Exception -> pytest.skip`, turning real sidecar-startup regressions
  into green skips on the opt-in hardware run. It now skips only on the
  unconfigured-environment errors (missing binary / dev deps / unsupported
  platform); health-timeout, version-mismatch, integrity, and HTTP failures
  propagate as failures.

@kovtcharov kovtcharov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three silent-failure seams are fixed: the seam-coverage test now asserts the full manifest dict (fails when a pending seam merges, forcing the update — no longer a tautology), SerialEvalLock uses a verify-at-unlink reclaim so two contenders can't both acquire, and the golden-path skip is narrowed to missing-env/deps. Suite runs 34 passed / 4 skipped without live infra. Verified on the branch.

@kovtcharov-amd
kovtcharov-amd added this pull request to the merge queue Jul 18, 2026
Merged via the queue into main with commit 68773d4 Jul 18, 2026
36 of 38 checks passed
@kovtcharov-amd
kovtcharov-amd deleted the test/sidecar-eval-harness-2180 branch July 18, 2026 04:50
kovtcharov added a commit that referenced this pull request Jul 18, 2026
…has landed

The distributed-seams suite (#2202) is a forcing function: each pending-seam
placeholder raises the instant its source module appears, so the PR that lands
the seam must replace the placeholder with a real assertion. Two seams have
landed and left the suite red:

- model-slot broker (#2151) — this PR's net-new gaia.daemon.broker
- ~/.gaia migration (#2155/#2200) — already on main; #2202 merged after it and
  never flipped the placeholder, so main's Distributed Seams job is currently red

Replace both placeholders with real assertions — the broker serializes two
concurrent different-model lease requests (no race-evict of the single Lemonade
slot) and grants an interactive lease ahead of a queued background one; the
migration run-twice-is-a-no-op against a real populated pre-v2 fixture — and move
both entries from the pending map to covered so the manifest reflects reality.
Secret-file (#2149) and custody (#2153) stay pending and correctly skipped.
@itomek itomek mentioned this pull request Jul 22, 2026
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent::email Email agent changes dependencies Dependency updates devops DevOps/infrastructure changes eval Evaluation framework changes performance Performance-critical changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(eval): sidecar eval harness + distributed-seams test suite (V2-19)

2 participants