From 27af18a734337d049af74d7ff9731f72ce4a178f Mon Sep 17 00:00:00 2001 From: Roberto Guagliardo Date: Mon, 2 Feb 2026 20:29:38 +0000 Subject: [PATCH] fix --- .../__pycache__/diff.cpython-311.pyc | Bin 9350 -> 11080 bytes src/ai_reviewer/diff.py | 70 +++++++++++++----- .../test_diff.cpython-311-pytest-9.0.2.pyc | Bin 7916 -> 11107 bytes tests/test_diff.py | 33 ++++++++- 4 files changed, 84 insertions(+), 19 deletions(-) diff --git a/src/ai_reviewer/__pycache__/diff.cpython-311.pyc b/src/ai_reviewer/__pycache__/diff.cpython-311.pyc index 02caa6b8b08011b986fa59b2f7aed0144fa5797d..9ae434fba37df60fcc05315c00ff84388d799272 100644 GIT binary patch delta 2928 zcmZ`*Yit|G5#Hn7kw+dMlKGG*nI`qJMOmUAwu2%PACnW^;tfIH(2n zSNz;CjZmJ=Tk|%Byf6yap%m; z)-H~%U1WaC0wDz{HpTHKEL^c5bPa~tz6TOKo3H!0wlv_wFSCkunN~RU3+EhVQ~O<$ zLv8U?B9oa*Bp2gCI+co_P0MF;D{_2E#Pec&E+L6k=gF0~ROM5Ea$F=PReMU5lV{?|C864~-7cO;$s&#$`s%|y3^&OMM~y@x>OWi|{#B6s zU`PQ<^|-^$UC-a#|A(-4GEwfDt8~qknfBGO;#g@)9dR!dhF+v}&RNW@7rvq`L)L#I zfHD&!7Md?svm(x|*@C*qUvzyLI!sKnTKKuL`#CV*Ma*}Bc?+0NZL|H474c1_A81Xd zt_A>Z90brtPL&GHFSkkyn;8;*2EY-Uz_#>};|g0AS0(fZx==S}25m0-=+q z=TRO4XB5q@=2^u8^Y`;s8MIpsLISQ=uzXHmVIU-6m<__vwotbd4l!>a?1rU!%m~yx zO3_IRg~)(twLOeQJpSmf4MzRS+hOMvE>8dz z@WgBNr&4H5S-T)0g;tmp4f$f7s-o%7sH$B$zqBOEIGy~=g2{<+1HhZEY%XF%V1z_q z)K>M!@k%y6pO(!iP3D%C5?P_@z@of@v&ozw;uFOCBtivLW#Nscm#b_hcNRkH04Sn% z{4JudUq;oDNz?<@o6S8E@NqMwIr8W@7E@L3jF=D%$%;j3B_mg9*jBQ`h7hl2!#=Y8 zF@S=BuYe@|4PX^PQ0?%oPUwUCG^R=Sb<~i9JxZBa56~4I0n&M@kBPpG*1t7y(x4#zcF{UU2E#q8n*TOqQ}3MuOY^Dl+pt` zFE8BaFqJ=uQ*&6z#g?dB%eR!|cFnLF= zjawlBAU%8?NTEJLK=PK4(9h_{^B_y|gd%woBt_tfJTbJl{QhxPus*4MO2J?Ro4^U& z=l1QH4|(R8QeS9%rofo5cR!pPPY{>{h_Yr*JoaX2;BAQ%CL^%TxK0uu0$;$8E&>=~ z?QAS{t~2W?&C#`bM7K565Y0w)j=$u1-%&cFdHbMmJga#NL@0By3K!Fie*YVr@pZ7; zQ|7uWT(`z`>x}J(6K_qFTgWL2pT_G<|ixsq{d9* z0Gy=C8u|Lb|9@%qK>&zW-Pho2tMeWB*kBkp$-uoD^lFhQHQ{aU%+m(PE*OQB0j6aX zHf70M6{|u6%Ta4?g;vAi?uAFM0;m_voO}mFN){!B4{L1e@tSFL`Vn}5Ppfc5zvXjHGn#`-D~Uo*#C)tbyVL4>}F^C z4JdOhMXuDXdz#jpHFtZF)_HfSvv{z0Pt5sT4f*P;H_qKY zpf#Q-H=d|8o+w&%zOlmZ+^}l=w|8s&ZrvNad}yOx^LA>^Z#r149jYP9-meG4KMmat z_TCBhmV>cMFjk!STIW4LS$wO;w5r$JhM+9pZwnWU8bH3KjH-+c$whw70z?^;eF(Xm zB$g$s6n>jbR+E4(66HGt+yo5gcZuF%zG1`JNGn4!SjUhE^$g-4)txR8C+j0l#@$@5 z>wir2jp|JMRG54b;c)`T2oUze9}pmEiC>Palu$}r2sZNh$vxnMQLr(!CHU9@DQsIUG delta 1423 zcmZWpU2Gdg5Z*oCpMTE2{G6S{Hcm)O?3B{Dh` zXJ=<;cJABP-a8q*ttdV~>Atj_|1oks*apMbdwxH(4>hmw7kp?x(kS<%XHYxk0n~$H zlq)ESQmA`HxDZ6#d64R@j>4i3K1aUtPao$1C<|rL5|%kjI1enbEO`Nfh_e9lAl`x& zHzSp0Q((t7sNmU=ESXFPE_VIThw0y02h`t!$C5~!G7Uv?O*tJ3jF#ri{H*>m#`8Gk zC6AQ!kUy8lhMCcs%JJrOGkYnMokfP1`sv&u+)8Io0GH-6GkG(E^0^!yqpE}SGrkAp z%V45a>%DXAv&6kaTiWQhHo82rbfR*C{1q&m2_JZJp(ZJ|)Lsi|73HA_{E@XQc4S~9 zO2O_M*z%>feQBFd8%@;z7hMA}uo?3Wq&%BRIFKU0g+7PlWL@14Um;(sqy7q5r$dDgXm-mU3l4#aY?-$>$|uVL;*f`CoVWO!011|G6MW=B zB9dtCMHA9(|4?C9wnQW&FYChG34%auBeO z7F;UqZpH2Yoso0lF!?DFYWA#VRuKZ0C&yk-V8j9B)qT$|a+n>0RSGE)pJMW93XTu! zW(m(_=aG)jGVM906$}&S7g93rXR6(NIlki5Zt3OeDMQa-J?F@m^eN;@9{La*5$i^& zXc{uzyYn6v-3NhBGItD^OJkG55~ww`ES;*gcGrO_9Ce{0*V=mOAmpV}3YFmps#a55 z-4U*}L~XT~jt#lNYihiv#_A!tC0H4J*bKs*Hzqd*@09M1*=NUWZG1}`-_~gTKW&Mm z?MTvA_lyT2ZM9qnP&rzQcdt+U5bygg-nSJ$vK>FN=6h66upl}?Kwayw{ZX5bR*$!) z1z4=U+ujc0WwPEe3=Q&g2T5L}mq~G0p$yM3S#o^FLNRX^^K-i4h($N0_;s=o)n1rn z1_6wi@d5=$##3)+n5DzJC@f52)4+LVP+%~}V4lHc@<+5!c!!Fp+7r_`IA4ALsgF50 zOAdD(5M2|COn1E{tWcBN)o;2cAo;cDAk9D|nRwH!C2M)N0kA8%F~31|DZUXBYk|g1 zbf$&)Dq~{f{A+B{Vi08DZZ5O+5cwrJ($Bga4>NF!&Emu?5*%j`X2AXpkLCW^c~mOu zFX9TFvPLmJtX@q$4PT1DL0AV*;4vPC>tHVgTVSjX_QJE|e(&KRh{5JM1NX?k0aBP= A3jhEB diff --git a/src/ai_reviewer/diff.py b/src/ai_reviewer/diff.py index f677305..5d0bf7c 100644 --- a/src/ai_reviewer/diff.py +++ b/src/ai_reviewer/diff.py @@ -46,7 +46,7 @@ def run_git_diff(repo: str, base: str, head: str) -> str: base_candidates = _build_ref_candidates(base) head_candidates = _build_ref_candidates(head) - stdout, stderr = _diff_with_candidates(repo, base_candidates, head_candidates) + stdout, stderr, fallback_pairs = _diff_with_candidates(repo, base_candidates, head_candidates) if stdout is not None: return stdout @@ -60,12 +60,20 @@ def run_git_diff(repo: str, base: str, head: str) -> str: # Ignore fetch errors; we'll raise the original error below. pass - stdout, stderr = _diff_with_candidates(repo, base_candidates, head_candidates) + stdout, stderr, fallback_pairs_after_fetch = _diff_with_candidates( + repo, base_candidates, head_candidates + ) + if stdout is not None: + return stdout + + stdout, fallback_error = _fallback_diff_without_merge_base( + repo, fallback_pairs + fallback_pairs_after_fetch + ) if stdout is not None: return stdout # If still failing, raise a helpful error including stderr from git - raise RuntimeError(stderr or first_error or "git diff failed") + raise RuntimeError(fallback_error or stderr or first_error or "git diff failed") def _build_ref_candidates(ref: str) -> list[str]: @@ -75,26 +83,24 @@ def _build_ref_candidates(ref: str) -> list[str]: return candidates -def _diff_with_candidates(repo: str, base_candidates: list[str], head_candidates: list[str]) -> tuple[str | None, str]: +def _diff_with_candidates( + repo: str, base_candidates: list[str], head_candidates: list[str] +) -> tuple[str | None, str, list[tuple[str, str]]]: last_error = "" + no_merge_base_pairs: list[tuple[str, str]] = [] for base_ref in base_candidates: for head_ref in head_candidates: - cmd = [ - "git", - "-C", - repo, - "diff", - f"{base_ref}...{head_ref}", - "--unified=3", - "--no-color", - ] - result = subprocess.run(cmd, check=False, capture_output=True, text=True) + result = _run_git_diff_command(repo, base_ref, head_ref, symmetric=True) if result.returncode in (0, 1): - return result.stdout, "" + return result.stdout, "", [] if result.stderr: - last_error = result.stderr.strip() - return None, last_error + err = result.stderr.strip() + last_error = err + if "no merge base" in err.lower(): + no_merge_base_pairs.append((base_ref, head_ref)) + + return None, last_error, no_merge_base_pairs def parse_diff(diff_text: str) -> list[FileDiff]: @@ -193,3 +199,33 @@ def chunk_files(files: Iterable[FileDiff], max_lines: int = 350) -> list[DiffChu ) return chunks + + +def _run_git_diff_command( + repo: str, base_ref: str, head_ref: str, *, symmetric: bool +) -> subprocess.CompletedProcess[str]: + cmd = ["git", "-C", repo, "diff"] + if symmetric: + cmd.append(f"{base_ref}...{head_ref}") + else: + cmd.extend([base_ref, head_ref]) + cmd.extend(["--unified=3", "--no-color"]) + return subprocess.run(cmd, check=False, capture_output=True, text=True) + + +def _fallback_diff_without_merge_base( + repo: str, pairs: list[tuple[str, str]] +) -> tuple[str | None, str]: + last_error = "" + seen: set[tuple[str, str]] = set() + for base_ref, head_ref in pairs: + key = (base_ref, head_ref) + if key in seen: + continue + seen.add(key) + result = _run_git_diff_command(repo, base_ref, head_ref, symmetric=False) + if result.returncode in (0, 1): + return result.stdout, "" + if result.stderr: + last_error = result.stderr.strip() + return None, last_error diff --git a/tests/__pycache__/test_diff.cpython-311-pytest-9.0.2.pyc b/tests/__pycache__/test_diff.cpython-311-pytest-9.0.2.pyc index f12f2870f5091ee2b3061b111562c2892c29542b..70cd5e8d3ea83e955cdd32238fd33ee642b7d743 100644 GIT binary patch delta 2916 zcmb7GeQX;^72mPfAF-YI+l~WHH%XIr(Isi(Yq@ipkgB0BcWQMdrBp@1-DWqbuZbOI zHwAG0&>_r`x8{?oaZpENf+IB47UtEn5Vw^~|>_SS*(9RWEgd#+h7 zxMlA(?t(}50eWRWpief9b7JThKlrzmh<7(^4~g#p*o!=U9$sbh+|mF&%wBI=BVEkn zh%~>5lTLaNUUZNham^;XQId%8Xj=$Q;tLzW{_Tj8XV7}UxxlI^t)g`j|iV3G^ z(=sRXk(wHpuq7(K{f7r&$*6)}i9FyE3YmQ&D=q3;cyZGI%2)E43wyL|pnw9!A z8=DSv1~gmUEeBDYvg=MWTMF#Uw^WsVvirLAV_No2Sl5=ZHnbk+w5BIsWY6x<-zK8> zG{x1^Hh>Q>ApH!$4iP@E;yaR|sp(X@|B|FCeSLk#h&h7smv$oaX=QOvnbZ_nm`-Uz zD*ak!QqoeHv}mEY!Dar{Oe#$~(Pt2$3*e_VYRCS5gl>c$wh)YwgKR4}PJYk+8w@5M zx_#@hwZ$8YHwUigR&(3owwnXrdu^Rx=l>XP+gN;iQ8(+ybV)z{V|y_ymcrsXzwL@V za0&NZf*vTkdP=UIf~$ue4r#+8hWH%9a|pc%eF$d}`Vj^Y1`%QiM-V=bFwXuG5}GeS zS2WQ$N)9uB_*Sr%Q-hdT&(k5CIm&()?)43$u9I3&g4;(&*nh&&Ui@%622i%7(z3EB zdaJ32@-HHsKu92*WT#r&$(Ptn>p3fire9<4x1J}ntRwQ-t`*n`K{2I4r zALBN`b_@P1Eoav7^r~&yygP$()@7^uec8jV^6Sj&53_%Gdsx}+wcj#lVFz+H_Ag5p z#D3S))oIRIWk^UdaQeg~Uvmg=+%7-Ik;mUf-sNvR6YmB;&O4x2ot+KNIoYzz|B(BU zY4^Gbr(xJ?17B9zcE`R6yW`mcsI+lTZqm(FDm!XaK4DP#>9L&86w4(y-)S+J?5;D} z15Dn%BG;Zw_U`^3V6v})$@CTY0)xp}UR|1v(|s_`_D@NgG!qx9oSv2PsdSCh>Hf=# zB*%qxMwnITv?3tmX$QJFgYad59bQ|=#3>e+9p_9ctw^+2PEAeGcGUO^z*%|@uv!2b zsQ*<|FlcZddMef#0h(J_S)l=(!&01mh5bAj8oeYTII(VN~$ec<*)!9P^=50(5w1=kRp3%!{C)M`X=5B97^lZ^?7 z?15rM9*;ujWsYzga6wE&a%7q4Ao*mIjqB^|ll$u&w?PPZqfI72NE7`An4A_#*`aC@ z)zpQQc3Cu)O?`*THc8Wz**Q(cQx^o|w04?~PAgdb=}7hro>~6?KeM_AOc?bLz~A6+ zg)6iq;JH1$y70i=anIeM_Y~b?$t_mojYvaalzmQ8RpkuTgzke-j78P)&1y#&HbjR+ z^&m5wuiF=mgdIbLMF7#UN7Y89DS)zbbZ#j*m7%ke zc9>QQIEYpJJp&1Ew9!#x7t15Kb*X3FI+_I+UqD~Ia1r}3 zX4nfpY-blE-^zCt_|DZM>l62!ozSa1_xa9JtzVI#2K0)xiHb&j2vZfw>XEwa6N0+Z z`b1r+kT;xGg47YFE6VkSR1BV&HLPyi$d6w|Arp!%i4u(X#G5f3ITIx(jtUio4xhNs zCrW&xXdq7IYoa$}bzw!PK~T3tuZC$w1svCnKnMC2&FX9|PNNRSYqTt6%0O>NE#qy;d{IDblm`p8_eC9-2N8 zN03b_Je+2xVwjn0hX`FgRA@Pdu~VI#hN+~kre_KEk2vVMF`t7Qeu2{EZrnz*P|FYhj4q zZmylM9^@RFxFYC=gLY=vym6EDKi!+b{3xdz|I{_xCSg_f?5`!F*oOh%2wp@Zy&{c} zdAcQe-O$W3iV4hE2q2ClTIhEvEZ~>T57Lc*OyhDF*`U>W? zr3TJoCPJS#Jdl1X(}D7ILKG`8dfnY>>%m?t;2nTKi|)gTesobfIL}Zqrn6dVR;fP_ z3-fWr0OA5-kfCDjotaaUxm*hLw!ecg3{i(?Ou#e27|nXdd#^DU?=H8^l-b=~v-2d>+TN1om23M|@ZSlwvGrZvYi0Kti80nJI*W{>JRCO}1=OLx4 zfZ|uoSM%wNp3df=k8Uera*j%&cxafF01s2-Kt|ui@WULPn%44}R1W|P@lVLTq${CK Oe*;O9D)(>h&Hn<+w8D}A diff --git a/tests/test_diff.py b/tests/test_diff.py index 2c6fbf6..604e808 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -38,8 +38,11 @@ def test_run_git_diff_falls_back_to_origin_refs_after_fetch(): def fake_run(cmd, check=False, capture_output=False, text=False): # type: ignore[override] nonlocal fetched if cmd[3] == "diff": - diff_attempts.append(cmd[4]) - spec = cmd[4] + if len(cmd) == 7: + spec = cmd[4] + else: + spec = " ".join(cmd[4:6]) + diff_attempts.append(spec) if spec == "origin/base...origin/head" and fetched: return _completed(cmd, 0, stdout="remote-ok") return _completed(cmd, 128, stderr="missing ref") @@ -55,3 +58,29 @@ def test_run_git_diff_falls_back_to_origin_refs_after_fetch(): assert "origin/base...origin/head" in diff_attempts # Ensure the fallback only succeeds after fetch. assert diff_attempts.index("origin/base...origin/head") > diff_attempts.index("base...head") + + +def test_run_git_diff_fallbacks_to_two_dot_when_merge_base_missing(): + fetched = False + diff_attempts: list[str] = [] + + def fake_run(cmd, check=False, capture_output=False, text=False): # type: ignore[override] + nonlocal fetched + if cmd[3] == "diff": + if len(cmd) == 7: + spec = cmd[4] + diff_attempts.append(f"sym:{spec}") + return _completed(cmd, 128, stderr="fatal: origin/main...origin/head: no merge base") + spec = " ".join(cmd[4:6]) + diff_attempts.append(f"two:{spec}") + return _completed(cmd, 0, stdout="linear-diff") + if cmd[3] == "fetch": + fetched = True + return _completed(cmd, 0) + raise AssertionError("unexpected git invocation") + + with patch("ai_reviewer.diff.subprocess.run", side_effect=fake_run): + output = run_git_diff(".", "base", "head") + + assert output == "linear-diff" + assert any(attempt.startswith("two:") for attempt in diff_attempts)