Skip to content

Reject a malformed BASELINE_SYNC instead of ignoring it - #533

Open
josephdviviano wants to merge 1 commit into
masterfrom
fix/baseline-sync-validation
Open

Reject a malformed BASELINE_SYNC instead of ignoring it#533
josephdviviano wants to merge 1 commit into
masterfrom
fix/baseline-sync-validation

Conversation

@josephdviviano

Copy link
Copy Markdown
Collaborator

_apply_baseline_sync read the payload with .get and skipped the assignment when it returned None:

global_baseline = msg.message_data.get("global_baseline_log_reward")
if global_baseline is not None:
    self._global_baseline = float(global_baseline)

A BASELINE_SYNC with a missing key, a null value, or a non-finite value left the manager on its shard-local threshold and returned normally. The adopted value gates what every assigned worker sends, so that manager filters against a different cutoff from its siblings — the state the message exists to remove — and nothing in the run says so.

It now raises ValueError on a non-dict payload, on a missing or null global_baseline_log_reward, and on a non-finite one. A non-finite baseline keeps every trajectory or none.

testing/test_replay_buffer.py::test_manager_baseline_sync_ignores_missing_payload asserted the old behaviour, so it is replaced rather than added to. The new test is parametrized over all four malformed payloads and checks _global_baseline is untouched.

testing/test_replay_buffer.py and testing/test_dependency_free.py: 100 passed, 1 skipped. Each new case verified to fail against the previous implementation. black, isort and flake8 clean.

_apply_baseline_sync read the payload with .get and skipped the assignment
when it came back None. A sync with a missing, null or non-finite value left
the manager on its shard-local threshold with no signal, so it filtered
against a different cutoff from its siblings — the state the message exists
to remove.

The test that pinned the old behaviour is replaced by one that covers a
missing key, a null value, a non-finite value and a non-dict payload.
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.64%. Comparing base (f39bfc9) to head (614d4bf).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #533      +/-   ##
==========================================
+ Coverage   75.62%   75.64%   +0.01%     
==========================================
  Files          62       62              
  Lines       11329    11335       +6     
  Branches     1670     1672       +2     
==========================================
+ Hits         8568     8574       +6     
  Misses       2234     2234              
  Partials      527      527              
Files with missing lines Coverage Δ
src/gfn/containers/replay_buffer_manager.py 42.10% <100.00%> (+1.88%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant