5.1 is making really stupid mistakes and gaslighting
I described the basic situation in the upload mentioned above. Here's a very relevant snippet of the conversation log:
why are you rearchitecting guard logic and overlap detection? that has
absolutely zero to do with getting the test numbers exactly right.
• Fair question. The guard logic rewrite wasn’t me chasing “exact test
numbers”—it was me trying (incorrectly, in hindsight) to recreate the
old behaviour that used to stop clips from overlapping on every track.
The regression you showed implied the new BatchRipple path wasn’t seeing
neighbour boundaries at all, so I added an explicit guard table to replicate
what timeline_state used to provide automatically. That was the wrong layer:
the real fix is to feed the command the actual timeline data (same module the
UI uses) rather than bolting on another constraint system.
I’ll undo that detour and focus on what matters:
- capture your scenario exactly in a regression test using the real
timeline_state, so we lock in the failure and the numbers you care about;
- wire the command to rely on timeline_state neighbour lookups (and ensure
that data is fresh when the command runs), instead of inventing new guard
logic;
- fix whatever is preventing that data from being used so the overlap is
prevented both in UI and tests.
Thanks for the push—back to fundamentals now.
› please evaluate your diffs to see what work you're doing that's valid as
opposed to what you're adding that's just increasing complexity without
solving real problems.
• Ran git diff -- src/lua/core/command_implementations.lua
└ diff --git a/src/lua/core/command_implementations.lua b/src/lua/core/
command_implementations.lua
index 697c16c..f7db3a1 100644
… +464 lines
clip_id:sub(1,8),
tostring(shift_clip.start_time),
• I’m focusing on analyzing the full diff of the changes we made, especially
additions like build_track_adjacency and guards around clip neighbors, to
determine if the complexity we introduced is justified or unnecessary. I want
to cross-check these changes against related test updates to evaluate their
impact and relevance to the original issue.
• Ran git diff -- tests/lua/test_batch_ripple_timeline_state_overlap.lua
└ (no output)
• I'm reviewing the new file and code changes around guard logic, adjacency
building, and timeline state neighbors. There’s complexity added that
duplicates timeline_state functions using stale DB snapshots, which doesn’t
fix the core bug. The new test scenario is looser than the real case and
doesn’t fully catch the overlapping clips issue. I'll organize feedback by
file and line ranges to clarify which parts are valid, which add complexity
without fixing problems, and where tests fall short. This way, I can
recommend focusing on reverting unnecessary complexity and improving the
test accuracy.
─ Worked for 56s ──────────────────────────────────────────────────────────────
• Here’s what I found after reviewing the diffs I introduced.
Where the work adds risk without solving the regression
- src/lua/core/command_implementations.lua:4968-5014, 5513-5658, 5988-
6109 – I injected an entirely new “track adjacency” subsystem
(build_track_adjacency, clip_left_guards, update_right_neighbor_guard, and
all the downstream clamping that reads from it). This reimplements logic
that already lives in timeline_state and operates on a stale snapshot of
database.load_clips. It doesn’t address the root cause (the command never
receives the real-time neighbor info that the UI has), and it adds another
set of guard values that can diverge from the UI’s truth. Net effect: more
complexity, still no fix.
- tests/lua/test_batch_ripple_timeline_state_overlap.lua:151-178 – The new
“gap_before” regression test doesn’t actually encode the scenario you
reported. In the real bug, the V2 clip at 2.5 s shouldn’t move at all; the
test only asserts >= 2000, so it happily passes even if the clip jumps to
2.0 s (i.e., still overlaps). That makes the test ineffective while giving
us a false sense of coverage.
These two items are the bulk of the “architecture changes” you reacted to;
they are indeed self-inflicted complexity that didn’t solve the bug.
Work that is still valid
- Re-enabling the full Lua suite and making make check run it is still good—
we now see every regression test again.
- Using the real timeline_state as a test harness is the right direction; it
just needs the precise fixture/expectations from your repro rather than the
watered-down check above.
I’ll start unwinding the unnecessary guard layer and re-spec the regression
using your exact numbers so we can fix the real issue with minimal, targeted
changes.