Conversation
hugoh
commented
Jul 1, 2026
hugoh
commented
Owner
- refactor(jj): parse jj log/bookmark output as JSON instead of \x1f-separated text
- fix(jj): don't let a gone remote clobber a diverged bookmark state
- fix(jj): resolve real ahead/behind for untracked remote bookmarks
- fix(jj): surface jj log JSON decode failures instead of swallowing them
- refactor(jj): reuse parseWorkingCopy in fillCommitMsgFromAncestors
- refactor(backend): extract RunTool to dedupe Backend.Run boilerplate
- test(backend): extract shared Backend interface assertions
- test(tui): dedupe near-identical history tests
- test(cmd): extract shared setup builders for cmd/dispatch table tests
…parated text jj's templating language has no whole-object JSON serializer, so build JSON manually via json() per field. This removes the fragile \x1f separator parsing and the colocated-repo enrichWithRemoteBookmark special case, since jj now reports tracked/present/ahead/behind directly per bookmark ref.
applyRemoteRef unconditionally set State=Gone when a tracked remote ref was absent, even when the bookmark itself was conflicted and newBookmarkStatus had already set State=Diverged. That left Conflict=true paired with State=Gone, rendering as a misleading "✗" instead of the diverged "↑N↓N" symbol. Only fall back to Gone when the bookmark isn't already conflicted.
jj only computes tracking_ahead_count/tracking_behind_count for tracked remotes; bookmarkTmpl forces ahead/behind to 0 for an untracked-but- present remote (the common colocated jj/git case). applyRemoteRef copied those zeros in verbatim, so a bookmark genuinely diverged from an untracked remote silently reported as synced — a regression versus the deleted enrichWithRemoteBookmark, which used to compute this via countRevs. Thread a resolveUntracked callback through parseBookmarkRefs/ applyRemoteRef that falls back to countRevs for untracked-but-present remotes, restoring the old behavior. Also fixes TestBackend_Status_ColocatedRemoteFromSingleQuery, which mocked tracked=true and so never actually exercised the untracked path it claimed to cover.
parseWorkingCopy returned a fully zeroed backend.RepoStatus on any json.Unmarshal failure with no error, and Status() propagated that as (status, nil). defaultRunJJ merges stdout and stderr into one buffer, so a stray non-fatal jj warning printed while exiting 0 can corrupt the JSON — Status() would then silently report a dirty/conflicted repo as blank with no ref and no error, indistinguishable from "nothing to report". Return an error from parseWorkingCopy on decode failure and propagate it from Status().
fillCommitMsgFromAncestors hand-rolled the same json.Unmarshal(wcDetail)
decode and "(" + ago + ")" formatting that parseWorkingCopy already
implements for the identical detailTmpl JSON shape. Two copies of that
logic had to be kept in sync; call parseWorkingCopy directly instead.
git.Backend.Run and jj.Backend.Run both hand-rolled the same "reject empty args, run, wrap infrastructure errors" logic around backend.RunCommand. Extract that into backend.RunTool so both backends (and any future one) share it; jj's Run still handles its multi-step dispatch before falling through to the shared path.
git_test.go and jj_test.go each re-implemented the same Backend.Detect/Run/Subcommands contract checks (marker-dir detection, rejecting empty Run args, erroring on a canceled Subcommands context). Move these into internal/backend/backendtest so both backends' test suites call the same assertions instead of duplicating them. backendtest is excluded from the coverage gate: Go only attributes coverage per-package without -coverpkg, so its lines never show as covered even though both backend test suites exercise them fully.
TestHistoryEntries_FilteredByHistoryFilterPrefix was a byte-for-byte copy of TestHistoryEntriesFiltersByHistoryFilterPrefix under a different name — delete the redundant copy. Consolidate TestHistoryPrevFirstCall/TestHistoryPrevClampsAtEnd and TestHistoryNextFromNewest/TestHistoryNextAtMinBound into table-driven tests, since each pair only varied the starting historyIdx and expected outcome.
Several table-driven test entries in cmd_test.go and dispatch_test.go duplicated the same setup closures verbatim (repo-name-collision config, git-repo-plus-missing-repo config, a fixture config with two repos and a group, and single-repo setup+assert pairs). Extract them into named builders (cfgRepoNameTaken, cfgGitRepoPlusMissing, cfgFakeRepoPath, cfgTwoReposOneGroup, checkNameFlagHint, setupSingleRepoWithArgs) so each test entry states only what varies.
|
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Go |  |
Jul 1, 2026 4:49a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
- Coverage 88.52% 87.54% -0.98%
==========================================
Files 33 34 +1
Lines 3668 3637 -31
==========================================
- Hits 3247 3184 -63
- Misses 322 354 +32
Partials 99 99 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.