test: pin the lineage.csv reader/writer contract and flag legacy-scale runs in plots - #22
Merged
Conversation
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.
Two loose ends from the complexity split.
The reader half of the lineage.csv schema was unpinned. Both writers go through one
LINEAGE_COLUMNSlist, so a row cannot misalign with the header, and tests assert that. Nothing checked the other direction:plot_run.load_lineageandclean_runs.load_complexities_from_lineageaddress columns by string literal and fall back to0.0, so a reader looking for a column the writer no longer emits degrades to zeros in silence.That is not hypothetical. I introduced a stale reader deliberately, and the entire pre-existing suite passed:
The new tests write with the real
FileStorageAdapterand read with the real script loaders, so both sides have to agree. They cover the score and both complexity objectives round-tripping, an eviction row resolving to the correct final pool,clean_runsreading the same values, and one guard that every key the reader produces corresponds to a column the writer actually emits — which is what catches an invented column name.For completeness I checked the opposite direction too: a writer-side rename was already caught by the existing storage tests, so only the reader direction was exposed.
Legacy runs are now labelled in plots. Splitting complexity changed the scale of that axis. Pre-split runs read their single
complexitycolumn into the visual objective, so old and new runs both plot fine but on incomparable scales, and an overlay silently mixed them.uses_legacy_complexitydetects the old header and_labelappends(legacy complexity); routing it through_labelmeans all five call sites (score history, Pareto scatter, both convergence series, and the summary panel) pick it up at once. Verified on a mixed overlay:I chose labelling over the stronger option of excluding legacy runs from the Pareto panel: showing the data with a caveat is less surprising than silently dropping a run the user explicitly pointed at.
The new test module skips rather than fails if
scripts/or matplotlib is unavailable, since the analysis scripts are not part of the installed package.451 passed, 13 skipped, ruff clean.