Repository navigation
Fill missing config keys from packaged defaults on offline replay - #34
Merged
Merged
Conversation
recording.py snapshots tiptop.yml into each run directory, and tiptop-offline reloads that snapshot to reproduce the run. Because set_tiptop_cfg_from_file did a bare OmegaConf.load, any snapshot predating a later config key crashed on attribute access, e.g. when perception.m2t2.apply_bounds (added in #27) is missing from an older recording but read at perception_wrapper.py. Merge the packaged defaults under the loaded config so omitted keys resolve instead of raising. Recorded configs cannot be updated after the fact, so tiptop-offline opts in via fill_missing; every other caller raises, since a live config the user can edit should be fixed rather than silently patched. Adds tests/test_config.py and a test-unit pixi task, as test-integration filters on -m integration and would not collect them. Split out from #30.
- Assert the whole restored section matches defaults, not just one leaf. - Cover the case where a recording carries a key since dropped from the defaults: extra keys are kept and don't raise, since only missing keys are an error.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves offline replay robustness by allowing recorded tiptop.yml snapshots that predate newer config keys to be replayed without crashing, while still preserving strictness for normal (editable) configs.
Changes:
- Add
fill_missingsupport toset_tiptop_cfg_from_file()by merging the loaded config over packaged defaults and raising by default if keys are missing. - Update offline replay (
tiptop-offline) to opt intofill_missing=Truewhen restoringtiptop.ymlfrom a run directory. - Add unit tests covering raise/fill/override behaviors and add a
pixi run test-unittask to run them.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tiptop/tiptop_offline.py | Offline rerun now loads recorded configs with fill_missing=True to tolerate older snapshots. |
| tiptop/config/init.py | Implements default-merge behavior with strict-by-default missing-key detection and an opt-in fill path with warning. |
| tests/test_config.py | Adds unit coverage for missing-key raise behavior, fill behavior, preserving user values, and cached-path semantics. |
| pixi.toml | Adds a test-unit task to run non-integration tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ryanlindeborg
approved these changes
Jul 30, 2026
2 tasks done
2 tasks done
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
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
Splits the config
fill_missingwork out of #30 into its own PR (no experimental / near-placement changes).recording.pysnapshotstiptop.ymlinto each run directory, andtiptop-offlinereloads that snapshot to reproduce a run.set_tiptop_cfg_from_filedid a bareOmegaConf.load, so any snapshot predating a later config key crashed on attribute access — e.g.perception.m2t2.apply_bounds(added in #27) is read atperception_wrapper.pybut absent from older recordings.Changes
config/__init__.py— merge packaged defaults under the loaded config so omitted keys resolve. Missing keys raise by default (a live, editable config should be fixed, not silently patched); callers opt into filling viafill_missing=True.tiptop_offline.py— replay opts in withfill_missing=True, since recorded configs can't be updated after the fact (and warns when it fills).tests/test_config.py+test-unitpixi task — unit coverage for the raise/fill/keep-user-values behavior. Added a task becausetest-integrationfilters on-m integrationand wouldn't collect these.Notes
perception.m2t2.apply_bounds/perception.depth_trunc_m/ thecamerassection as the missing-key fixtures, so the tests stand alone onmainwithout the experimental config block.pixi run test-unit→ 8 passed.