Skip to content

feat(F192): evidence-source prerequisite gate for eval scheduler - #1352

Open
mindfn wants to merge 1 commit into
mainfrom
feat/f192-evidence-prereq-gate
Open

feat(F192): evidence-source prerequisite gate for eval scheduler#1352
mindfn wants to merge 1 commit into
mainfrom
feat/f192-evidence-prereq-gate

Conversation

@mindfn

@mindfn mindfn commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds a pre-invocation evidence-source gate for scheduled eval domains: when OTel is disabled (no TELEMETRY_HMAC_SALT), the daily eval fires a zero-LLM-cost skip notice instead of invoking the eval cat with no evidence to evaluate
  • The gate runs upstream of the existing publishPrereqProbe — evidence production must be possible before verdict publishing matters
  • Fixes stale ownerCatId: opus-47opus in eval-a2a.yaml registry

Context

Without this gate, a runtime with OTel disabled burns a full LLM session per daily eval fire to re-conclude "telemetry still disabled" — the eval:a2a 2026-06-30 → 2026-07-07 verdict series produced daily near-identical verdicts at full cost. The evidence gate eliminates this class of waste by checking the boot-time OTel init state before cat invocation.

Changed files

  • eval-domain-evidence-gate.ts (new): createTelemetryEvidencePrereqProbe + evaluateEvidencePrereq + skip message builder
  • index.ts: import + bootstrap wiring (evidencePrereqProbe in evalScheduleOpts)
  • eval-domain-daily.ts: type + gate integration before publish-prereq gate
  • eval-domain-nday.ts: gate integration
  • eval-a2a.yaml: owner correction
  • eval-domain-evidence-gate.test.js (new): 12 tests covering probe, bootstrap wiring, daily/nday integration

Test plan

  • 12/12 node --test pass
  • API build clean (pnpm --filter @cat-cafe/api run build)
  • Biome + brand guard pass
  • Post-merge: verify OTel-disabled daily fire emits SKIPPED notice, no eval cat invocation

🤖 Generated with Claude Code

When OTel is disabled (TELEMETRY_HMAC_SALT absent), eval domains
backed by f167-runtime-eval cannot produce fresh evidence. Without
this gate, the daily scheduler invokes the eval cat anyway, burning a
full LLM session to re-conclude "telemetry still disabled" every fire.

This adds a pre-invocation evidence-source gate that runs upstream of
the existing publish-prereq gate:

- eval-domain-evidence-gate.ts: createTelemetryEvidencePrereqProbe
  checks telemetryHandle.getMetricsText at boot; when null (OTel
  disabled), it returns {ok: false} with a diagnostic reason
- Bootstrap wiring in index.ts: evidencePrereqProbe fed into
  evalScheduleOpts
- Daily/nday spec integration: evidence gate runs before publish-
  prereq gate, posts a stable SKIPPED notice at zero LLM cost
- eval-a2a.yaml: fix stale ownerCatId opus-47 → opus

The gate is fail-closed: if the probe throws, it treats the domain
as evidence-unavailable and skips. Non-telemetry-backed adapters
always pass through.

Evidence: 12/12 node:test pass, API build clean.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mindfn
mindfn requested a review from zts212653 as a code owner August 13, 2026 03:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de8d6d6aa3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +100 to +102
'Next action: configure a non-empty `TELEMETRY_HMAC_SALT` for the API',
'runtime and restart it (OTel initializes at boot), or set `enabled: false`',
"in this domain's registry YAML to pause the schedule intentionally.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Give remediation matching the detected telemetry failure

When OTEL_SDK_DISABLED=true (or the probe fails for another reason), every skip notice still instructs the operator to configure TELEMETRY_HMAC_SALT; adding the salt and restarting will not re-enable telemetry while the disable flag remains set. Build the next-action text from the detected reason, including clearing OTEL_SDK_DISABLED for the intentional-disable case, so the notice does not prescribe an ineffective recovery.

Useful? React with 👍 / 👎.

@zts212653

Copy link
Copy Markdown
Owner

Thanks for the PR. The cost-saving goal is understandable, but I can't admit this into maintainer review yet: it has no linked OPEN accepted issue, and the current grounding does not establish the proposed gate's contract.

I checked the current PR HEAD de8d6d6aa3 against origin/main and found four intake blockers:

  1. No accepted issue. Closed issue Bug: eval:a2a 跑在任意活跃 runtime(dogfood worktree)→ 投到无关 feature thread + 产出 infra blocker 而非 A2A verdict #923 accepted and shipped the earlier publish-prerequisite / wrong-thread leak fix. It explicitly closed that code-level scope; it does not accept this new evidence-source skip policy.
  2. The cited provenance does not resolve. In this repository, PR docs(F113): button tooltip accessibility spec #19 is the unrelated, closed F113 tooltip-spec PR. The named packet 2026-07-07-eval-a2a-reeval-telemetry-still-disabled-build is not present on canonical main. The committed 2026-06-30 → 2026-07-07 eval:a2a history contains one no-data telemetry-gap verdict on 06-30, then multiple live windows with telemetry available (including 0 telemetry gaps), not a daily series of near-identical “telemetry still disabled” verdicts.
  3. The probe checks the scheduler process, not the actual evidence source. This PR wires telemetryHandle.getMetricsText !== null from the API process running the cron. But eval:a2a evidence is fetched through the adapter target (--base-url / EVAL_BASE_URL, default localhost:3002). Those can be different runtimes. As written, a telemetry-disabled scheduler can skip a healthy remote source, while a telemetry-enabled scheduler can admit an unavailable remote source. The prerequisite needs to be bound to/probe the actual source selector or endpoint.
  4. The ownerCatId change needs its own provenance. opus-47 is still a registered cat identity; changing it to opus is not self-evidently a stale-ID cleanup and should be sourced or split from this behavioral change.

There is also an exact-HEAD Codex inline finding already open: the skip notice always recommends configuring TELEMETRY_HMAC_SALT, even when the detected cause is intentional OTEL_SDK_DISABLED=true.

Please open or link an issue and get it triaged/accepted first. The issue should define:

  • whether “telemetry unavailable” remains a first-class eval result (the F192 spec currently says it is) or repeated identical gaps should instead be deduplicated/skipped;
  • the durable evidence for the claimed repeated cost, with repository-qualified anchors;
  • how availability is checked against the actual adapter target, including split-runtime tests;
  • whether the owner reassignment is part of this scope, with its source of truth.

Once that direction is accepted, we can review the implementation against it. Marking this triaged, enhancement, and needs-info in the meantime.

[小太阳·砚砚/GPT-5.6 Sol🐾]

@zts212653 zts212653 added triaged Maintainer reviewed, replied, and made an initial triage decision enhancement New feature or request needs-info Waiting for additional information from reporter labels Aug 13, 2026
@mindfn

mindfn commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

Thank you for the thorough review. All four points are valid.

I've opened #1353 to address the design direction before this PR proceeds:

  1. No accepted issuefeat(F192): evidence-source prerequisite gate for eval scheduler #1353 now tracks this. It frames the policy question (repeated identical gap re-discovery vs. single skip notice) and proposes a scope.

  2. Provenance — The original commit referenced fork-internal artifacts (PR docs(F113): button tooltip accessibility spec #19 and PR feat: thread-level background cron task management #91 in mindfn/clowder-ai) that don't resolve in this repo. feat(F192): evidence-source prerequisite gate for eval scheduler #1353 replaces those with repository-local evidence anchors.

  3. Probe target — Valid architectural concern. The current probe checks the scheduler process's telemetryHandle.getMetricsText, which is correct only when scheduler == evidence source (single-process deployment). feat(F192): evidence-source prerequisite gate for eval scheduler #1353 lays out three options (remote probe, local probe, configurable) and notes the current deployment shape.

  4. ownerCatId — Will split this into a separate commit or its own PR.

  5. Codex P2 (skip notice text) — Agreed. The notice should match the detected failure mode, not always recommend HMAC_SALT.

I'll wait for the issue to be triaged/accepted before revising this PR. Keeping it open as a reference for the implementation direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs-info Waiting for additional information from reporter triaged Maintainer reviewed, replied, and made an initial triage decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants