Record what prep wrote so --clean can reach the orphans stems cannot (#84) - #86
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d66322561
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
All three findings were real and one class: the dataset guard was gating the manifest branch only. Fixed in The label-mismatch one was worse than reported — deciding it at The stem match now consults the record when there is one, so a hand-added clip sharing a recorded stem is spared (that was the case the "only removes what prep made" guarantee most needed to cover), and the write is gated on the same question as the removals, so repeating a stray Three tests added, one per hole. 49/49, ruff clean. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796f5c30df
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…84) --clean matches orphans by stem, which structurally cannot reach two cases the normal iterate-on-recordings loop produces. Drop an input between runs and its clips are orphaned under a stem no rerun ever writes. Re-run a basename collision with only one of the pair and the disambiguated `take-1_*.wav` clips are orphaned the same way. Both linger and warn; training and eval glob the whole directory and read them as data nobody asked for. Prep now records the clips it writes in .prep-manifest.json, and --clean uses that record to remove the orphans the stem match misses. The record is what separates a clip prep made from a clip a user curated, so the guarantee that --clean never deletes a file prep did not create still holds. Provenance is not permission, though. "Prep made this file" does not say the file belongs to the dataset being refreshed, and without that second fact a mistyped --output -- positives re-run into the background directory -- would delete everything there, since none of those stems are in the inputs. So the manifest rule only arms when the run looks like a refresh of the recorded set: same label, at least one stem in common. A stray --output shares neither and falls back to the stem rule, which cannot reach past what the run just wrote. The write path refuses to relabel a directory recorded under another label, because doing so would hand a second stray run the matching label it needs. The prior clips of a take this run attempted but wrote nothing for stay excluded, manifest or not: that is the silent re-recording, where they are the only copy. The lingering warning now says which case a file is in, since a take that came back silent wants "check the recording," not the "remove them by hand" the old copy gave every file. Refs #84, #8 wake-20260723T1705-0bf0ee
CI gates on `ruff format --check .`; both files were written by hand and missed it.
The cloud review found the dataset guard was applied to the manifest branch alone, leaving three holes in the same class. A label mismatch disarmed the manifest rule but not the stem match, so a positives run into a negatives directory still deleted that dataset's higher-numbered take_* clips. Worse, deciding this at --clean time is too late: _write_unique has already overwritten take_000.wav with the wrong dataset's audio, while the warning claimed the files were left alone. A mismatched label is now refused before any audio is decoded, beside the sibling precondition that already rejects a non-directory --output. There is no reading of it where writing positives into a directory of negatives is what the user meant. The stem match ignored the record even when one existed, so a clip a user dropped in beside a manifested dataset was deleted by filename shape alone -- the one case the "only removes what prep made" guarantee most needed to cover. It consults the record now, falling back to shape only for a directory that predates manifests. The write path seeded its own stems into a record it had just refused to delete from, so repeating the same wrong --output would find the overlap it needed and start deleting. Removals and the write are now gated on the same question. Three tests, one per hole: a mismatched label refused before writing, a hand-added clip spared under a recorded stem, and a same-label stray that never gains permission by being repeated. 49/49.
796f5c3 to
53120a2
Compare
|
@codex review Current head: |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Closes #84. #85 is merged, and this branch is rebased onto current
main.Problem
The legacy
--cleanrule could only remove higher-numbered clips under a stem written by the current run. It could not safely reach clips from an omitted source or a prior disambiguated stem, while training and evaluation still globbed every clip in the output directory.What changed
.prep-manifest.jsonmapping the dataset label and each resolved source path to the exact clips prep owns.Verification
test_prep_wake_samples.py: 80/80test_brain_bridge.py: 25/2553120a2