Write spike clusters atomically so a crash cannot truncate spike_clusters.npy

[adityasingh2400]

This fixes cortex-lab/phy#1249. The issue was filed on phy, but the code responsible lives here, in TemplateModel.save_spike_clusters().

The reporter lost a nearly complete manual curation session. phy crashed during a save and spike_clusters.npy was left empty. Every other .npy and .tsv file in the directory was intact, so the only thing actually lost was the one file holding the cluster assignments.

The cause is that save_spike_clusters() called np.save(path, spike_clusters) directly on the destination. np.save opens that path for writing, which truncates it to zero length before a single byte of the new array is written. Between that truncation and the last write there is a window where the old assignments are already gone and the new ones are not yet on disk. Anything that interrupts the process in that window, a crash, a kill, a full disk, leaves the file empty or half written, and the previous contents are unrecoverable. The window is proportional to the size of the array, so it is widest exactly on the long curation sessions where the loss hurts most.

The fix writes the array to a temporary file in the same directory as the destination, fsyncs it so the bytes are really on disk, and then moves it into place with os.replace(). os.replace() is atomic on POSIX and on Windows when both paths are on the same filesystem, which is why the temporary file is created in the destination's own directory rather than in the system temp directory. A reader therefore sees either the complete old file or the complete new one, never a partial one. If anything raises during the write, the temporary file is removed and the existing file is left exactly as it was.

Two small details in the helper that are easy to miss on review. The temporary file is deliberately not named *.npy, because a leftover *.npy file in a phy data directory would be picked up by the spike_*.npy glob in _load_spike_attributes(). And np.save is handed an already open file object rather than a filename, since np.save appends .npy to a filename that lacks the extension but never to an open file object.

The regression test replaces np.save with a stand in that opens whatever it is given for writing, writes a partial header, and then raises, which is what a full disk looks like from the caller's point of view. It asserts that the exception propagates, that spike_clusters.npy still holds its original bytes, and that no temporary file is left behind. It also covers the happy path round trip. On master the test fails with the destination truncated to the 6 bytes written before the failure, which is the reported loss reproduced exactly.

Full suite passes per the README instructions, pytest phylib gives 274 passed. A CHANGELOG entry is added under Unreleased. flake8 reports 7 errors in phylib/io/model.py, all confirmed pre-existing on origin/master at the same lines offset by the added line count, so this adds no new lint errors.

One piece of follow up left out on purpose. Cluster metadata is written by _write_tsv_simple() in phylib/utils/_misc.py and has the same truncate on open exposure, and cluster_group.tsv is real curation data. Making that atomic needs a text mode context manager rather than this helper, in a different module, next to write_tsv() and save_json() and write_text() which all share the pattern. That felt wider than belongs in a fix for this issue, so I have left it and am happy to do it separately if you want it.

Disclosure: this change was prepared with AI assistance. I have reviewed and tested it.

[rossant]

Reviewed after merging #61 and #62. spike_clusters.npy now uses the shared atomic writer, preserves existing group-readable permissions, leaves the old assignments byte-identical on a simulated partial np.save failure, and cleans up temporary files. Focused tests and the full 280-test suite pass locally; changed files add no lint errors beyond the six pre-existing model.py findings.