feat: public evaluation and optimization suite - #13
Draft
jdehorty wants to merge 124 commits into
Draft
Conversation
This was referenced Jun 2, 2026
jdehorty
force-pushed
the
feat/eval-and-optimization
branch
5 times, most recently
from
June 3, 2026 17:19
7f3f970 to
b13aecb
Compare
jdehorty
force-pushed
the
feat/eval-and-optimization
branch
from
June 3, 2026 17:22
b13aecb to
74a3a5e
Compare
…ation # Conflicts: # docs/skills-catalog.md # skills/cesiumjs-entities/SKILL.md # skills/cesiumjs-primitives/SKILL.md # wiki/Domain-Mapping.md
- Updated public-status.json with new evaluation results for multiple skills, including changes in iteration numbers, scores, and decision statuses. - Enhanced SKILL.md files for cesiumjs-entities, cesiumjs-interaction, cesiumjs-primitives, cesiumjs-terrain-environment, cesiumjs-time-properties, and cesiumjs-viewer-setup with additional guidelines and clarifications to improve user understanding and performance. - Added new tips for silhouette rendering, framing techniques, and translucency effects to enhance visual evaluations.
Replace the removed `claude` CLI adapter with a shared, provider-neutral agent-CLI harness, and add cardinal-panorama multi-shot visual grading. - New neutral `harness/` package as the single source of truth: subprocess env hardening (scrubs OPENAI_API_KEY), opencode/codex CLI wrappers, model discovery, harness selection, and vision-fallback policy. - Optimization adapters become thin delegators (agent_cli/opencode_cli/ codex_cli); delete claude_cli.py; the evaluation judge adapter delegates to `harness` (keeping the evaluation->optimization import boundary intact). - Billing routing: text runs on the Copilot subscription, image/vision calls fall back to the codex (ChatGPT) subscription; the metered OpenAI API key is never used. - Judges and scorecard attach multi-shot screenshots and grade across a cardinal panorama (screenshot_mode); add run-metadata/scorecard schema fields. Tests cover the harness package and adapters; CI runs harness/tests.
… grading Add `screenshot_mode: cardinal_panorama` and expanded visual_expectations to every scenario manifest and its derived public case, and refresh baseline content hashes and coverage so validate-evals stays consistent.
When a public/no-token scenario asks for an identifiable landmark or skyline, add an explicit visual surrogate (tower/mast/skyline bar/labeled marker) so the subject is inspectable, rather than relying on a map label or tiny point.
Vite/React evaluation console (apps/evaluation-console) plus Python launchers for the evaluation/audit review viewers. Build artifacts, node_modules, and __pycache__ are gitignored; only source is tracked.
jdehorty
marked this pull request as ready for review
July 14, 2026 15:42
arobison
reviewed
Jul 14, 2026
arobison
left a comment
arobison
left a comment
There was a problem hiding this comment.
A code review isn't the best way for me to give feedback. Happy to have you walk me through running code or anything you'd like feedback on during standup or another time, if that's helpful.
This was referenced Jul 15, 2026
Both agent-CLI harnesses now default to gpt-5.6-sol pinned to the low reasoning-effort variant, replacing the stale gpt-5.5 default and the previous "auto" fallback for codex (which silently deferred to whatever model happened to be configured locally). Also fixes a real bug: invoke_agent() resolved a codex reasoning-effort variant and then discarded it, so effort was never actually controllable for the codex harness. invoke_codex() now accepts reasoning_effort and forwards it via `-c model_reasoning_effort=<level>`. Updates CLI help text and docs (README, wiki, run-metadata schema) to match, and adds test coverage for the new defaults and effort forwarding.
build_scorecard() and run-scorecard.py now accept --model and --model-variant, recorded under artifacts.model / artifacts.model_variant alongside the existing harness and harness_judge fields. This is additive and optional, so existing scorecards stay schema-valid; a run that doesn't pass these flags simply omits them, which downstream consumers must render as "not recorded" rather than guessing a default.
harness-registry.json is a bona fide, data-only description of the two
agent-CLI harnesses (Codex CLI: OpenAI ChatGPT subscription, fully
multimodal; OpenCode CLI: GitHub Copilot subscription, text-only since
Copilot disables the multimodal endpoint account-wide) and their model
catalogs: tier, relative cost band, native vision, effort levels, and
context. Adding a harness or model is adding an entry here, no code
changes required.
insights_data.py aggregates two new read-only truths from the artifacts
on disk:
- registry(): the declared registry, with default_model/default_effort
overlaid from the live harness/models.py at read time so the console
always reflects what a run started today would actually use.
- insights(): observed performance grouped by (harness, model, effort)
combination from the optimization generated/*.meta.json sidecars,
including win-rate stability (stddev across scored iterations).
server.py exposes /api/registry, /api/insights, and /api/run-cases (light
per-case rows for a comparison-baseline run), and stamps recorded model/
model_variant/harness_judge provenance onto /api/runs and iteration
summaries. Provenance fields are always nullable; a run that didn't
record one renders as unrecorded rather than defaulting silently.
Adds the official Cesium logomark (cesium.com/logos) for the console's
branding, and format.ts helpers for cost-tier and duration display.
Clarify the orientation skill description while preserving the explicit "Use when" trigger required for passive activation.
The codex credential is a ChatGPT-subscription OAuth pair that the CLI refreshes in place, rotating the refresh token server-side as it goes. The previous behavior reseeded auth.json from the frozen repository secret on every run, which discards the rotated pair and eventually resurrects a revoked one: an invalid_grant lockout on a timer. OpenAI's CI guidance is explicit that the runner-resident file is the live credential and the secret is bootstrap material only. Three changes, one per failure mode: - setup-agent-harness seeds the codex credential only when the runner has no parseable auth.json (plus a force-reseed input for the recovery path). OpenCode keeps overwrite-every-run: its credential is a static key. - Codex-capable lanes (skill-eval live, baseline-audit qualitative, optimization-loop) prefer the new CODEX_RUNNER_LABELS variable, pinning them to the one runner that holds the session; two runners refreshing copies of one session invalidate each other. - A twice-weekly keepalive lane exercises the credential so the session never crosses the ~8-day staleness cliff during a quiet week, and doubles as the reseed entry point after a human re-login.
Document render-aware current-view readiness for Cesium3DTileset: fromUrl resolving does not mean the current view has rendered, so await camera framing and gate on tilesLoaded across consecutive postRender frames before checking settled state.
Preserve explicit passive skill activation while clarifying that the 3D Tiles skill applies whenever a task involves the listed workflows.
Model ids like openai/gpt-5.5 and reasoning-effort levels like high are configuration, not credentials, but ten expressions across two workflows resolved them through a secrets.OPENCODE_* tier sitting between the repository variable and the literal default. Nothing was ever stored there. The cost is that the pipeline read as though it held a dozen secrets when it holds exactly one (CODEX_AUTH_CONTENT): answering "what credentials does CI have" meant hand-filtering ten non-credentials out of the grep. It also hid the effective value from anyone reading the settings page, since a secret's value is not displayable, and bought nothing in exchange, as a model id is not sensitive. Precedence is now inputs, then vars, then the literal default. The four real credential secrets (CESIUM_ION_TOKEN, CODEX_AUTH_CONTENT, OPENCODE_AUTH_CONTENT, OPENCODE_CONFIG_CONTENT) and the auto- provisioned GITHUB_TOKEN are untouched. Behavior is unchanged: none of the removed names is set as a repository secret, so every one of those expressions already fell through to vars or the default.
Hosted concurrency can queue the gate, secret scan, and skill-eval discovery for a long time while a configured self-hosted pool sits idle. Route every pull_request-reachable job through a fork guard rather than a bare RUNNER_LABELS expression: a pull request whose head repo is a fork is pinned to ubuntu-latest no matter what the repository variable says, so a fork's npm lifecycle scripts can never reach a persistent shared node. A merge_group payload carries no pull_request object, so a fork test there would be not merely false but blind, on the one event whose merge commit already contains the contributor's code. The expression tests event_name first and pins merge_group to hosted unconditionally. workflow-safety.sh rule 5 changes from a ban to an exact-shape check: a blocking lane may name RUNNER_LABELS only in the one guarded expression form, compared as a whole normalized expression rather than as substrings, which the || operand order could otherwise defeat (putting RUNNER_LABELS first contains every token a substring check looks for and still routes a fork to the pool). A bare self-hosted literal stays banned outright.
Eval runs were silent between launch and verdict: the agent CLIs emit rich event feeds, and nothing surfaced them. A new progress reporter prints one flattened line per event with a heartbeat when the feed goes quiet (EVAL_HARNESS_STREAM and EVAL_HARNESS_HEARTBEAT_SECONDS tune it), runSubprocess gains per-line stdout and stderr callbacks, and event translators cover codex, opencode, and claude-code with delta tracking, falling back to plain-text line reporting for pi and copilot.
The codex bootstrap credential moves to CODEX_AUTH_JSON_B64 in a new codex-inference deployment environment, with shape validation (auth_mode chatgpt plus both tokens present in the decoded file) before install and only a byte count ever printed. A new main-only provisioning workflow is the sole workflow that receives the secret; it writes the credential to the persistent runner selected by CODEX_RUNNER_LABELS, with a force-reseed input for recovery after a fresh login. The keepalive lane is stripped of all secrets and reuses the runner-resident file, failing with a runbook instead of silently reseeding. Credentialed skill evaluation moves off pull requests entirely: the live lane runs from main pushes and manual dispatch only, with an in-job ref re-assertion independent of the workflow triggers. Every external GitHub action is pinned to an immutable commit SHA, and CODEOWNERS covers the workflows, actions, scripts, codex harness code, and the lockfile, since each of those surfaces can redirect what CI executes.
The provision and keepalive lanes ran actions/setup-node on a persistent runner, where its cache restore/save can outlive the work it is meant to speed up and was timing out the job. Both lanes now assert node and npm are preinstalled in the existing runner-guard step, failing fast with a clear error instead, and the keepalive guard runs under set -euo pipefail so a partial guard failure cannot slip through.
gate.sh records every stage's status and duration to a TSV, and a summarize-on-exit trap always renders summary.md, including the fail-fast paths that previously produced no summary at all. The new render-gate-summary script turns stages, skill-contract findings, deterministic scores, and fixture reconciliation into job-summary Markdown, so a red gate explains itself on the run page instead of in a log scroll. Two runner accommodations ride along: cold-start setup gets enough timeout to finish, and the slow ephemeral npm cache path is skipped where it cannot pay for itself.
setup-node v5 auto-enables remote package-manager caching when package.json declares packageManager, and on a persistent self-hosted runner the cache restore/save round-trip can outlive the build and tests it is meant to speed up, while the runner already holds npm's local cache between jobs. setup-eval now branches on runner.environment: GitHub-hosted keeps the lockfile-keyed npm cache, self-hosted runs with package-manager-cache disabled. The optimization loop's own setup-node call switches the same way.
Three defects around the server's less-traveled paths: - A --state-dir override resolved differently at boot (the raw override) and on /api/select-run (a per-run subdirectory), so re-selecting the current run silently dropped every persisted grade and a subsequent handoff 409'd. Both paths now resolve through one stateDirFor helper that keys review state per run. - With a state dir outside the repository, the optimization launch passed an absolute focus path that the launch validation rightly refuses (the path becomes subprocess argv), dead-ending the workflow. The focus file is now mirrored into the gitignored artifacts tree so the argv stays repo-contained. - The checkout identity guard compared resolved paths, so symlink aliases of one checkout (/tmp vs /private/tmp) read as a mismatch and 409'd every dev request. Roots are now compared by real path. Also note in the serve --host help that the console is loopback-only: the Host allowlist rejects non-loopback clients by design.
Every sibling path flag (--emit-visual-review, --emit-cases, --bundle-root, --journal, --output-dir) resolves relative to the repo root, but --visual-review read from the current directory, so the documented emit-then-consume round trip failed with ENOENT from any subdirectory.
start() short-circuited only when the proxy was running AND healthy. A running proxy that failed its transient health probe fell through to a second launch on the same port: the new child could not bind, died, and had already overwritten the pid file, leaving stop() killing nothing, status reporting stopped, and the real process orphaned on the port. A running-but-unhealthy adapter now fails fast with the remediation.
The token fixture was written before the adapter checks, whose throws run outside the try/finally that removes it, leaking one fixture directory per failed preflight.
The Decide view resolved a candidate's baseline bundle by exact directory name only, so after a scenario rename the iteration detail showed an empty baseline column although the bundle exists. Apply the same id-prefix fallback evaluation/baselines.ts uses everywhere else.
A repository with no codex runner variable configured previously got a guaranteed-red twice-weekly keepalive cron and, on the first skills-touching main push, a live skill-eval job that burned five minutes of setup before failing at credential seeding. A configuration gap is not an evaluation regression, and a permanently red scheduled lane trains maintainers to ignore red. - skill-eval's eligibility preflight now also requires CODEX_RUNNER_LABELS, routing unconfigured repositories through the existing loud green skip. - the keepalive job skips at the job level until a runner variable exists. - the provisioning workflow gains a hosted runs-on fallback so an unconfigured dispatch reaches the runner guard's curated error instead of an opaque fromJSON failure.
GitHub reads only the first CODEOWNERS it finds (.github/ before the repository root), so with both files present the root file carrying the gate-guard rules (eval.config.json, .gitleaks.toml, the evaluator sources, the tests, the secret scanner) was silently inert, and the comments that lean on those guarantees were overstating them. Merge everything into .github/CODEOWNERS, delete the root file, and reword the pr-gate comment that claimed a repository VARIABLE can sit under CODEOWNERS (only the workflow files carrying the expression can).
Artifacts are immutable per name within a run and the upload steps run under if: always(), so "Re-run failed jobs" hit a name conflict on the second attempt's upload even when the job itself now passed.
The push trigger still listed a historical working branch, so a push there could republish the live wiki from unreviewed content.
The GitHub-hosted setup-node branch said "deliberately uncached" while that branch actually keeps the lockfile-keyed npm cache (and should: hosted runners start cold). The stale text described the self-hosted branch's rationale.
Multi-harness evaluation platform, console workflow, and deterministic CI gate
Screenshots could fire while the scene was still loading, producing gray/blurred imagery (judge failure mode gray_unloaded_globe, e.g. eval-114). The old gate passively polled globe.tilesLoaded, which reads a vacuous true right after a camera move (every panorama orbit shot) and while an async Terrain is resolving (terrainProvider undefined). Port the readiness logic CesiumGS/cesium's own test suite uses, verified against the 1.142 tag (pollToPromise + renderForSpecs, GlobeSurfaceTileProviderSpec updateUntilDone, Cesium3DTilesTester, loadAndZoomToModelAsync, DataSourceDisplay.ready): - Drive viewer.render() + scene.requestRender() inside every poll; readiness getters only update during a rendered frame. - Gate on globe queues + tilesWaitingForChildren + terrain provider defined, shown ImageryLayer.ready, tileset tilesLoaded, primitive/ collection boolean ready (skipping show=false subtrees and empty collections that can never become ready), dataSourceDisplay.ready. - Capture the constructed viewer via a Viewer/CesiumWidget prototype.render hook (the namespace exports are getter-only) so a candidate's local `const viewer` can no longer silently disable the settle gate, the panorama orbit, and scene-state extraction. - Reset the quiet streak while the camera moves (moving cameras cull tile requests, so tilesLoaded reads stale-true), with wrap-aware angular epsilons and a motion escape for deliberately animated scenes. - Contain terminal failures honestly: errorEvent-latched imagery providers stop gating and are reported, later shots run a reduced budget once one times out, Ion auth detection covers 403/429, and timeout evidence names its blockers. - Propagate shot-level wait_for_tiles opt-outs into synthesized cardinal_panorama specs (previously dropped silently). - Stamp capture_protocol_version (settle-gate-v2) into bundle metadata and the environment fingerprint; baselines rendered under the old gate pair as environment mismatches and should be re-rendered. Verified by unit tests executing the real in-page probe against mock viewers (50 tests in browserRunner alone, 239 total) and a live headless-Chromium smoke run: four-shot panorama, every shot settled before capture, sharp imagery edge to edge.
…e colors Pure fixtures runs (evaluator self-tests) no longer appear in the Dashboard's headline surfaces: Recent Runs, Run Pass Rate, freshness chip, Run Score Trend, and the Harnesses KPIs all read a new store-derived evalRuns list, and `cesium-eval serve` never auto-focuses a fixtures run. They stay browsable in the Run Browser and Harnesses station, now explicitly labeled "synthetic"; a deliberately focused one carries a Synthetic pill on the dashboard hero. Score magnitudes are graded instead of verdict-binary: healthTone(pct) maps >=90% green, 70-89% amber, <70% red across the Recent Runs score/bar, trend dots, and KPI tiles, while PASS/FAIL/INCOMPLETE chips keep their binary colors. A 93% FAIL now reads as a red verdict beside a green score instead of an all-red row.
- Camera: composable Controller framework (5 controllers, ControllerHost, MouseButton, ScreenSpaceInputBindings), canonical asset-inspection setup - Interaction: Scene.snap experimental edge-snap picking + SceneSnapResult - 3D Tiles: UrlTemplate3DTilesDataProvider base class, automatic terrain draping of clamped vector tile content - Imagery: draped-on-tileset feature-info picking (1.144) - Models: BENTLEY_materials_planar_fill now supported (wireframeFill no-op), KHR_mesh_primitive_restart, edge-visibility typed-array memory note - Materials: Texture.defaultColor placeholder note - Domain-Mapping: 1.144 baseline, +12 symbols, August coverage section - eval.config: browser cesiumVersion 1.142 -> 1.144
Verified against the published cesium@1.144 npm package (Cesium.d.ts + runtime exports), CHANGES.md, and Khronos glTF PR #2569: - KHR_mesh_primitive_restart is NOT ratified (Khronos spec PR still open); drop the 'ratified successor' claim in REFERENCE.md and Domain-Mapping - GRID_TARGET_SEGMENTS_PER_CELL leaks into Cesium.d.ts only; it is undefined at runtime, so say that instead of 'accidental top-level export' - Camera controller tuning knobs vary per class (panSpeed/inertia* on map/elevator, dampingEnabled on tilt-orbit/zoom, angleThreshold + nested controllers on hybrid); stop implying one shared knob set - 'critically damped' -> 'damped easing' (matches shipped JSDoc wording) - Keep SSCC *EventTypes as the canonical path for input remapping so the Controller framework note cannot misdirect remap tasks (camera eval-010) - Interaction hover recipe: entity material-swap variant + graphics-type fidelity note (interaction eval-003 rendered rectangle instead of polygon) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…js-v1.144-skills) Make skills/ byte-identical to PR #27 (49e0612) so the operational eval branch renders baselines from the same wording that ships to main: - camera: three-way merge of main's #24 (flyToBoundingSphere target framing, quick-reference rows) with the 1.144 Controller framework - bump the version-baseline line to 1.144 in the 8 skills the release sync did not otherwise touch (core-utilities, custom-shader SKILL + REFERENCE, entities, primitives, spatial-math, terrain-environment, time-properties) Validated as-is: deterministic gate PASS; 46/47 live scenarios pass (camera eval-006 settle-gate failure is host-load-caused, control-proven with a byte-identical previously-passing source). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Publishes the evaluation and optimization suite as a clean public branch built on current
origin/main.This replaces the older draft PR #6. The previous branch carried older tuning-layout history and commit messages that were not ideal for a public-facing publication branch.
What changed
evaluation/framework, deterministic scorecards, visual quality review flow, and baseline-audit dashboard.optimization/framework, scorecard-guided proposer loop, public-safe optimization results, and dashboard artifacts..env, and untracked generated optimization artifacts out of the branch.pytest.iniso CI and local barepytestinvocations resolve the top-levelevaluationandoptimizationpackages consistently.Validation
optimization/scripts/check-secrets.shpython3 optimization/scripts/check-public-artifacts.pygitleaks detect --redact --log-opts="origin/main..HEAD" --report-format json --report-path /tmp/gitleaks-feat-eval-public.jsonpython3 evaluation/scripts/validate-evaluation.pypython3 optimization/scripts/check-canonical-eval-surface.pypytest -q optimization/tests evaluation/tests