Repository navigation
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r args Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…evaluate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… history Four fixes to the max_reflection_cost tracking added for gepa 0.1.4: 1. GEPA.compile() now raises ValueError when max_reflection_cost is set but dspy.settings.disable_history is True or max_history_size is 0. TrackedReflectionLM derives its cost totals from the reflection LM's history, so with history disabled the stopper's totals stay at 0.0 forever and spend is silently unbounded. Checked in compile() (not __init__) since settings can change between construction and compile. 2. TrackedReflectionLM now snapshots baseline cost/token sums at construction and reports max(0, current - baseline). Previously a reflection_lm reused across compile() calls entered a new run with the budget already "spent" from the prior run's history, tripping the stopper at iteration 0. Since DspyAdapter.__init__ constructs a fresh TrackedReflectionLM per compile, this scopes the budget to the run. Docstring notes this does not protect against sharing the LM instance as both task and reflection LM within the same run. 3. DspyAdapter.batch_evaluate's worker threads now deep-copy a shared usage_tracker override before running, mirroring dspy's ParallelExecutor (dspy/utils/parallelizer.py), so each worker tracks its own usage instead of contending over one shared tracker. 4. Documented that max_reflection_cost complements rather than replaces the metric-call budget, and that multi-proposal sampling strategies can evaluate up to 8 candidates concurrently (each using up to num_threads threads), multiplying effective LM concurrency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d08ca7d3fb
ℹ️ 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".
| @staticmethod | ||
| def _sum_cost(history) -> float: | ||
| return sum(entry.get("cost") or 0.0 for entry in history if isinstance(entry, dict)) |
There was a problem hiding this comment.
Count typed LM history entries toward reflection cost
When reflection_lm uses DSPy's typed LM contract or runs with experimental=True, its history contains LMHistoryEntry objects, which implement Mapping but are not dict instances. This filter therefore ignores every reflection call and leaves total_cost at zero, so GEPA's max_reflection_cost stopper never fires and can allow unbounded spend. Accept mapping-like entries rather than only concrete dictionaries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 76216b2 — the filter is now isinstance(entry, Mapping), so typed LMHistoryEntry objects count toward the budget. Regression test: test_tracked_reflection_lm_counts_mapping_history_entries.
| @property | ||
| def total_cost(self) -> float: | ||
| # max(0, ...) also tolerates history eviction at settings.max_history_size: if entries this | ||
| # instance counted toward the baseline get evicted, the raw sum can dip below the baseline. | ||
| return max(0.0, self._sum_cost(self.lm.history) - self._baseline_cost) |
There was a problem hiding this comment.
Preserve cumulative cost when LM history evicts entries
When a run makes more calls than dspy.settings.max_history_size, LM.update_history() evicts old entries, so this recomputed sum is only a rolling window rather than cumulative spend. For example, with a history size of 1 and $1 calls, total_cost remains $1 forever and a $2 budget never stops the run; a nonempty baseline can keep it at zero even longer. Since compile() explicitly accepts any nonzero history size, cost must be accumulated independently of the bounded history window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 76216b2 — TrackedReflectionLM no longer recomputes from the history window. It accumulates each entry once (keyed by uuid) into run-scoped running totals, synced on every wrapper call, every stripped_lm_call, and every property read, so instruction-proposal spend is tallied before eviction can occur even at max_history_size=1. Remaining caveat (documented): a custom proposer making more direct LM calls than the window holds within a single proposal step can still have the excess evicted before the next sync. Regression test: test_tracked_reflection_lm_survives_history_eviction reproduces the 3×$0.60 / window=1 scenario and asserts $1.80.
Greptile SummaryGEPA now accumulates reflection spend independently of the bounded LM-history window, rejects reflection-cost limits when history cannot support accounting, and preserves callback parentage for concurrent batch evaluations. Executed checks disproved the two previously reported failures: three $0.60 reflection calls with a one-entry history window correctly total $1.80, and concurrent worker callbacks retain the enclosing trace as their parent. Confidence Score: 5/5No blocking failure remains. No accepted P0 or P1 finding remains after exercising reflection-budget accounting, history validation, and concurrent callback propagation.
What T-Rex did
Reviews (4): Last reviewed commit: "fix(gepa): preserve reflection cost vali..." | Re-trigger Greptile |
…istory entries Addresses two P1 review findings on stanfordnlp#10161: - isinstance(entry, dict) filtered out dspy's typed LMHistoryEntry objects (Mapping, not dict), zeroing the budget on the typed LM path; the filter is now isinstance(entry, Mapping). - Totals were recomputed from the evictable history window, so spend beyond settings.max_history_size vanished from the tally. TrackedReflectionLM now accumulates entries once each (keyed by uuid, synced on every wrapper call, stripped_lm_call, and property read) into run-scoped running totals, which also replaces the construction-time baseline snapshot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses a P2 review finding on stanfordnlp#10161: ACTIVE_CALL_ID is a ContextVar separate from thread_local_overrides, so worker-thread evaluations emitted callbacks as new root traces instead of children of the enclosing GEPA trace. Wrap the worker fn with _bind_active_call_id, mirroring ParallelExecutor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@greptile review |
|
Thanks for working on this. GEPA |
Amp-Thread-ID: https://ampcode.com/threads/T-019ffe40-c240-70bb-8cb2-db33146ed8e6 # Conflicts: # uv.lock
Amp-Thread-ID: https://ampcode.com/threads/T-019ffe40-c240-70bb-8cb2-db33146ed8e6 Co-authored-by: Isaac Miller <isaac@cmpnd.ai>
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
📝 Changes Description
This PR bumps the pinned
gepadependency from 0.1.1 to 0.1.4 and surfaces the new upstream capabilities as first-classdspy.GEPAfeatures:gepa[dspy]==0.1.4— every API surface DSPy touches was verified unchanged upstream; the full GEPA test suite passes with no code changes required.dspy.GEPAconstructor args, forwarded verbatim togepa.optimize:sampling_strategy/selection_strategy(gepa 0.1.4's multi-proposal iterations),acceptance_criterion("strict_improvement","improvement_or_equal", or a custom criterion),max_reflection_cost(USD budget for reflection spend),wandb_attach_existing/mlflow_attach_existing/tracking_key_prefix(embed GEPA in a training loop that owns the tracker run).TrackedReflectionLM— the reflection callable passed to gepa now exposes real cost/token totals derived from the reflection LM's history, so gepa'smax_reflection_coststopper actually works (a bare lambda would be wrapped in gepa's zero-costTrackingLMand the stopper would never fire). The budget is scoped to the run via a baseline snapshot, andcompile()raises ifmax_reflection_costis set while LM history is disabled.DspyAdapter.batch_evaluate— gepa 0.1.4 discovers this seam automatically; multi-proposal iterations now evaluate up to 8 candidates concurrently (each still parallelized across examples bynum_threads), with dspy's thread-local settings overrides propagated into the worker threads and per-worker usage-tracker isolation matchingParallelExecutor's pattern.get_adapter_state/set_adapter_state— persists the adapter's RNG state through gepa's checkpoint save/resume solog_dirresumes are deterministic.✅ Contributor Checklist
traceargument now receives a real trace during valset scoring where 0.1.1 passedtrace=None, which can change valset scores for trace-sensitive metrics. This is upstream gepa's design (thebatch_evaluatecontract requires trajectories), not something this PR opts into.max_metric_calls), so runs will not be iteration-for-iteration identical to 0.1.1 at the same budget.gepa_kwargswould now get a duplicate-kwargTypeError— impossible in practice, since DSPy was pinned to 0.1.1 where these parameters did not exist.🤖 Generated with Claude Code