When Sorting Silently Rebinds Your Files to the Wrong Records

A pipeline processed items in groups, flattened them into one slice for a shared conversion step, and wrote each output file as item-%04d.ext using the loop index. Then someone asked for the output to be regrouped by owner. The obvious change produced files that existed, had correct sizes, passed every schema check, and contained the wrong data for every single record.

Nothing threw. The build passed, the tests passed, and the artifact opened in the downstream tool with the right structure. Only the payload was wrong, and payloads are exactly what automated checks tend not to inspect.

The setup that makes this possible

Three ordinary decisions combine into a trap. Each one is defensible alone.

First, a shared processing step takes a flat slice, because downloading and converting items in one batch is cheaper than doing it per group.

Second, that step sorts in place. Sorting by timeline position, by key, by anything, is common when downstream work assumes order. The sort is correct and the comment explaining it is accurate.

Third, output filenames come from the loop index at write time.

for i := range items {
    items[i].LocalPath = filepath.Join(dir, fmt.Sprintf("item-%04d.src", i))
}
sortItems(items)                                     // in-place, reorders everything
for i := range items {
    out := filepath.Join(dir, fmt.Sprintf("item-%04d.out", i))
    convert(items[i].LocalPath, out)
    os.Remove(items[i].LocalPath)                    // source deleted, field not updated
}

Read that carefully. Inputs are named by pre-sort index. Outputs are named by post-sort index. The LocalPath field is never updated to the output, so after conversion it points at a file that no longer exists. The only surviving link between a record and its converted file is its position in the sorted slice.

That works as long as one loop writes the manifest:

for i, item := range items {
    manifest = append(manifest, Entry{Path: fmt.Sprintf("item-%04d.out", i), ...})
}

Same slice, same order, same index. Correct by coincidence.

The change that breaks it

Now group the output by owner. The natural implementation walks each group and numbers its files:

idx := 0
for _, group := range groups {
    for _, item := range group.Items {
        group.Entries = append(group.Entries, Entry{
            Path: fmt.Sprintf("item-%04d.out", idx),   // fresh numbering per group
        })
        idx++
    }
}

Every path in that manifest exists on disk. Every path is distinct. Every file has plausible content of the right type and roughly the right size. And every one of them belongs to a different record, because the flat slice was sorted and the group walk is not in sorted order.

This is the failure mode worth naming: the bug does not produce a missing file or a broken file. It produces a valid file with the wrong contents. Existence checks pass. Size checks pass. Format validation passes. Schema validation passes. The downstream consumer opens it happily.

In an audio pipeline, that means clips land at the right timeline positions with the right durations and the wrong voice. In a document pipeline, it means correct page counts with swapped page bodies. In an image pipeline, correct dimensions, wrong subject. The symptom is always semantic, never structural, which is why it survives every structural check you have.

Why the obvious fix is not enough

The first instinct is to add an owner field so membership survives the sort:

type Item struct {
    // ...
    GroupIdx int   // survives the in-place sort
}

That is necessary, and it does restore grouping. But it does not fix the file binding, because the binding never depended on grouping. It depended on position. Adding GroupIdx lets you rebuild the groups and then regenerate positions inside them, which is exactly the code path that produces the swap.

I made this mistake in the first pass. The field went in, grouping worked, and a reviewer pointed out that the manifest builder still derived paths from a counter. The fix that actually holds is to stop deriving paths at all:

// conversion updates the record to point at its own output
for i := start; i < end; i++ {
    os.Remove(items[i].LocalPath)
    items[i].LocalPath = outputs[i-start]     // the record carries its file
}

// manifest reads the field instead of computing an index
for _, item := range items {
    entries[item.GroupIdx] = append(entries[item.GroupIdx], Entry{
        Path: item.LocalPath,
    })
}

Now the record carries its own artifact reference. Reordering, regrouping, filtering, and parallel processing all become harmless, because nothing recomputes what the record already knows.

The general rule: when a record and its artifact must stay together, store the reference on the record. An index is a position, and positions are the first thing any sort, filter, or partition destroys. The moment two different loops both compute "the Nth file," you have an invariant nobody is enforcing.

Testing this is harder than it looks

I wrote a regression test for exactly this bug. It set up two groups with interleaved sort keys, ran the pipeline stages, and asserted that every entry pointed at its own record's file. It passed.

It also passed when I reverted the fix.

The test had reimplemented the sort and the manifest construction inside the test file, because the production functions were buried in a larger routine that needed network calls and a subprocess. Testing a copy of the logic verifies that the copy is correct. It says nothing about the code that ships.

The fix was to extract the pure parts so the test could call them directly:

func flattenWithGroupIdx(groups []Group) []Item      // tag membership
func sortItems(items []Item)                          // the actual sort, now callable
func groupIntoEntries(groups []Group, items []Item) []GroupEntries

Three small extractions, no behavior change, and the test now exercises shipping code. Then I verified the test actually fails when the bug is present:

$ # revert manifest to index-derived paths
$ go test -run TestKeepsBinding ./...
--- FAIL: TestKeepsBinding
    record a2 bound to wrong file: got item-0000.out, want /tmp/x/item-0000.out
    record a1 bound to wrong file: got item-0001.out, want /tmp/x/item-0001.out
    ... (5 records)
FAIL

That two-minute check is the whole point. A regression test you have never seen fail is a hypothesis, not a guarantee. If reverting the fix leaves it green, it is testing something else, and the something else is usually a copy of the logic you meant to protect.

Assertions that catch semantic swaps

Structural checks miss this class of bug by construction, so the assertions have to compare identity, not shape.

Capture the expected mapping before the reordering happens, then compare after:

want := map[string]string{}
for _, item := range items {
    want[item.ID] = item.LocalPath      // snapshot before regrouping
}
// ... build manifest ...
for _, e := range entries {
    if e.Path != want[e.ID] {
        t.Errorf("%s bound to wrong file: got %s, want %s", e.ID, e.Path, want[e.ID])
    }
}

Assert uniqueness explicitly. Two entries pointing at one artifact is a swap that a per-entry check cannot see:

seen := map[string]string{}
for _, e := range entries {
    if prev, dup := seen[e.Path]; dup {
        t.Errorf("path %s shared by %s and %s", e.Path, prev, e.ID)
    }
    seen[e.Path] = e.ID
}

Use sort keys that actually reorder. A fixture where items are already sorted proves nothing. Interleave the groups so the sort moves records across group boundaries, which is the condition that triggers the bug.

Check the extension, not just the path. In the original code, an unconverted LocalPath still pointed at a deleted source file. Asserting the suffix catches a whole family of "field not updated after transformation" mistakes.

Where else this pattern hides

The audio pipeline was where I hit it, but the shape is generic. Look for it wherever these three appear together:

• A flatten-process-regroup sequence, common when a batch API is cheaper than per-group calls.
• An in-place sort or filter somewhere in the middle, including one hidden inside a helper you did not write.
• Artifact names derived from a loop counter rather than stored on the record.

Concrete places this shows up: thumbnail generation that batches images then regroups by album; report builders that render pages in one pass then assemble per-chapter; ETL jobs that fetch rows in bulk, sort by timestamp for windowing, then partition by tenant; anything that shells out to a tool taking --output-%d patterns.

The tell is a comment like "index matches the sorted order" or "same loop, same index." That comment is documenting an invariant with no enforcement. It is true when written and silently false after the next refactor, and nothing in your test suite will notice.

Common objections

"Just do not sort in place." Reasonable, but you often do not control the sort. Here it lived in a shared helper used by another pipeline that depends on the ordering. Changing it to return a copy would have been a wider change with its own risk, and it would not have fixed the underlying problem of position-derived names.

"Use a map keyed by ID instead of a slice." That works and eliminates position entirely. It also costs allocation and ordering control, which matters when the next stage streams in sorted order. Storing the path on the record gets the same safety while keeping the slice.

"Validate the output afterward." You can, but validating semantic identity usually means decoding the artifact and comparing content, which is expensive and often impossible in a unit test. Making the binding structurally impossible to break is cheaper than detecting the break.

"Our types are immutable, so we cannot store the path." Then return a parallel structure that pairs record with artifact, and pass that around instead of two slices you re-index independently. The principle is unchanged: one value carries both halves.

What to take away

1. An in-place sort between "name inputs" and "name outputs" turns index-derived filenames into a silent correctness bug.
2. The bug produces valid artifacts with swapped contents, so existence, size, format, and schema checks all pass.
3. Adding an ownership field restores grouping but not binding. Store the artifact reference on the record.
4. Extract the pure functions so tests call shipping code, then revert the fix once to confirm the test fails.
5. Assert identity and uniqueness, not shape, and use fixtures whose sort order actually moves records.

The cheapest version of all this is the revert check. If you write one regression test this quarter, make it one you have watched fail.

FAQ

How do I know if my pipeline has this bug right now? Find every place that formats a filename with a loop index. For each one, ask whether the slice could be reordered between that line and the line that reads the file back. If a sort, filter, dedupe, or parallel scatter sits in between, you have the setup. Then write the mapping assertion above and see if it passes.

Would a static analyzer catch it? No. Every individual line is correct. The bug lives in the relationship between two loops that a type checker has no reason to connect. This is why the mitigation is structural rather than a lint rule.

Is this the same as an off-by-one? No, and that difference matters for debugging. An off-by-one usually crashes at a boundary or produces one obviously wrong element. This produces a full permutation: every element is wrong, none is missing, and the count is correct. If you are chasing a report that "everything is subtly wrong but nothing is broken," permutation is a better hypothesis than arithmetic.

Does this apply outside file pipelines? Yes. Any time you carry a correspondence between two collections by position, a reorder breaks it. Parallel arrays, index-keyed caches, and "the Nth response matches the Nth request" assumptions in batched API clients all have the same shape. The fix is the same too: pair the two halves in one value.