https://docs.all-hands.dev/ logo
Join Slack
Powered by
<@U0BME18CF7S> please review <https://github.com/O...
# automations-debug
c
@fresh-machine-37269 please review https://github.com/OpenHands/OpenHands/pull/18093 and feel free to approve, request changes or comment, as you think appropriate.
👀 1
f
🤖 On it! View progress here: oss-agent-canvas.ngrok.dev/conversations/…
✅ Done! Done — I reviewed PR #18093 and left a review comment on it. Short version: the tooling is solid. I checked out the branch and confirmed 
map check
is clean (27 families / 716 IDs) and catches the F02 
Source:
drift, 
map testids
resolves all 985 cited IDs, 
map affected
works both from the index baseline and an explicit range, 
map baseline --set
correctly refuses a non-
main
ref, and the unit suite passes 36/36. Keeping the baseline in the map index is the right call. The one blocker is that the PR description no longer matches the head. The last commit removed the E2E work, but the description (and title) still advertise 
E2E:
lines, spec handling in `map affected`/`map check`/`map ids`, the 
map coverage
e2e section, "38 total" tests (it's 36), and the old 
--set HEAD
behavior. I asked for the description to be brought in line before merging — happy to approve once it is. I also flagged one design trade-off worth a conscious decision (dropping spec coverage means the map no longer records which CI suite covers a family, now documented as deliberate and tracked in #18092) and a minor broken image link in the description.
c
@fresh-machine-37269 okay fixed, please re-review accordingly
👀 1
f
✅ Done! Re-reviewed and approved ✅ The head SHA is unchanged (
95115e84
) — the fix was description-only, and it now matches the diff exactly: • Title dropped "E2E links in the map"; the Summary now correctly describes 
map affected
(families/shared/unmapped/nonUserFacing only), 
map check
(Source paths + baseline syntax), and 
map coverage
(routes + feature-component dirs, no e2e key). Confirmed against the code, and there are zero 
E2E:
lines in the map. • Test counts now read 11 added / 36 total (correct vs 25 on 
main
), and every 
How to Test
output matches: 
map check
clean, 
map testids
985/0 unresolved, 
map baseline
commitsSince:9
, 
map affected
29 changed paths / 2 shared / 0 unmapped / 27 nonUserFacing. • The 
--paths
example reproduces exactly, 
map baseline --set HEAD
is documented as exit 2, 
--set a2a74bdb…
succeeds and writes the right SHA/date (I reproduced both on a scratch copy), and the two screenshots now render. The code is unchanged from my first pass — 
map check
still catches the F02 drift when reverted, 
map affected
works both ways, the suite passes 36/36, and CI is green. Left the approval on the PR.
Open in Slack
PreviousNext
Powered by