[Harbor 1/4] vero core: split visibility, budget ledger, evaluation engine refactor

[varunursekar]

Draft · Stack 1 of 4 — review/merge in order (1→4). No Harbor concepts here; foundations the rest builds on (and useful on their own).

• 3-tier split visibility: SplitAccessLevel gains no_access (hidden) alongside visible / non_viewable (aggregate-only).
• BudgetLedger (core/budget.py): metered, optionally-durable per-split eval budget.
• Staged eval: TaskOutput is a model; inference and scoring run as separate resumable disk-backed stages, with label-field scrubbing before inference.
• Evaluation refactor: consolidate under vero/evaluation/ as a shared EvaluationEngine + Evaluator, port ExperimentRunnerTool onto it, and add the injectable EvalStrategy seam (Mode B's extension point).

Review focus: the visibility enum, BudgetLedger, the EvaluationEngine API, and the strategy seam. Much of evaluation/ is relocated code (skim).

Part of the Harbor integration (full squashed view: #2). Stack: this → [2/4] sidecar → [3/4] compiler → [4/4] docs.

🤖 Generated with Claude Code

Greptile Summary

This PR adds the core evaluation pieces for split visibility, budget metering, and staged evaluation. The main changes are:

• A new no_access split tier and fail-closed split access resolution.
• A BudgetLedger for per-split run and sample budgets.
• A shared EvaluationEngine used by evaluation frontends.
• Staged inference/scoring persistence with label-field scrubbing.
• An injectable evaluation strategy seam for alternate execution modes.

Confidence Score: 4/5

Merge is blocked by the durable budget reload behavior because persisted spend state is ignored after restart.

The implementation is generally well-scoped, but the persistence path for budget accounting does not restore recorded remaining values, which can allow repeated spending across process restarts.

vero/src/vero/core/budget.py

T-Rex Logs

What T-Rex did

• Reproduced the persisted budget reset behavior by running the focused tempfile repro script with PYTHONPATH=vero/src, which used a direct module-load fallback due to a missing toml.
• The first reserve reduced remaining_run_budget to 0 and remaining_sample_budget to 2 and wrote those values to the persisted JSON.
• Constructed a fresh BudgetLedger with the same persist_path and configured totals to confirm remaining_run_budget=1 and remaining_sample_budget=5 after restart.
• The fresh ledger allowed check(3) and a second reserve, proving it spent from reset totals rather than the persisted remaining budget.
• The split-visibility gating behavior changed from before to after, with the after state satisfying the requested gating and policy budget-tier contracts.
• The staged eval resume workflow moved from a TypeError caused by create_task with an unexpected keyword argument to a successful after-state with has_run_inference_stage True and a persisted JSON showing output, counters, and later scoring updates.
• The evaluation engine strategy before showed a ModuleNotFoundError for vero.evaluation.engine, while the after shows head imports ok, engine_type as EvaluationEngine, and budget decrement during evaluation.
• A supplemental attempt shows pytest is unavailable in the environment, so the pytest run failed for environmental reasons rather than PR code behavior.

_{Ran code and verified through T-Rex}

Comments Outside Diff (1)

1. vero/src/vero/core/budget.py, line 175-193 (link)

   Blocking I/O inside asyncio.Lock in reserve()

   _flush() calls Path.write_text() (synchronous) while reserve() holds self._lock — an asyncio.Lock. Any concurrent coroutine that awaits reserve() will be blocked for the full duration of the file write; on a loaded Harbor sidecar with rapid eval requests, this stalls the entire event loop. The fix is to wrap the blocking writes in asyncio.to_thread() (or use aiofiles) and await them inside reserve(), keeping the lock protecting only the in-memory check+decrement step.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: vero/src/vero/core/budget.py
   Line: 175-193
   
   Comment:
   **Blocking I/O inside `asyncio.Lock` in `reserve()`**
   
   `_flush()` calls `Path.write_text()` (synchronous) while `reserve()` holds `self._lock` — an `asyncio.Lock`. Any concurrent coroutine that awaits `reserve()` will be blocked for the full duration of the file write; on a loaded Harbor sidecar with rapid eval requests, this stalls the entire event loop. The fix is to wrap the blocking writes in `asyncio.to_thread()` (or use `aiofiles`) and `await` them inside `reserve()`, keeping the lock protecting only the in-memory check+decrement step.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
vero/src/vero/core/budget.py:102-110
**Persisted budgets reset**

`_flush()` writes `remaining_sample_budget` and `remaining_run_budget` to `persist_path`, but constructing a ledger with that same path never reads the file back. After the durable Harbor sidecar spends budget and restarts, `BudgetLedger(configured_budgets, persist_path=...)` rebuilds fresh `SplitBudget` objects with their original totals, so callers can spend the same run/sample budget again even though the JSON file already recorded it as used.

_{Reviews (2): Last reviewed commit: "Merge pull request #7 from scaleapi/harb..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[shehabyasser-scale]

_validate_budget_splits() only checks budget splits against the dataset; it never reconciles budget against split_accesses. These two lists jointly enforce visibility (budget = which splits the agent may RUN; split_accesses = what gets WRITTEN back per tier). An init-time cross-check would close a real hole: every agent-budgeted split should be explicitly tiered non_viewable/no_access, raising on viewable-or-unlisted. Combined with tier_for_split defaulting an unlisted split to viewable (over in harbor/protocol.py), the concrete failure is: budget a split, forget to tier it, and the agent both runs it and gets its per-sample labels written to its own volume.

[shehabyasser-scale]

This docstring is honest that no_access gating is implicit — it relies on those splits being absent from the budget ledger so reserve() raises. That couples a security tier to a budgeting structure: a split that is in the ledger but should be no_access would not be rejected here. Recommend evaluate() consult split_accesses directly and hard-reject tier == no_access for non-admin callers, independent of ledger membership.

[shehabyasser-scale]

reserve() holds self._lock (an asyncio.Lock) across this synchronous tmp.write_text() + replace(). In the durable sidecar path under concurrency this stalls the event loop on every reservation. Keep the lock around only the in-memory check+decrement, and push the flush out with await asyncio.to_thread(...). (Confirms Greptile's P1.)

[shehabyasser-scale]

On the design question raised in review (should split access be a dataset property checked in real time?): yes. I implemented the minimal version in the fix PR for this branch (#7) and wrote up the full reasoning there: #7 (comment) (budget vs access as separate axes, trust ownership, flexibility, and the deferred DatasetInfo follow-up).

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	pypi/​fastapi@​0.137.1

View full report

[greptile-apps]

T-Rex pricing update — T-Rex was free through June 2026. Effective July 1, 2026, T-Rex adds 2 credits on top of the standard 1-credit review (3 total). T-Rex settings

[greptile-apps]

Persisted budgets reset

_flush() writes remaining_sample_budget and remaining_run_budget to persist_path, but constructing a ledger with that same path never reads the file back. After the durable Harbor sidecar spends budget and restarts, BudgetLedger(configured_budgets, persist_path=...) rebuilds fresh SplitBudget objects with their original totals, so callers can spend the same run/sample budget again even though the JSON file already recorded it as used.

Artifacts

Repro: focused persisted budget reset script

• Contains supporting evidence from the run (text/x-python; charset=utf-8).

Repro: execution output showing persisted remaining budgets ignored after restart

• Keeps the command output available without making the summary code-heavy.

_{Ran code and verified through T-Rex}

Prompt To Fix With AI

This is a comment left during a code review.
Path: vero/src/vero/core/budget.py
Line: 102-110

Comment:
**Persisted budgets reset**

`_flush()` writes `remaining_sample_budget` and `remaining_run_budget` to `persist_path`, but constructing a ledger with that same path never reads the file back. After the durable Harbor sidecar spends budget and restarts, `BudgetLedger(configured_budgets, persist_path=...)` rebuilds fresh `SplitBudget` objects with their original totals, so callers can spend the same run/sample budget again even though the JSON file already recorded it as used.

How can I resolve this? If you propose a fix, please make it concise.