Skip to content

HOK-1596: Fix router to aggregate evals from global wavemill install dir#563

Merged
timogilvie merged 1 commit intoauto/integrationfrom
task/router-not-aggregating-records-properly
May 7, 2026
Merged

HOK-1596: Fix router to aggregate evals from global wavemill install dir#563
timogilvie merged 1 commit intoauto/integrationfrom
task/router-not-aggregating-records-properly

Conversation

@timogilvie
Copy link
Copy Markdown
Owner

Summary

  • When running in a non-wavemill repo (e.g. gtm-backend), the router only found records from that repo's per-repo eval file, not the cross-repo aggregated data stored in the wavemill installation directory
  • Added resolveGlobalAggregatedEvalsPath() in evals-paths.ts that resolves to <wavemill-install>/.wavemill/evals/aggregated-evals.jsonl using import.meta.url to navigate up from shared/lib/
  • The router's loadMergedEvalRecords() now tries both the per-repo aggregated path and the global wavemill install path, deduplicating records by ID across both sources
  • Added WAVEMILL_AGGREGATED_EVALS_PATH and WAVEMILL_DIR env overrides as test hooks for the new path resolution

Changes

  • shared/lib/evals-paths.ts — Added resolveWavemillInstallDir() and resolveGlobalAggregatedEvalsPath() with env overrides for testability
  • shared/lib/model-router.ts — Updated loadMergedEvalRecords() to iterate candidate paths (per-repo + global), merging and deduplicating records from all found sources
  • shared/lib/check-routing.ts — Added global aggregated path to the router health check readSource entries
  • shared/lib/evals-paths.test.ts — Tests for new path resolution functions
  • shared/lib/model-router.test.ts — Tests covering global fallback, deduplication, missing-file resilience, and error handling
  • shared/lib/stage-aware-router.test.ts — Tests for stage-aware router with global aggregated evals
  • shared/lib/check-routing.test.ts — Tests for health check with global aggregated source

Test plan

  • Unit tests cover: global path fallback when per-repo path missing, deduplication of records appearing in both sources, graceful handling when neither path exists, parse errors in one source don't prevent loading from the other
  • WAVEMILL_AGGREGATED_EVALS_PATH env override allows tests to inject a fixture file without touching the real wavemill install

Self-review

  • Verdict: ready (exit code 0, 1 iteration)
  • 4 warnings (no blockers):
    • check-routing.ts:169 — Two sources share the aggregated label; could use aggregated-global to avoid ambiguity (cosmetic)
    • model-router.test.ts:95process.chdir() is fragile if tests are parallelized (currently sequential, safe)
    • Plan compliance: eval-aggregator.ts not updated (writer still writes to per-repo path; global reads from wavemill install where eval-aggregate skill writes)
    • Plan compliance: .wavemill/context/router.md docs not updated in this diff

@timogilvie timogilvie force-pushed the task/router-not-aggregating-records-properly branch from 195e1e2 to dff80b7 Compare May 7, 2026 13:37
@timogilvie timogilvie merged commit 8c1b921 into auto/integration May 7, 2026
@timogilvie timogilvie deleted the task/router-not-aggregating-records-properly branch May 7, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant