fix: reflog N+1 queries, hard reset false positives, broken test script#4
Open
sulthonzh wants to merge 1 commit into
Open
fix: reflog N+1 queries, hard reset false positives, broken test script#4sulthonzh wants to merge 1 commit into
sulthonzh wants to merge 1 commit into
Conversation
- ReflogParser.getReflog() spawned a git process per entry to fetch
timestamps (N+1 queries — 30+ git calls for limit=30). Now uses
'git reflog --format=%H\ %ct\ %gs' to get timestamps inline in
a single call.
- HardResetDetector falsely detected ALL resets (soft, mixed) as hard
resets. It only excluded literal --soft/--mixed strings in the reflog
message, but 'git reset HEAD~1' (mixed, default) doesn't include
--mixed in the message. Fixed by checking diff-index to verify the
working tree was actually nuked.
- HardResetDetector used getPreviousState(1) which returns HEAD@{1}
relative to current HEAD, not relative to the reset event. Now uses
the reflog entry at index+1 for the correct pre-reset commit.
- Test script referenced 'dist/tests/**/*.test.js' but test files
compile to 'dist/*.test.js'. Fixed glob pattern.
- Updated .gitignore to prevent build artifacts (*.js, *.d.ts, etc.)
from leaking into the src/ and tests/ directories.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Round 4 fixes:
1. Reflog N+1 query problem (performance)
ReflogParser.getReflog()was spawning a separategit log -1process for every reflog entry to get timestamps. With limit=30, that's 30+ git process spawns for a single call. Fixed by usinggit reflog --format='%H %ct %gs'to get timestamps inline in one call.2. Hard reset false positive (correctness)
HardResetDetectordetected ALL resets as hard resets. The logic excluded--softand--mixedliteral strings, butgit reset HEAD~1(the default mixed mode) doesn't write--mixedin the reflog message. Every normalgit resetwas being flagged as a hard reset. Fixed by usinggit diff-index --quiet HEADto check if the working tree was actually nuked.3. Wrong pre-reset commit in hard reset recovery (correctness)
HardResetDetectorusedgetPreviousState(1)which returnsHEAD@{1}relative to current HEAD — not relative to when the reset happened. If there were other operations after the reset, this would point to the wrong commit. Now uses the reflog entry atindex+1for the actual pre-reset state.4. Broken test script
Test command referenced
dist/tests/**/*.test.jsbut test files are atdist/*.test.js. The glob matched nothing, so tests never actually ran vianpm test. Fixed the glob.5. Build artifacts leaking to git
Updated
.gitignoreto prevent.js,.d.ts,.js.map,.d.ts.mapfiles from being tracked insrc/andtests/directories.