"That's a quiet clobber, not a fix"
This is the single most instructive comment in your review history, because it's the clearest case of "the workaround was the wrong altitude, the model was the real problem." On #508, in FlightNoteRepository.cs, you'd hand-rolled machinery to survive concurrent edits: load the row, mutate it, catch the unique-index violation, clear the change tracker, retry. Mustafa's comment:
Read what he's actually saying, because it's not "your loop has a bug." It's deeper: the model created the race. A mutable, one-row-per-flight note means two people editing the same flight's note at once collide by construction. Your defensive loop didn't fix that, it masked it, and it masked it in the worst possible way: by silently overwriting one user's work.
There were two legitimate fixes, and it's worth knowing both:
- Optimistic concurrency. Add a version token. When two writes race, the second one to arrive sees the version has moved and fails loudly, surfacing a 409 to the client. The loser of the race knows they lost and can retry with the current state. The conflict is surfaced, not swallowed.
- Append-only. Don't update at all. Every edit is a new row (Lesson 2). With no in-place update, there's no lost-update race to guard. The problem doesn't exist.
You took append-only, and notice what happened: the concurrency machinery disappeared entirely. No catch block, no retry, no ChangeTracker.Clear(). The better model didn't make the workaround cleaner, it made the workaround unnecessary.
This connects to the recurring meta-note Mustafa left across your reviews: "a not-yet-idiomatic foundation is worked around rather than fixed, and the workaround compounds." It's the highest-leverage lesson in the course. Get the value-object / immutability / append-only / layering call right first, and a whole class of defensive code never needs to be written.
Three lessons, one PR, one root decision (note = value object) that rippled through identity, immutability, persistence, and concurrency. That's what it looks like when a reviewer is teaching you to model, not just to patch. The next lessons move to the other reviewers and other concepts: typed errors from Sergii, the anaemic factory from Nick, and layering from Nisha.
Try it yourself
What actually caused the race
The core insight, stated plainly.
You'd written a load-then-mutate-then-catch-unique-violation loop to handle concurrent edits. What did the reviewer say actually caused the race, and what were the two correct fixes?
The meta-signal
You catch yourself writing a catch-and-retry loop with a ChangeTracker.Clear() to guard a race. What is that most likely telling you?
Why "silent" was the real bug
Name the specific harm.
Beyond being inelegant, why was the catch-and-retry loop actually dangerous?