From 77b77b662c2957bd23aadf0f5e108adae50067dc Mon Sep 17 00:00:00 2001 From: Ege Sekkin <72572910+nvsekkin@users.noreply.github.com> Date: Thu, 3 Sep 2026 22:02:58 -0400 Subject: [PATCH 1/6] test: Add all-environment perspective golden (#7550) # Description Adds the perspective-camera visual regression requested as a follow-up to [#7053](https://github.com/isaac-sim/IsaacLab/pull/7053) and [NVBUG 6632117]. - Creates four spatially separated Cartpole environments for one PhysX + Kit viewport case. - Frames all four scene partitions in a single perspective-camera capture. - Rejects flat or missing-content captures before comparing against the new golden image. ## Type of change - Test update ## Release backport - [x] Backport this pull request to the active release branch after it merges into `develop` ## Screenshot ![Four scene-partitioned Cartpole environments](https://raw.githubusercontent.com/nvsekkin/IsaacLab/test/scene-partitioning-perspective-golden/source/isaaclab_visualizers/test/golden_images/cartpole_all_envs/physx-kit-viewport.png) ## Validation - `uv run isaaclab -f` - The final candidate was captured twice against the current CI Isaac Sim image (`sha256:50d2491d...`, Isaac Sim 6.1.0-alpha.72); the repeat comparison passed. ## Checklist - [x] I have read and understood the contribution guidelines. - [x] I have run the pre-commit checks. - [x] Documentation changes are not applicable to this test-only update. - [x] My changes generate no new warnings. - [x] I have added a test that covers the requested behavior. - [x] I have added a test-only changelog `.skip` fragment for `isaaclab_visualizers`. - [x] My name already exists in `CONTRIBUTORS.md`. (cherry picked from commit 9be91022519e3f0226bab688261771a27d11cbe0) --- ...scene-partitioning-perspective-golden.skip | 1 + .../cartpole_all_envs/physx-kit-viewport.png | Bin 0 -> 10177 bytes .../test/test_visualizer_golden_physx.py | 13 ++++ .../test/visualizer_golden_utils.py | 17 +++++- .../test/visualizer_integration_utils.py | 57 +++++++++++++++--- 5 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 source/isaaclab_visualizers/changelog.d/test-scene-partitioning-perspective-golden.skip create mode 100644 source/isaaclab_visualizers/test/golden_images/cartpole_all_envs/physx-kit-viewport.png diff --git a/source/isaaclab_visualizers/changelog.d/test-scene-partitioning-perspective-golden.skip b/source/isaaclab_visualizers/changelog.d/test-scene-partitioning-perspective-golden.skip new file mode 100644 index 000000000000..a882c8161446 --- /dev/null +++ b/source/isaaclab_visualizers/changelog.d/test-scene-partitioning-perspective-golden.skip @@ -0,0 +1 @@ +Test-only visual regression coverage; no user-facing changelog entry. diff --git a/source/isaaclab_visualizers/test/golden_images/cartpole_all_envs/physx-kit-viewport.png b/source/isaaclab_visualizers/test/golden_images/cartpole_all_envs/physx-kit-viewport.png new file mode 100644 index 0000000000000000000000000000000000000000..aa2e5f9b1ea41fe165b919afee2f22a2f0a4c5f4 GIT binary patch literal 10177 zcmeHtWm{WK7j7tpQruliixzi^YjAfj?(R-0QnUpE6c13`-JwXKxVyVUa7loZKJWPj z=kv+F@@4kyS@)8exo543QdO40KqW;5000ljize=`RTJMNl4xN&Ov zat<|jPEwkh_+~QEX1VjL9lIYlL~d3e)k7Q#WRIwjN39o1prlX2XI>2n8Bg1YGTb zH&Fbwpx8N7L-!||rN`;sH5Jx==`6;Y0f-YV<98_37UM|*0JdkKoudn-ra0jnx9vDS z##}XL8AV6JfJHOYJUR|FBL@-yCM-d?WO$oLa%%gHUH(D)6Y&Wt81+5Uj8|AxG}s3` ze^nx_urzwC!qm))2X5k*B87OY6xbDa{*5)nbb0%bdyjXgYnQbWb)7{Jip~VJV**%UY6JVLV6d?d$QMm63 zr+5+QBIiB2s{MENTHfvftJ->`Nr=dKq{e?28sol#Ap&*9UXwTQ;>4ev9-f&Fzg|hl zu+8|s*SVyO(*OEPg0q#$ssU_AJ8ik)c!O{QIgSbJJwI#RX&9^-a`URIj6A=ja?nB0$bP^56_z2F zJDd>xGquCGX-?VLIQlZW^hVVBLcaMBG2nStAN+y&0oLjx2#V#)pHW;{~ zi1qdA)v2IvcD$^^YF3rv?N(45%&gHgDG9e8#r|l_9QZGt$9k}KlCP`Cq#N060q9O>_zr{wZ*4;sY4I{UVFY8m# z=H_Z>YMTO12wW;h6cTs;_|8|p4-T1`pQz{0SDm?>Qc5NbAU&$O6AKKqPP~bIs>fp# zuTarqFmu{(dY{|=-1tEl4?f}(LVzlLxL-&8ye|_B($`v&f?*D0N2|J5u#J`CED)Mm8meiYDTp9>^9>t z_-g`GwE7suApt4f`{tj%zRNiBd=6b6*Ak{{A68OO!obyH_JcI#2C<%ZAix{%`OB%= zNxY`aW?0y{>TfRi&^!n-tBy1scv{ zVvV)aL^o~}yA?GYNr|$cPxiZiSENB1Kb5(AOnUm!<~Co}c+XNhriAI2v(K>+EyJyS zf)QcOd;6jyLAPV2Cg+;kWe;OWH^|{gtr6;GwlpWx`Zzdyvc%r|PMbH&D%xl5`TZef zhmxjW#&A(>GQ{eh8Q11enUooX+^#R%&)Gy-D|ZEn+6C|l5BTHN%me3omg`llsm1|p zJFv+i1tRvDStz#k(VOPxtEr0x{JXd?ESNrA8$)b zIE1$xXx?+Drr5<2sx8R}J?(BU9oboq6gAhQ>wiKH~6KKA%Tjb+;MIRqW$JllOM4Nt(+u0w(cu%By+PH23K-u|0J{d-VZe* zmR%^n3pHB;o-YP@z_vV+Q6*U>jOY4$j4z@g*x_Hx%rB<%keE~q|N5ohHAkEqjvAQ< z`_#YXHhLVDRepCJm8Ki5AuiUYCVw&-*4xGu7(g-NP=4v|7OA+T1F1*$Z@%y8wc-2w z=ks$tCLwB3n3KtxzwY8a0>C&$s!#tRgGO4=2PzO#keF=$-Ekqf>XkPmuRu`YJ}&ig zU1@5*x7hw!j&ZYEj{b7R=OL0mz&MUXa3PbXEJGzpZN_PS{3rb$H%=gQ)p%O>>GA20mgt_Q2ixe znn-SbLYlA3RC_5Yh>)B1JpzV%YC*l1muF3Fb&YBX57qdBFY`8sNFW*Ji;hF@3c6Lm z%-rvV>Tw7L-_=Lsdnm)eTdj{mm6$$s1U0n$kMQVg8R2lsEy$=Ag_Pzo#_4c7GN>6{+(R$~H%F;J*1?9L`9x&g+h z1%2AAvzJ>y<}N=bsL{q zm)N#`^Yw2z^Isl`wsjfOR8`x-PP@dh#+ADeYEEOG8Xxg zG)r*Lr^-p=8gbigQfzaN!~1G&iX^*IrYHUJ%95wM7}iX&^cl;k?3|+6xz2laVSp$x zZGwC+rK6F9gCa}$j`SbP92S9~+ofsbU?aL}bt!EfVV03`n}d0~P6pZ%*Ev3Al@>Rf z*t8S9Y00I^W$X7*uI9*yJ`}N2{3c0JPstfp160P#{!jZATdx#v`Y`wvu%ne6$@6Z| z9Z1h;Nt1ZvtDnECQ$Ow1#m^~MmLL)pt}{Hnm~4Em(BwI82*-iL@NG4WP0p1n?J?cJhvDd~8C; zj=FI>&n(WN`pf6Y#w_veNB;Esg~H)lQClLInA%Zw$_9HJc^*M1(ItTyKmY?4K?A<(M5`DMY&~#GK72Xs+>m>3JW< z4ir7vU1~FVMUUO2VC3;KnKOW^CXXke^zigJm@$6~$-v&YQIl^!EkImqV>}sUso2Fu z>^O7}S`FO$@~Gu}i-UquK|uZR$(Cn!VQ;OOfwppPE3VF9--4NVarN}&xl{TWD^wqS zkAqn3DG&6M1l6x!KZuLhVZ0WUB#Oxo3pBANJ7ij!GyS}CKry59?gy`!q7MS*Uf!tt z@=6DPl~MC-PVn|pp)?E%D|}^xqTSz9sg~j%0X^4U*@TUmLbW}jZaI8Ql&<=%vd=_} zJZ!7Xrhno-6BO)J(EJHI?4{gUv03#`7PEu%#f^3{S6R{1Yru-LBx<7JrKSvCa05!)v7~xHmyRPzp9-B&)HbEM0oZfwzrY@{f&q z^nCC=Y9fTVKv_EvN4P_%gF;IEE$#KTp8vRoNM&IEHV%VjidSy9Z>qihFzF||Ufaa* zn>YUMlYAPuJB?##!3-U4k!>z}26>b0@fv0I2fA1xkJ5}ayf}?-IkB#qqHIV;-Zo!P zrv?cJn6EoyML&1O#o)=ZI7*_PDahJ(YUHn>!~?=bt5%bXgku$&o)7!gl; zbGr3sps|XYUmC-Mbin49`W)k!nyIy&6ANF$aOC$@7rLuMs&nX1Q1@Ns`bV~ptc;yH zYL6lNLUOyWbCtN5;*azT%(CT0|l2ecP0` z#rbI%ttLn@YG`CG`P>2!r&O?sLSUm=pSLxcT)+LM@g-n)rfO}fUS%zYh^oePu*~d{>T0P# zhz}6zQzXpfuNXAFJgC0n;jbWrTGo20rnRd9)RWfp-FsSE)j=;sQ@N4TRwUaR_vQ`( zYH8>_Lc>4!0sSJ}G;(Dv9O#-nZUvLgMVy9GuDXnLTJjw`R26mmc!fi{-9f)A%5kYHBfCoWAEGLjrqfCpmAsV}ffhWne z0yetT-|}}CxYtPdosuvvy%A~^WU0{-RICwxgQbp>AxcNX`e{@C@ejhbcQVLak`X{1 z=CdRx0V))!v^tYFXeof1u6+%+4-#~*D9)2#W>Y_OGsTf$phg&byejglc5$JE%;nYT z(FV}Jns^UX=WQ=5t7?kNP52^Z^76bIio^At!1h9>EuLtOYQ?dSYoPB@dO$I|+3abp zP00d+HX%!#zjiy7E8K3h>i9?9)=PVAf4y*ZtW33};p|1ejsd=0s)`k1-pU2G-Q8TT zt4BDOl~;YRLGVBtC@zS4_i3e7sl2SCPIw@pjD%0bBq1fR%g|rDIbGo&N?N&Ys~Ex# zHqcO_K#<$b*>Scg8rYwKT3*l=l;aC7YKbaQ+;k|ieh0N!+Uu|lUxE`0VE9d>3)bRw z&D)`kV#i~?4n8RQx+|a_Kfg~m$DYcG1kcXd%5-PrNY|wcJW%Yo+s-<|*9KpHoAqOe zRFrC~H3AxZpy@{54V&(WV)i6*EtC}wJupW@;S*`Ii-+$cDT+mUfiM|y)B5w^7Imej;C~%?}Nw zhXvUmJsDHs&*d=CJ}ocAlPvd6jC|~T0%iD)`$pgbV2B038oK1Z6|MA+_N+|(0v3D< z+`FT~TJ5+4mlie#rK5%4@a;u}R?#=2+h_5+uISj`?SA$`fuM$wE1~lqryQ_O;@P6q z)hAF1`8>o>RcdwIQqVD!2Tu{zRSn1hX`2GoT|_(QqZ3gTu@H>m;T2Z`4*aDg^&_h$ zikco@BtVt>YPtR-b4rI26W+g*t)Irg#4_jLi)|vum7HF0@{qzfbMBGV zcdQFbvy+;oY&^U?rIn?A-bc@&^gL{xSt#LJL}-%TMZI4Q=$a}^%h8pxUmm$!0N8Fb z-4dDU2gTJKyqr@y{L2MJmVxZ&iZ^Jm7YS40Hvv(CLb55x*Ro4JVX=`quyXu8X*&i<~>SzC4$!k6T*(t~{cO(vt zSc8F<^DWV~{Zvy@Q$mz!MTGvcer#s-gU8-)V=ul*wcy1Vs^IT)jVK_ z0tMLjhg^#ok)Gj?sIHd+oo&RG3SJ#`&&gTh z{Gm2diy+qa$spQ`?KI%7he!uAj_ZdD90Q;@siMSXjJ+$ye~s<0F-0Nwo**VbEQ^5_ zZwBjVL+Jgj_Fv=gt3D{%xxpd}+f_zOz642cA|b$rkBI5wXmE0op3ai8|6hYJ zf{Zohh0^kc(_EAsM^NE^GVlftepo<19IHELKXEgzWS;W_0ONQYzq;JaG;d272Y7F{ ztA8N@^v9-TtNr9>b`i}hDJjXQEc*YcBJh^~QM>lN<3QUTCt>IG;9Eo%z=Rzr(YPiB zs^is`%_qZaZz8}4%mo>UP;cF7Xeh(@ zvD@kaV#}_vK~v+~2ala_mHj>uLnz(=z{;S+zIW3mgfy!i?A{J0v|jLJT_c*u+`3TH z=;>9R)AMafF1|Iq(JKjMQO8_@m5Ny7M~9uJ3g3VJlcNFWU|lG^c;3T1Yb<|G{byHs zxpV%qK&I9;0Y3Nr#4+8eSNDj|Og3zST$`C3Y9glMAlI6#t^h;zVR+nySSQIXj8J#l z3P$W18LaVOsx$xqr$9!M$v=k&cO|}Q!;P%+{nyxVjdJqW;{Q;b8N)75=9;8mf&R&& zSD1VxaqY^<_ItXQB5V2f&pn>FXp7O`Kfs|TWwg*V5d720%N&F$UoTk8%0q9y`M4Mb z2L-4AzO}{se7Rb`Ab9BRob$3WCH40oA_0Av3Wm?C&Aq+Nq)%qfW*O%?k=tz{Vns{HF7zez&0G z!?{uVxoy?Je?f5=v-s&l zCa^bLzd{}tf4=#KM@+9dqnWPm$dK!n?IU}9yY zpe@z2@2dB|5td&6BsZMqVsl?C2+T)XDEJqvt3cRvd)Pm%i$>@0%;nGk^%@^pN}*V5G$fh3ENHD9^Nk=>Q_Pn$JEj5hxKPd)4$9;V;uytjQBp zlQi^kHX!m9z=s#h;II)%jsz;RE}yY``1$vL8p92G1$>?Pd^p-}&!lb!p`ip!IKe3t z=U?~rA*TmDrum>~TKLp(0wvC9&n*)*bR~)09c{YyKi*Vti@%#mI?CvJ9!CJ&6Lqs9 zm(j^so=|1^-yG|jB+Wmlk<%XvTId}am~6{q0=V(}4~Raz!Yn^a%V@EfuhkLV{^DxJ zcdi`8s6mW3V=or+VixcQkX7#T6;3@pA+D^bKM!}Mm8|APUzPGt0^FjOXpOs&{m7)BjCnwZ`x2Ha!!uR z3FdtI+g*gzgmsd({=7S0kP9+>LIse1SM2G(@e*uq-S3FGnh&NNH}+AAQMW4i7Ar^m z0b7=go0{8NmnyuAu|#ux?Pu0Yu~0*)0B6lwJ#f|JV)5qtG4XSq_&i+mOem-y)nA<@{JXPcw&B?u5C40q9yWNX&U`%4uZUuOP zN;Hc>x9)HytwT_gM~f(a+^z1(J);;X`3fM$jy))P+Qs+VO0sT9m&$`|K@#fakk!3pz$eV>LfENYN0! zUCcPgOsnyWl#Sju6bqCv?Jk?P`CDt`9lR3Ybr+u`HLh&xSgJOiSX+9OPAc&&t07=e z1!`HC5-{+3^-E9aeBR`b-L^A0ckjNnCq$x^h$~F7;|{~dpeDt@adWtcpg8Mxh4-a^ z_w{qxwt7xWvK}6m=O*PD;E-o{S|3v{)7&l$R3=%oRmFnvhWe=*XsN+=#4UZJ^tH<7 zieH*jB%^>8+GSb|y95LxYNrolD6N2Sq#FrIlKj}&IZaFB7JY;KJdhptulCx~=JMJ? zV1ZiMy)wMln|pELiDE5M3|dlkBdrwI+pvha`}>E#yM?U%JU~C@HZX8~;^%oR7>ykc z4m@dtGKnoeG)$FDDB2!y*7x9UOG|61CGCBxL<#&f`}%?ZA`8a@0Tu0Npn=PONC6G$ z{TEFuF_e*!L$3%ouJ-p;Nvk#`bvvy{`Rt{_PiNXu>Czg;S5(lzf2c{zYbMN=UOL*$-H*5 zg0#bi)vw81M0U%lKUyLIxvj>BNuPJ2^Weyvv&Z_-WHEWC4E)mm$w2H zk%i;?OhKWI%P%iN_kl)jvP_-ZVt65p)DXG9lWCHHk{<$%zUmXMjG-tS|C;;khXPoN zHm|n_{h13NY#JYLakWZMN(P-v!8b&Cbj~MN<&5J2dP0kebpD0!=ltLTcG}mDrfft? z6qKacI2Ol*&kTIiWxLitDSAZV)s`69Fb74p`JJ%@)TWT7qgwDiql$w4111sxf(}Y- z6fZRBlx+_RFjMY-FG@tR9tFp{WBTFQ6u8oIdcJ33@+sJZc&q>cP+?`s{1a;YJ4)$^ z3f6hsQ;9<^X^Sui7XaS|2N2tP-h(KqIk3*HBO6W1)y*j=0rdpt^^!jWhKZumWcv55 zSUu}{m4x6X*MB5sBO&rKn-0m04X47ogwr7ivWA2WDRUeW9`fTS0fX>9`u+7z(zHt@ zPnN>SQA|{{bo81Zap%Wp`}XHl1T5Hc%LR&q?FRkc3~=7@8FZ^s{dAi|2xpWme~r27 zW6=Jmh7GkbpYYzvocCpA1%G`dRxfXb6=IjIO=91xM{%~M<`m;+jzKK~>7j3~$+JFw z!~h$3Sp(eY^>5$uI!S`+blQ?x4gSX@!dnOD6yxNUMYzu&hiLKi;fE@K{^*j_{Y|ih zvIFUljF#)ak9_DfKI&h_diHS+UL}MmQ|_GZI_{I<(1H%`PWhgE`tdp?5dcDn=GPtg z14?xUCL)j=Sov%Y`sJn|&WvbmwOEmqKgrGUFLY-TtD;z6pYyz6*#2xovtO~GsxH{R zTrsRpCS0&pu=T#7n8AOA(|75q$L_x|zh-|^qC99hExUyW%;>P8NO2b8dK5V5a~M)< zl-g2Tvevu>)l0EWTAJiiu47ijQ@Q$P&|x74yduTCB6(|^BWA5){iaH_znOQ?ymGDo zX-bJ070n|vuEfPK*u+`XdOpO34*Nzv%k2pDKW4c_99WiRHVXR56xlD*ej%A7h4jzn z*>DckGYx^N7-TQsIt9%gl}N%|@vs0kJnkR5+i||g%-syOHEtb5Jk!As4tU+e-9_l! zY;Xd>!~9#R1cDw%D2D;P&2nLQ!dcJVkj||JFXFg)*jN`W5VCgSB=o1S3eE>Z_dxli z4n8)h@ERw%n!MkD9(^DGo3J-at&zqmf(xbR-U|fx_Gp<9?uW1~ak`m>^M`7Qt z(gxlOvQ?JC>7hPCI1+@-EHL)bOZD-!Uo&z8E8E9|8FT&q3ibc_JW7PmS?YH-fzi3K z{*U0(Tv#EX$3ECLIUCm^EjOEmjv}6rT;|z0#+==54J|qvQUDn0mk2IMEHK? zyQ_i&T?{ivW;UU;F*3of5$CG^=I4fV)a&akOw7s6F@#B$*paL;ils^41vWc!#p!RV zx&XE>xOtkgqJB($OcMiZOoi8ntB^T7D#BSv78+oKWCR^4raz3_=%qgk?dqyd+w*(f zwxF8(7aYRhW!P^I??os(MNq3Y-k}w3z=bN4wj;lT;3x!%e)gywI0sB)F49p>Z zRv+;K76ztAM~4JyX8!irPKBkdJ9T_{g510cM^b$urbWa%)el>UY>2IPbyjl`WMMER zjAsc9gnx@H&Ui>^Iaw3e)=?%wlCG=v__ys(^W{f^|8(gdRBwlT-FPbVTN&0h{PARD z_nva*QJ=l%m5naEcmpGo=kq4B1a~F7A7m>rK?CdGDCk$g>(LV%473%9icrtnTga6Q zhWtlg53anM0VEaI{$2%hIKQ3vCZ}4J%b;r7pU<15MyaAdeGX0B z6oH!;z9I8AvPj@lbL660&LqK@yRSv?}2WoAevKVbuZ*Q)fh9Q9o-h*nz8I3FD z--MT?CL|xrnA26Q_H{;tamRx3*~SGN-cX{VTc6va@zP0kB9b-k%>cgx1E>e-ifL>+ZoSV! z%$IOn&7Z=0`pNGj$z?K$3!wl2o{h9BtVTPbyU8EFd}I0;P>deGcEfVWW9F{L6@-fg z)CDY!4XSfLVu-rC=PxLCgsE>kG4{qAeRRl{WWgsC!+VEQ>-*trx#MmSi&Zc>ePc|i;N+OnE3CWn4Gk-RHek{(EkICCr?5E literal 0 HcmV?d00001 diff --git a/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py b/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py index e23eeddb0cec..7dfe5fb254a3 100644 --- a/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py +++ b/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py @@ -9,6 +9,8 @@ viewport and tiled-camera capture modes for the cartpole, shadow hand, and AnymalD environments. Golden images are stored under ``golden_images/{scene}/{physics_backend}-{visualizer_type}-{mode}.png``. +The Kit perspective-camera coverage also includes one four-environment cartpole +view to guard the all-partitions spectator path. On the first run for a new combination the current frame is saved as the golden; the test fails so the file can be reviewed before committing. @@ -89,6 +91,17 @@ def _run_anymal_d_physx( ] +def test_kit_perspective_all_envs_golden() -> None: + """The Kit perspective camera renders all four scene-partitioned environments.""" + _golden.run_visualizer_golden_cartpole( + _PHYSICS_BACKEND, + "kit", + "viewport", + _COMPARISON_SCORES, + all_envs_perspective=True, + ) + + @pytest.mark.parametrize("scene,visualizer_type,mode", SCENE_VISUALIZER_MODE_COMBINATIONS) def test_visualizer_golden(scene: str, visualizer_type: str, mode: str) -> None: """Golden image correctness for PhysX across scenes, visualizer types, and modes.""" diff --git a/source/isaaclab_visualizers/test/visualizer_golden_utils.py b/source/isaaclab_visualizers/test/visualizer_golden_utils.py index e7cc7e70a420..871f0198cbc7 100644 --- a/source/isaaclab_visualizers/test/visualizer_golden_utils.py +++ b/source/isaaclab_visualizers/test/visualizer_golden_utils.py @@ -359,6 +359,7 @@ def run_visualizer_golden_cartpole( comparison_scores: list[dict], *, buffer_steps: int = 0, + all_envs_perspective: bool = False, ) -> None: """Run a golden-image test for one ``(physics_backend, visualizer_type, mode)`` combination. @@ -372,7 +373,11 @@ def run_visualizer_golden_cartpole( comparison_scores: Module-level accumulator forwarded to :func:`validate_visualizer_frame`. buffer_steps: Physics steps to run before capture (default 0 — capture the reset pose so the pole remains at its initial 45° angle and is clearly attached to the cart). + all_envs_perspective: Whether to frame four environments in one Kit perspective-camera image. """ + if all_envs_perspective and (visualizer_type != "kit" or mode != "viewport"): + raise ValueError("The all-environment perspective golden requires Kit viewport capture.") + import torch import visualizer_integration_utils as _viz_utils @@ -396,7 +401,12 @@ def _capture_frame(env, viz_type: str, capture_mode: str, backend: str, actions: _viz_utils._prepare_visualizer_test_process() sim_utils.create_new_stage() tiled = mode == "tiled" - env = _viz_utils._make_cartpole_camera_env(visualizer_type, physics_backend, tiled_camera=tiled) + env = _viz_utils._make_cartpole_camera_env( + visualizer_type, + physics_backend, + tiled_camera=tiled, + all_envs_perspective=all_envs_perspective, + ) _viz_utils._configure_sim_for_visualizer_test(env) actions = torch.zeros((env.num_envs, env.action_space.shape[-1]), device=env.device) # Pin the initial pole angle to a fixed value so the golden image shows a clearly @@ -417,8 +427,11 @@ def _capture_frame(env, viz_type: str, capture_mode: str, backend: str, actions: env.step(action=actions) frame = _capture_frame(env, visualizer_type, mode, physics_backend, actions) + if all_envs_perspective: + _viz_utils._assert_non_flat_frame_array(frame) - validate_visualizer_frame("cartpole", physics_backend, visualizer_type, mode, frame, comparison_scores) + test_name = "cartpole_all_envs" if all_envs_perspective else "cartpole" + validate_visualizer_frame(test_name, physics_backend, visualizer_type, mode, frame, comparison_scores) finally: _viz_utils._cleanup_visualizer_test_process(env) diff --git a/source/isaaclab_visualizers/test/visualizer_integration_utils.py b/source/isaaclab_visualizers/test/visualizer_integration_utils.py index c3dc7cd086a4..929a95e47a92 100644 --- a/source/isaaclab_visualizers/test/visualizer_integration_utils.py +++ b/source/isaaclab_visualizers/test/visualizer_integration_utils.py @@ -69,12 +69,24 @@ _CARTPOLE_TILED_CAMERA_INTEGRATION_NUM_ENVS = 4 """Vectorized env count for generated visualizer tiled-camera integration tests.""" +_CARTPOLE_ALL_ENVS_INTEGRATION_NUM_ENVS = 4 +"""Vectorized env count for the all-environment perspective-camera golden test.""" + +_CARTPOLE_ALL_ENVS_INTEGRATION_SPACING = 8.0 +"""Environment spacing [m] for a clear four-cartpole perspective view.""" + _CARTPOLE_INTEGRATION_VISUALIZER_EYE: tuple[float, float, float] = (2.25, 0.0, 3.5) """Passed to :class:`~isaaclab.visualizers.visualizer_cfg.VisualizerCfg` subclasses (``eye``).""" _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT: tuple[float, float, float] = (0.0, 0.0, 2.25) """Passed to visualizer cfgs (``lookat``); also applied to :class:`~isaaclab.envs.common.ViewerCfg` for the env.""" +_CARTPOLE_ALL_ENVS_VISUALIZER_EYE: tuple[float, float, float] = (9.0, 9.0, 10.0) +"""Perspective-camera eye position that frames all four cartpole environments.""" + +_CARTPOLE_ALL_ENVS_VISUALIZER_LOOKAT: tuple[float, float, float] = (0.0, 0.0, 2.25) +"""Perspective-camera target at the center of the four-environment grid.""" + _CARTPOLE_INTEGRATION_TILED_CAMERA_EYE_OFFSET: tuple[float, float, float] = tuple( eye - lookat for eye, lookat in zip(_CARTPOLE_INTEGRATION_VISUALIZER_EYE, _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT) ) @@ -307,17 +319,26 @@ def _allocate_rerun_test_ports(host: str = "127.0.0.1") -> tuple[int, int]: return web_port, grpc_port -def _cartpole_integration_visualizer_camera_kwargs() -> dict[str, tuple[float, float, float]]: +def _cartpole_integration_visualizer_camera_kwargs( + *, all_envs_perspective: bool = False +) -> dict[str, tuple[float, float, float]]: """Eye/lookat for all :class:`~isaaclab.visualizers.visualizer_cfg.VisualizerCfg` subclasses in these tests.""" + if all_envs_perspective: + return { + "eye": _CARTPOLE_ALL_ENVS_VISUALIZER_EYE, + "lookat": _CARTPOLE_ALL_ENVS_VISUALIZER_LOOKAT, + } return { "eye": _CARTPOLE_INTEGRATION_VISUALIZER_EYE, "lookat": _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT, } -def _get_visualizer_cfg(visualizer_kind: str, *, tiled_camera: bool = False): +def _get_visualizer_cfg(visualizer_kind: str, *, tiled_camera: bool = False, all_envs_perspective: bool = False): """Return (visualizer_cfg, expected_visualizer_cls) for the given visualizer kind.""" - cam = _cartpole_integration_visualizer_camera_kwargs() + if tiled_camera and all_envs_perspective: + raise ValueError("Tiled-camera and all-environment perspective modes are mutually exclusive.") + cam = _cartpole_integration_visualizer_camera_kwargs(all_envs_perspective=all_envs_perspective) tiled_cam = ( { "streaming_view": True, @@ -1815,16 +1836,29 @@ def _make_franka_cloth_env(visualizer_kind: str | tuple[str, ...], *, tiled_came def _make_cartpole_camera_env( - visualizer_kind: str | tuple[str, ...], backend_kind: str, *, tiled_camera: bool = False + visualizer_kind: str | tuple[str, ...], + backend_kind: str, + *, + tiled_camera: bool = False, + all_envs_perspective: bool = False, ) -> CartpoleCameraEnv: """Create cartpole camera env configured with selected visualizer and physics backend.""" + if tiled_camera and all_envs_perspective: + raise ValueError("Tiled-camera and all-environment perspective modes are mutually exclusive.") physics = "newton_mjwarp" if backend_kind == "newton" else "physx" env_cfg = _compose_task_cfg("Isaac-Cartpole-Camera-Direct", physics, "renderer=isaacsim_rtx") env_cfg.scene.num_envs = ( - _CARTPOLE_TILED_CAMERA_INTEGRATION_NUM_ENVS if tiled_camera else _CARTPOLE_INTEGRATION_NUM_ENVS + _CARTPOLE_TILED_CAMERA_INTEGRATION_NUM_ENVS + if tiled_camera + else _CARTPOLE_ALL_ENVS_INTEGRATION_NUM_ENVS + if all_envs_perspective + else _CARTPOLE_INTEGRATION_NUM_ENVS ) - env_cfg.viewer.eye = _CARTPOLE_INTEGRATION_VISUALIZER_EYE - env_cfg.viewer.lookat = _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT + if all_envs_perspective: + env_cfg.scene.env_spacing = _CARTPOLE_ALL_ENVS_INTEGRATION_SPACING + camera_kwargs = _cartpole_integration_visualizer_camera_kwargs(all_envs_perspective=all_envs_perspective) + env_cfg.viewer.eye = camera_kwargs["eye"] + env_cfg.viewer.lookat = camera_kwargs["lookat"] tw, th = _CARTPOLE_TILED_CAMERA_INTEGRATION_WH env_cfg.tiled_camera.width = tw env_cfg.tiled_camera.height = th @@ -1832,7 +1866,14 @@ def _make_cartpole_camera_env( env_cfg.observation_space = [th, tw, env_cfg.observation_space[2]] env_cfg.seed = None visualizer_kinds = (visualizer_kind,) if isinstance(visualizer_kind, str) else tuple(visualizer_kind) - visualizer_cfgs = [_get_visualizer_cfg(kind, tiled_camera=tiled_camera)[0] for kind in visualizer_kinds] + visualizer_cfgs = [ + _get_visualizer_cfg( + kind, + tiled_camera=tiled_camera, + all_envs_perspective=all_envs_perspective, + )[0] + for kind in visualizer_kinds + ] env_cfg.sim.visualizer_cfgs = visualizer_cfgs[0] if len(visualizer_cfgs) == 1 else visualizer_cfgs return CartpoleCameraEnv(env_cfg) From 94e7ba2b2c99f24931c57217a7d803d2c77db362 Mon Sep 17 00:00:00 2001 From: Antoine RICHARD Date: Fri, 4 Sep 2026 19:51:41 +0200 Subject: [PATCH 2/6] Report why fetching a pre-trained checkpoint failed (#7535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Running a play script with `--checkpoint pretrained` on a machine whose `.pretrained_checkpoints` directory was left behind by a container run (owned by `root`) prints: ``` Fetching pre-trained checkpoint : https://.../rsl_rl/Isaac-Humanoid-Direct_newtonmjwarp_none_rsl_rl.pt A pre-trained checkpoint is currently unavailable for this task. ``` and exits. The checkpoint is published and reachable; the download failed with `PermissionError: [Errno 13] Permission denied: '.../.pretrained_checkpoints/rsl_rl/https'`. That error was discarded by a bare `except Exception` in `isaaclab_rl.utils.pretrained_checkpoint.get_published_pretrained_checkpoint`, which reported every failure — a missing checkpoint, an unreachable server, an unwritable cache, a full disk — with the same sentence and returned `None`, after which the play scripts return without further explanation. This separates the two cases: * **The asset server does not provide the checkpoint.** `None` is still returned and the message still starts with `A pre-trained checkpoint is currently unavailable for this task.`, so callers and tests matching on that line are unaffected. It now also names the location that was tried, the task and backends that location was derived from, and what to do instead: ``` A pre-trained checkpoint is currently unavailable for this task. The asset server does not provide 'https://.../Isaac-Not-A-Real-Task_newtonmjwarp_none_rsl_rl.pt'. Either no checkpoint is published for task 'Isaac-Not-A-Real-Task' with the 'newtonmjwarp' physics and 'none' render backends, or the asset server could not be reached. Train the task, or pass --checkpoint to use a checkpoint of your own. ``` * **The checkpoint exists but could not be downloaded.** This is a local problem the user has to fix, so it raises `RuntimeError` naming the checkpoint, the cache directory and the originating error, which is chained as the cause: ``` RuntimeError: Failed to download the pre-trained checkpoint 'https://.../Isaac-Humanoid-Direct_newtonmjwarp_none_rsl_rl.pt' into '/home/user/IsaacLab/.pretrained_checkpoints/rsl_rl': PermissionError: [Errno 13] Permission denied: '/home/user/IsaacLab/.pretrained_checkpoints/rsl_rl/https'. Check that the cache directory is writable and that the disk is not full; a directory left behind by a container run is owned by root. ``` Note that `omni.client` reports a checkpoint that was never published and a server it cannot reach identically, so both remain covered by the "unavailable" message. Both paths were verified end to end against the live asset server, using a published checkpoint with a read-only cache directory and an unpublished checkpoint name. ## Type of change - Bug fix (non-breaking change which fixes an issue) Behaviour note: callers that relied on `None` to mask a local download failure now see a `RuntimeError`. Callers that treat `None` as "no checkpoint published for this task" — every caller in the repository — are unchanged. This is called out in the changelog fragment. ## Release backport - [ ] Backport this pull request to the active release branch after it merges into `develop` ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source//changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit 768c305ccf3e07f47fc3290de4b2667be753f409) --- ...ner-pretrained-checkpoint-errors.minor.rst | 17 ++++++ .../utils/pretrained_checkpoint.py | 41 +++++++++++++-- .../test/test_pretrained_checkpoint.py | 52 +++++++++++++++++++ 3 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 source/isaaclab_rl/changelog.d/antoiner-pretrained-checkpoint-errors.minor.rst diff --git a/source/isaaclab_rl/changelog.d/antoiner-pretrained-checkpoint-errors.minor.rst b/source/isaaclab_rl/changelog.d/antoiner-pretrained-checkpoint-errors.minor.rst new file mode 100644 index 000000000000..536a6351a26a --- /dev/null +++ b/source/isaaclab_rl/changelog.d/antoiner-pretrained-checkpoint-errors.minor.rst @@ -0,0 +1,17 @@ +Fixed +^^^^^ + +* Fixed :func:`~isaaclab_rl.utils.pretrained_checkpoint.get_published_pretrained_checkpoint` reporting + every download failure as ``A pre-trained checkpoint is currently unavailable for this task.``. A + checkpoint the asset server does not provide is still reported that way, but the message now names the + location that was tried, the task and backends it was derived from, and what to do instead. + +Changed +^^^^^^^ + +* Changed :func:`~isaaclab_rl.utils.pretrained_checkpoint.get_published_pretrained_checkpoint` to raise + ``RuntimeError`` when a published checkpoint cannot be downloaded, for instance when the + ``.pretrained_checkpoints`` cache directory is not writable, instead of returning ``None``. The + originating error is chained as the cause. ``None`` is now returned only when the asset server does not + report the checkpoint, which covers both an unpublished checkpoint and a server that could not be + reached; callers that relied on ``None`` to mask local download failures must catch ``RuntimeError``. diff --git a/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py b/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py index 4d2a30ac252e..d64c31ff9f80 100644 --- a/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py +++ b/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py @@ -318,7 +318,16 @@ def get_published_pretrained_checkpoint( from :data:`sys.argv`. Legacy checkpoints do not use preset-qualified names. Returns: - The path. + The path, or None when the asset server does not report a checkpoint for this task + and backend combination. That covers both a checkpoint that was never published and + a server that could not be reached, which ``omni.client`` does not distinguish, so a + transient outage is not evidence that a checkpoint does not exist. The reason is + printed before returning. + + Raises: + RuntimeError: If the checkpoint is published but could not be downloaded, for + instance because the local cache directory is not writable. The originating + error is chained as the cause. """ if preset_names is None and physics_backend is not None and render_backend is not None: preset_names = get_pretrained_checkpoint_preset_names(task_name) @@ -339,9 +348,35 @@ def get_published_pretrained_checkpoint( print(f"Fetching pre-trained checkpoint : {ov_path}") try: resume_path = retrieve_file_path(ov_path, download_dir) - except Exception: - print("A pre-trained checkpoint is currently unavailable for this task.") + except FileNotFoundError: + # the asset server reports a checkpoint that was never published and a server it + # cannot reach the same way, so both are covered by the same message + backends = ( + "" + if physics_backend is None + else f" with the '{physics_backend}' physics and '{render_backend}' render backends" + ) + print( + "A pre-trained checkpoint is currently unavailable for this task.\n" + f" The asset server does not provide '{ov_path}'.\n" + f" Either no checkpoint is published for task '{task_name}'{backends}, or the asset" + " server could not be reached.\n" + " Train the task, or pass --checkpoint to use a checkpoint of your own." + ) return None + except Exception as exc: + # the checkpoint exists on the server, so this is a local failure the user has to fix; + # reporting it as an unavailable checkpoint would send them looking in the wrong place + hint = "" + if isinstance(exc, OSError): + hint = ( + " Check that the cache directory is writable and that the disk is not full;" + " a directory left behind by a container run is owned by root." + ) + raise RuntimeError( + f"Failed to download the pre-trained checkpoint '{ov_path}' into" + f" '{os.path.abspath(download_dir)}': {type(exc).__name__}: {exc}.{hint}" + ) from exc else: print("Using pre-fetched pre-trained checkpoint") return resume_path diff --git a/source/isaaclab_rl/test/test_pretrained_checkpoint.py b/source/isaaclab_rl/test/test_pretrained_checkpoint.py index 91aa9d494cd7..4983795dd8da 100644 --- a/source/isaaclab_rl/test/test_pretrained_checkpoint.py +++ b/source/isaaclab_rl/test/test_pretrained_checkpoint.py @@ -200,3 +200,55 @@ def _retrieve_file_path(remote_path: str, download_dir: str) -> str: "remote_path": f"omniverse://IsaacLab/PretrainedCheckpoints/rl_games/{filename}", "download_dir": expected_download_dir, } + + +def test_get_published_pretrained_checkpoint_reports_unpublished_checkpoint( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +): + """Test that a checkpoint missing from the asset server names the location that was tried.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(pretrained_checkpoint, "ISAACLAB_NUCLEUS_DIR", "omniverse://IsaacLab") + + def _retrieve_file_path(remote_path: str, download_dir: str) -> str: + raise FileNotFoundError(f"Unable to find the file: {remote_path}") + + monkeypatch.setattr(pretrained_checkpoint, "retrieve_file_path", _retrieve_file_path) + + path = pretrained_checkpoint.get_published_pretrained_checkpoint( + "rsl_rl", + "Isaac-Cartpole", + "physx", + "none", + ) + + assert path is None + output = capsys.readouterr().out + assert "A pre-trained checkpoint is currently unavailable for this task." in output + assert "omniverse://IsaacLab/PretrainedCheckpoints/rsl_rl/Isaac-Cartpole_physx_none_rsl_rl.pt" in output + assert "'physx' physics and 'none' render backends" in output + + +def test_get_published_pretrained_checkpoint_raises_on_unwritable_cache( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +): + """Test that a local download failure is reported instead of being reported as unavailable.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(pretrained_checkpoint, "ISAACLAB_NUCLEUS_DIR", "omniverse://IsaacLab") + cause = PermissionError(13, "Permission denied", str(tmp_path / ".pretrained_checkpoints")) + + def _retrieve_file_path(remote_path: str, download_dir: str) -> str: + raise cause + + monkeypatch.setattr(pretrained_checkpoint, "retrieve_file_path", _retrieve_file_path) + + with pytest.raises(RuntimeError) as error: + pretrained_checkpoint.get_published_pretrained_checkpoint("rsl_rl", "Isaac-Cartpole", "physx", "none") + + message = str(error.value) + assert "Isaac-Cartpole_physx_none_rsl_rl.pt" in message + assert str(tmp_path / ".pretrained_checkpoints" / "rsl_rl") in message + assert "Permission denied" in message + assert error.value.__cause__ is cause From cba808b47712120247e45b0d9d2a7d229067986c Mon Sep 17 00:00:00 2001 From: Kelly Guo Date: Fri, 4 Sep 2026 11:15:18 -0700 Subject: [PATCH 3/6] Fix OVRTX clone-plan test import (#7582) # Description Restores the missing `torch` import in `test_ovrtx_clone_plan.py`. #7462 converted the existing clone-plan test inputs from Torch tensors to NumPy arrays and removed the then-unused import. #6308 subsequently added a new object-scale test using `torch.ones` and `torch.arange` without restoring the import, causing the repository-wide Ruff pre-commit check to fail with `F821 Undefined name torch`. This PR contains only the import repair and an `isaaclab_ov` `.skip` changelog fragment. It unblocks #7579 and other changes based on the current `develop` branch. ## Type of change - Bug fix (non-breaking test fix) ## Release backport - [ ] Backport this pull request to the active release branch after it merges into `develop` No release backport is required: `release/3.0.0` already imports `torch` in this test. ## Screenshots Not applicable. ## Testing - `uv run --no-project --with pre-commit python -m pre_commit run --all-files` - `uv run --no-project python tools/changelog/cli.py check --include-worktree` - `git diff --check upstream/develop...HEAD` ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the full pre-commit checks - [x] Documentation changes are not required because no public API changed - [x] My changes generate no new warnings - [x] The fix directly covers the Ruff `F821` failure - [x] I have added an `isaaclab_ov` changelog fragment - [x] My name already exists in `CONTRIBUTORS.md` (cherry picked from commit 7bf6959dc1f1759b6f8578a8b032c409387f75a3) --- .../isaaclab_ov/changelog.d/fix-ovrtx-clone-plan-test-import.skip | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 source/isaaclab_ov/changelog.d/fix-ovrtx-clone-plan-test-import.skip diff --git a/source/isaaclab_ov/changelog.d/fix-ovrtx-clone-plan-test-import.skip b/source/isaaclab_ov/changelog.d/fix-ovrtx-clone-plan-test-import.skip new file mode 100644 index 000000000000..e69de29bb2d1 From 54a2e7bf18f66cea7ada18f0f577f4a66a428656 Mon Sep 17 00:00:00 2001 From: Antoine RICHARD Date: Sat, 5 Sep 2026 08:28:23 +0200 Subject: [PATCH 4/6] Add issue audit and backlog triage skills (#7359) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Adds two developer skills for issue work, split by scope: - **`isaaclab-auditing-an-issue`** — audits a single issue against current code and drafts the reply. Holds the reusable per-issue checks. - **`isaaclab-triaging-issue-backlog`** — batch sweeps. Requires the first as a sub-skill and adds only the orchestration: adversarial verification, approval bookkeeping, and the human review gate. The guidance is distilled from a full pass over the open backlog, where automated triage repeatedly produced comments that read well but asserted things that were not true. The skills encode the checks that caught those. ### Per-issue skill - **Mandatory checks table**, one row per claim shape. Grep `contrib/` before writing "Isaac Lab doesn't have X", since features move to contrib and absence claims go stale. Check the specific backend package rather than assuming parity. Run a script for behavioral claims instead of reading the raise site — a guard can be narrower than it looks. - **URL traps.** `blob/main/...` 404s for `isaaclab_newton`, `isaaclab_ov`, `isaaclab_ovphysx` and `isaaclab_contrib`, since those packages are not on `main`. `isaac-sim.github.io/IsaacLab/main/...` is the stale 2.x docs build. Both produced dead links during the triage pass. - **Do Not Close list.** Cases where the stated reason is defensible but closing still loses something: an open maintainer question, pre-merge feedback on an open PR, a duplicate of a bug that was itself closed without a fix, an unmet titled ask, and upstream-blocked work where nobody has filed upstream. - **Comment style.** Thank the reporter, cite a verified fix, invite a reopen, and do not assign fault to the reporter or to a sibling team. ### Batch skill - Adversarial verification that defaults to REFUTED or UNCERTAIN when evidence cannot be independently confirmed. - Approval bookkeeping: never infer approval, keep one plan file, re-show anything edited after approval, and report refuted and held counts alongside the reduction figure. - A failure-mode table observed in practice, so the next sweep expects them rather than rediscovering them. ## Type of change - New feature (non-breaking change which adds functionality) ## Checklist - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there ## Notes `uv run --no-project python tools/skills/cli.py check` passes (23 skills validated). No changelog fragment: no source package changed, and the fragment check passes. The skills have not yet been pressure-tested against agents with and without them, which is the usual bar for discipline-enforcing guidance. Happy to do that before merge if reviewers would prefer. --------- Co-authored-by: Kelly Guo (cherry picked from commit 3639364a493050313689fe89336952fdbb57ac61) --- .agents/skills/isaaclab-auditing-an-issue | 1 + .../skills/isaaclab-triaging-issue-backlog | 1 + skills/developer/issue-audit/SKILL.md | 142 ++++++++++++++++++ .../developer/issue-backlog-triage/SKILL.md | 88 +++++++++++ 4 files changed, 232 insertions(+) create mode 120000 .agents/skills/isaaclab-auditing-an-issue create mode 120000 .agents/skills/isaaclab-triaging-issue-backlog create mode 100644 skills/developer/issue-audit/SKILL.md create mode 100644 skills/developer/issue-backlog-triage/SKILL.md diff --git a/.agents/skills/isaaclab-auditing-an-issue b/.agents/skills/isaaclab-auditing-an-issue new file mode 120000 index 000000000000..0c26b7a577cc --- /dev/null +++ b/.agents/skills/isaaclab-auditing-an-issue @@ -0,0 +1 @@ +../../skills/developer/issue-audit \ No newline at end of file diff --git a/.agents/skills/isaaclab-triaging-issue-backlog b/.agents/skills/isaaclab-triaging-issue-backlog new file mode 120000 index 000000000000..97114ccf6e82 --- /dev/null +++ b/.agents/skills/isaaclab-triaging-issue-backlog @@ -0,0 +1 @@ +../../skills/developer/issue-backlog-triage \ No newline at end of file diff --git a/skills/developer/issue-audit/SKILL.md b/skills/developer/issue-audit/SKILL.md new file mode 100644 index 000000000000..c3d4b5247bfd --- /dev/null +++ b/skills/developer/issue-audit/SKILL.md @@ -0,0 +1,142 @@ +--- +name: isaaclab-auditing-an-issue +description: Audits one open GitHub issue against current code to decide close or keep-open, and drafts the closing comment. Use when deciding whether a specific issue is still valid, whether a reported bug was fixed, or what to reply before closing. +audience: developer +status: experimental +owners: + - isaaclab-maintainers +--- + +# Auditing an Issue + +## When To Use + +Use this skill for a single issue: deciding whether it is still valid against current code, and drafting the reply if it is being closed. + +For auditing many issues at once, use `isaaclab-triaging-issue-backlog`, which applies this skill per issue and adds the batch controls. + +Do not close an issue purely because it is old. Age is not evidence. + +## Core Principle + +A comment that reads well is not evidence that its claims are true. Verify every factual claim against current code before posting, and ask what is lost by closing, not only whether closing is defensible. + +## Workflow + +1. Read the whole thread, not just the first post: `gh issue view NUMBER --repo isaac-sim/IsaacLab --comments`. +2. Identify each checkable claim you intend to make in the reply. +3. Verify each one using the table below. Prefer running code over reading it. +4. Check the Do Not Close list before deciding. +5. Draft the comment in the style below, then confirm every link resolves. + +## Mandatory Checks + +| Claim shape | Required check | +|---|---| +| "X is fixed / landed in #N" | `gh pr view N` returns MERGED, and its diff touches the reported behavior. A generic "Merges changes from main" PR is never an acceptable citation. | +| "Isaac Lab doesn't have X" | Grep `source/isaaclab_tasks/isaaclab_tasks/contrib/`, `source/isaaclab_contrib/` and the `*_experimental` packages. Features move to contrib and absence claims go stale. | +| "`SYMBOL` no longer exists" | `grep -rn "SYMBOL" source/` — any live hit refutes it. | +| "`BACKEND` supports X" | Check that backend's own package. Backends differ; several classes raise `NotImplementedError` on one and not another. | +| Behavioral claim about an error or a limit | Write a temporary script and run it with `uv run python`. Reading the raise site is not enough — the guard may be narrower than it looks. | +| A usage answer | Confirm it is still the best current advice, not merely that some answer was given. Check the relevant capability on the Isaac Sim side too. | +| Any URL | `curl -s -o /dev/null -w "%{http_code}" -L "URL"` returns 200. | + +## URL Traps + +- `github.com/isaac-sim/IsaacLab/blob/main/...` **404s** for anything under `source/isaaclab_newton/`, `source/isaaclab_ov/` or `source/isaaclab_contrib/` — those packages are not on `main`. Use `blob/develop/...`, or cite the path in backticks with no link. Package layout shifts over time (e.g. `isaaclab_ovphysx` was merged into `isaaclab_ov`), so verify the path exists on the ref you link. +- `isaac-sim.github.io/IsaacLab/main/...` is the stale 2.x docs build. Use `release/3.0.0/...`, and confirm the anchor still exists — several 2.x anchors were removed in the 3.x reorganisation. + +## Do Not Close + +Hold the issue when any of these apply, even if the stated close reason is technically defensible: + +- A maintainer asked the reporter a question and the thread is still recent. +- It is pre-merge feedback on an open PR. +- It is a duplicate of an issue that was itself closed without a fix. +- The titled ask is unmet, even if a related bug in the thread was fixed. +- It is blocked on upstream work that is planned but not landed. Confirm the upstream issue exists; "file it upstream" is not a resolution if nobody has. + +When holding, record the concrete next step and the evidence, so the next audit does not redo the work. + +## Comment Style + +- Thank the reporter by handle in the first sentence, specifically and briefly. +- State what actually resolves it, with file paths in backticks and a verified PR link. +- Invite a reopen if it still reproduces on a current release. +- Do not assign fault. Avoid "we never heard back" (blames the reporter) and "nothing on our side" (blames a sibling team). Describe where the behavior lives, and route rather than deflect. +- Do not name a colleague as the source of a negative finding. Crediting a helpful answer is fine. +- If the reply was AI-assisted, end with the disclaimer footer below, and do not claim a level of human review that did not happen. + +### Disclaimer Footer + +End an AI-assisted reply with a horizontal rule and a one-line footer: + +```markdown +--- +*🤖 This issue was reviewed as part of an AI-assisted triage of the Isaac Lab backlog, with automated verification against the current codebase. If we got this wrong, please reopen — we'd rather hear about it!* +``` + +Match the claim to what actually happened. "with automated verification against the current codebase" is accurate for a machine-checked reply. Only say "verified by a maintainer" if a maintainer read that specific comment. + +### Worked Example + +A close for a bug that was genuinely fixed: + +```markdown +Thanks @Mitchell-Torok for catching that `reset_joints_by_offset` ignored the `joint_names` you passed in `SceneEntityCfg`. + +It's fixed: both `reset_joints_by_offset` and `reset_joints_by_scale` in `isaaclab/envs/mdp/events.py` now index the default joint state and the soft-limit clamp with `asset_cfg.joint_ids` (broadcasting `env_ids[:, None]` for subsets), so only the named joints are touched. Shipped in https://github.com/isaac-sim/IsaacLab/pull/2899. Sorry for the delay — please reopen if extra joints still move on a current release. + +--- +*🤖 This issue was reviewed as part of an AI-assisted triage of the Isaac Lab backlog, with automated verification against the current codebase. If we got this wrong, please reopen — we'd rather hear about it!* +``` + +Each part is doing work: the handle and the specific symptom in sentence one, the mechanism plus file path so the comment stands alone in search results, a PR link confirmed MERGED, a three-word apology rather than a paragraph, and a reopen invitation tied to a current release. + +### Fault Assignment + +The rewrites below are the difference between a reply that reads as closure and one that reads as a brush-off. + +| Instead of | Write | +|---|---| +| "We asked twice and never heard back." | "We weren't able to get to the bottom of this at the time, and a lot has changed since." | +| "There's nothing on our side to fix." | "That code path lives in ``, so `` is the better home for it." | +| "This is an Isaac Sim bug that their team owns." | "The `sim.reset()` requirement lives in that layer and is tracked there." | +| "As @SomeDev confirmed, it's broken." | State the finding directly, without attributing it to a colleague. | + +## Common Mistakes + +- Trusting a polished draft. Fluency correlates with neither accuracy nor completeness. +- Answering "was a reply given?" instead of "is that answer still correct and complete?" +- Citing a PR whose title matches the topic without reading its diff. +- Closing an issue whose stated reason is sound but which is the only remaining record of a live problem. + +## Validation + +Verify citations and links before posting: + +```bash +gh pr view PR_NUMBER --repo isaac-sim/IsaacLab --json state,mergedAt,title +curl -s -o /dev/null -w "%{http_code}" -L "URL" +``` + +For a behavioral claim, write a temporary script and run it: + +```bash +uv run python PATH_TO_SCRIPT +``` + +If skills changed, run: + +```bash +uv run --no-project python tools/skills/cli.py check +``` + +## Maintenance + +Keep the URL traps section synchronized with the current package layout. When a package moves between `main` and `develop`, or the docs build version changes, update it before it produces a dead link in a public comment. + +## References + +- [Backlog triage skill](../issue-backlog-triage/SKILL.md) +- [Contributing guide](../../../docs/source/refs/contributing.rst) diff --git a/skills/developer/issue-backlog-triage/SKILL.md b/skills/developer/issue-backlog-triage/SKILL.md new file mode 100644 index 000000000000..a89965a9eec9 --- /dev/null +++ b/skills/developer/issue-backlog-triage/SKILL.md @@ -0,0 +1,88 @@ +--- +name: isaaclab-triaging-issue-backlog +description: Triages many open GitHub issues at once into close/keep-open with drafted closing comments. Use when auditing the whole issue backlog, running a bulk close pass, or reporting how many issues can be retired. +audience: developer +status: experimental +owners: + - isaaclab-maintainers +--- + +# Triaging the Issue Backlog + +## When To Use + +Use this skill when auditing many issues at once: a backlog sweep, a bulk close pass, or a report on how much of the backlog can be retired. + +For a single issue, use `isaaclab-auditing-an-issue` directly. + +**REQUIRED SUB-SKILL:** Use `isaaclab-auditing-an-issue` for every individual issue. This skill only adds batch orchestration; it does not restate the per-issue checks. + +## Core Principle + +Scale multiplies confident errors. A batch of drafted comments will read uniformly well whether or not the claims are true, so verification must be adversarial and the human review gate is not optional. + +## Workflow + +1. Dump the backlog with metadata and split it into batches: + + ```bash + gh issue list --repo isaac-sim/IsaacLab --state open --limit 500 \ + --json number,title,author,createdAt,updatedAt,labels,comments,assignees,url + ``` + +2. Audit each issue per `isaaclab-auditing-an-issue`, recording a verdict, the evidence, and a confidence level. +3. Run an adversarial pass over every close recommendation. Instruct the reviewer to **refute**, and to return REFUTED or UNCERTAIN whenever it cannot independently confirm the evidence. Verify low-confidence recommendations as strictly as the rest — they refute at a far higher rate. +4. Sweep the drafted comments for claims that a per-issue audit misses across a batch: capability assertions, citations whose PR does not match, and dead links. +5. Review every comment with a maintainer, in batches, before anything is posted. +6. Post only what a maintainer explicitly approved. + +## Batch Controls + +- **Never infer approval.** Approve on an explicit statement, not on a reply that merely discusses a batch. Record approvals so a resumed session cannot post something unreviewed. +- **Keep one plan file** listing every issue to be closed with its final comment, plus appendices for held issues and for issues pulled out during review with their next steps. +- **Re-show anything edited after approval.** Later passes rewrite earlier comments; approval does not survive a rewrite. +- **Report what was excluded.** State how many issues were refuted, held, or pulled out, not only how many are closeable. + +## Expect These Failure Modes + +Measured over one full pass of the backlog; treat them as the default, not the exception. + +| Failure mode | What it looks like | +|---|---| +| Stale absence claims | "Isaac Lab doesn't support X" when X moved to `contrib/`. | +| Mismatched citations | A merged PR whose title fits the topic but whose diff never touches the behavior. | +| Capability drift | Asserting a backend or release supports something it does not. | +| Answered-but-wrong | A usage answer that was correct when written and is now outdated. | +| Blame framing | "We never heard back" or "nothing on our side", which read badly in public. | +| Defensible but lossy | A close whose reason is sound but which discards the only record of a live problem. | + +## Common Mistakes + +- Treating a high-confidence label as a substitute for verification. +- Batching approval across issues that were edited at different times. +- Reporting a reduction figure without the refuted and held counts. +- Posting before a maintainer has read the specific comment being posted. + +## Validation + +Before posting, confirm the batch is internally consistent: every comment ends with the intended marker, every cited PR is merged, and every URL resolves. + +```bash +gh pr view PR_NUMBER --repo isaac-sim/IsaacLab --json state,mergedAt,title +curl -s -o /dev/null -w "%{http_code}" -L "URL" +``` + +If skills changed, run: + +```bash +uv run --no-project python tools/skills/cli.py check +``` + +## Maintenance + +Keep the per-issue checks in `isaaclab-auditing-an-issue` and do not duplicate them here. Update the failure-mode table when a new class of error survives verification into a posted comment. + +## References + +- [Issue audit skill](../issue-audit/SKILL.md) +- [Contributing guide](../../../docs/source/refs/contributing.rst) From 21b813d59d144fdfc0ef5068328db35bad9f0c15 Mon Sep 17 00:00:00 2001 From: Kelly Guo Date: Thu, 17 Sep 2026 07:21:50 -0700 Subject: [PATCH 5/6] Update nightly image pull request automation (#7867) # Description Update the nightly Isaac Sim image job so its generated pull request body follows the repository PR template. Automatically comment `run-ci` when the workflow opens a PR or pushes a new update commit, while avoiding duplicate triggers when an existing update branch is unchanged. This addresses the manual follow-up needed on #7773. No new dependencies are required. Validation: - Parsed the workflow as YAML. - Validated the workflow with `actionlint` (excluding its pre-existing `client-id`/`app-id` metadata mismatch for `actions/create-github-app-token@v3`). - Checked the embedded shell with `bash -n`. - Mocked new, changed, and unchanged PR cases and verified `run-ci` is posted exactly when needed. - `uv run isaaclab -f` could not run because the repository lockfile does not support macOS ARM64. ## Type of change - Infrastructure update ## Release backport - [ ] Backport this pull request to the active release branch after it merges into `develop` ## Screenshots Not applicable. ## Checklist Docker and GPU tests run on demand. Push the commits you want tested, then comment `run-ci` on the pull request. - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation (not applicable; workflow-only change) - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works (mocked workflow cases) - [x] I have added a changelog fragment under `source//changelog.d/` for every touched package (not applicable; no source package changed) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit 24f61d963cfb81e65c448e84bde53b163da47812) --- .github/workflows/nightly-isaacsim-image.yml | 36 ++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/workflows/nightly-isaacsim-image.yml b/.github/workflows/nightly-isaacsim-image.yml index 42f59a3cc5de..5a3d97546cdc 100644 --- a/.github/workflows/nightly-isaacsim-image.yml +++ b/.github/workflows/nightly-isaacsim-image.yml @@ -179,6 +179,7 @@ jobs: CURRENT_PIN: ${{ steps.pin.outputs.current }} CANDIDATE_PIN: ${{ steps.pin.outputs.candidate }} DIGEST: ${{ steps.pin.outputs.digest }} + BRANCH_CHANGED: ${{ steps.pin.outputs.branch_changed }} run: | set -euo pipefail @@ -187,6 +188,8 @@ jobs: title="[CI] Bump Isaac Sim image to $short_digest" body_file="$RUNNER_TEMP/isaacsim-image-update.md" { + echo "# Description" + echo echo "This automated draft updates CI to the current Isaac Sim nightly image." echo echo "| Field | Value |" @@ -199,6 +202,30 @@ jobs: echo "Source: https://registry.ngc.nvidia.com/orgs/0947644777160149/teams/internal/containers/isaac-sim/tags" echo echo "New PRs are opened as drafts so maintainers can merge after the CI results are acceptable." + echo + echo "## Type of change" + echo + echo "- Infrastructure update" + echo + echo "## Release backport" + echo + echo '- [ ] Backport this pull request to the active release branch after it merges into `develop`' + echo + echo "## Screenshots" + echo + echo "Not applicable." + echo + echo "## Checklist" + echo + echo "Docker and GPU tests run on demand. This workflow comments \`run-ci\` after opening the pull request or updating its commit." + echo + echo '- [ ] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)' + echo '- [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format`' + echo '- [ ] I have made corresponding changes to the documentation' + echo '- [ ] My changes generate no new warnings' + echo '- [ ] I have added tests that prove my fix is effective or that my feature works' + echo '- [ ] I have added a changelog fragment under `source//changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that)' + echo '- [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there' } > "$body_file" repository_owner=${REPOSITORY%%/*} @@ -208,6 +235,7 @@ jobs: -f head="$repository_owner:$UPDATE_BRANCH" \ --jq '.[0].number // empty') + pr_created=false if [ -n "$pr_number" ]; then pr_url=$(gh api --method PATCH "repos/$REPOSITORY/pulls/$pr_number" \ -f title="$title" \ @@ -223,10 +251,18 @@ jobs: -F body=@"$body_file" \ -F draft=true \ --jq '.html_url') + pr_number=${pr_url##*/} + pr_created=true echo "Opened draft PR: $pr_url" echo "Draft PR: $pr_url" >> "$GITHUB_STEP_SUMMARY" fi + if [ "$BRANCH_CHANGED" = "true" ] || [ "$pr_created" = "true" ]; then + gh api --method POST "repos/$REPOSITORY/issues/$pr_number/comments" \ + -f body='run-ci' >/dev/null + echo "Triggered CI on PR #$pr_number." + fi + - name: Report no-op or dry run if: ${{ steps.pin.outputs.changed != 'true' || inputs.dry_run }} env: From a32b6e8e2c9644f19d708b1705067116393e45aa Mon Sep 17 00:00:00 2001 From: ooctipus Date: Thu, 17 Sep 2026 08:04:52 -0700 Subject: [PATCH 6/6] Let standalone demos complete the health check after startup (#7869) A standalone demo that becomes ready at 299 seconds is currently terminated at the 300-second startup deadline, before its five-second soak can finish. This caused the arms smoke test in #7608 and #7674 to fail after printing its setup and reset messages. Stop applying the startup deadline once the readiness marker has been observed, while preserving the existing 300-second startup and five-second soak settings. The unconditional deadline was introduced by [#6704](https://github.com/isaac-sim/IsaacLab/pull/6704), commit [16083cc85d3](https://github.com/isaac-sim/IsaacLab/commit/16083cc85d318e234dad1825af2554c0dae06db1). This fix does not address the separate arms failures in #7838 and #7866 that never reached readiness within 300 seconds. Validation: the deterministic regression fails on develop and passes with this change; all 11 supervisor checks pass; `uv run isaaclab -f` passes. (cherry picked from commit 0921d32621dc849da9e9458f03a7f5940255043f) --- .../fix-standalone-soak-deadline.skip | 1 + .../test/app/standalone_script_cases.py | 2 +- .../test/app/test_standalone_scripts.py | 32 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 source/isaaclab/changelog.d/fix-standalone-soak-deadline.skip diff --git a/source/isaaclab/changelog.d/fix-standalone-soak-deadline.skip b/source/isaaclab/changelog.d/fix-standalone-soak-deadline.skip new file mode 100644 index 000000000000..f08e4d9ff050 --- /dev/null +++ b/source/isaaclab/changelog.d/fix-standalone-soak-deadline.skip @@ -0,0 +1 @@ +Allowed standalone smoke tests that reached readiness before the startup deadline to complete their full soak interval. diff --git a/source/isaaclab/test/app/standalone_script_cases.py b/source/isaaclab/test/app/standalone_script_cases.py index d9a5e89351b7..f220f033ab16 100644 --- a/source/isaaclab/test/app/standalone_script_cases.py +++ b/source/isaaclab/test/app/standalone_script_cases.py @@ -489,7 +489,7 @@ def read_available_output(timeout: float) -> bool: _terminate_process_group(process) returncode = process.poll() break - if now - start_time >= startup_timeout: + if ready_at is None and now - start_time >= startup_timeout: _terminate_process_group(process) returncode = process.poll() break diff --git a/source/isaaclab/test/app/test_standalone_scripts.py b/source/isaaclab/test/app/test_standalone_scripts.py index 3d0b8c6480cf..a43cb2349b27 100644 --- a/source/isaaclab/test/app/test_standalone_scripts.py +++ b/source/isaaclab/test/app/test_standalone_scripts.py @@ -22,6 +22,7 @@ import sys from dataclasses import replace from pathlib import Path +from unittest import mock import pytest import standalone_script_cases as script_cases @@ -371,6 +372,37 @@ def test_subprocess_supervisor_soaks_then_stops_process_group(): assert result.elapsed < 2.0 +def test_subprocess_supervisor_completes_soak_after_startup_deadline(monkeypatch): + """Readiness just before the startup deadline must still receive the full soak.""" + process = mock.Mock(returncode=None) + process.poll.side_effect = lambda: process.returncode + process.communicate.return_value = (b"", None) + selector = mock.Mock() + now = 0.0 + poll_times = iter((299.0, 300.0, 304.0)) + + def select(timeout): + nonlocal now + if timeout == 0.0: + return [] + now = next(poll_times) + if now == 299.0: + return [(mock.Mock(fileobj=process.stdout), script_cases.selectors.EVENT_READ)] + return [] + + selector.select.side_effect = select + monkeypatch.setattr(script_cases.subprocess, "Popen", lambda *args, **kwargs: process) + monkeypatch.setattr(script_cases.selectors, "DefaultSelector", lambda: selector) + monkeypatch.setattr(script_cases.os, "read", lambda *args: b"READY\n") + monkeypatch.setattr(script_cases.time, "monotonic", lambda: now) + monkeypatch.setattr(script_cases, "_terminate_process_group", lambda process: setattr(process, "returncode", -15)) + + result = run_until_ready(["demo.py"], r"READY", startup_timeout=300.0, soak_time=5.0) + assert result.ready + assert result.stopped_after_soak + assert result.elapsed == 304.0 + + def test_subprocess_supervisor_ignores_fatal_output_after_intentional_teardown(monkeypatch): """Fatal-looking output caused by intentional teardown must not fail a healthy launch."""