Repository navigation
build: include available experiment suites in root checks (#90, conflicts resolved) - #181
Merged
Merged
Conversation
The marking prompt and the mark and remove labels lowercased the feature name, so they read "Tap the ac unit" and "Mark ac unit". ScanCopy.noun keeps "AC" in capitals. The result's measurement lines and the 3D view's summary now give VoiceOver feet and inches spelled out (Distance.spoken), since it reads "ft" and "in" as letters. The on-screen text is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The upload delegate held progress at 0.99 until the server answered, and the server works out the spot inside that request. So the screen sat on "Sending measurements, 99%" for the whole analysis and "Check clearances" never lit up. Report 1 once every byte of the body is sent. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
In field test run 1 "Slow down" showed twice while the tester stood still with "What do you see?" open: turning the phone to aim reads as moving to the capture gate. Hide that one prompt while the tray is open; tracking problems still show. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The ternary between two static methods made the compiler fail with "failed to produce diagnostic for expression" on CI. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"Measured 3 ft. The rule is 20 ft" read the cable run's maximum as a minimum. The server sends each check's comparison (at_least / at_most); the result row now carries it and the measurement line reads "The rule is at least 3 ft" or "at most 20 ft". A check without a comparison keeps the bare distance. The wording lives in the app (ScanCopy), not HouseScanKit, so the check is a UI-test assertion on the demo result's window row; the kit already tests decoding `comparison` (PlacementResultTests). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eter first Device run 1 set both ends within 4 in of the meter: "Can't get there" put an end at the unbroken covered reach, which was 0 because nothing had asked for the ground in front of the meter. The scan then left out the 19 ft the walk had photographed. - "Can't get there" while the walk asks to walk a side, and a new "Wall ends here" during the walk, put that side's end at the phone's place along the wall chain, kept within the stretch walked on that side (HouseScanKit WalkedEnd). With the phone across the meter, or its place lost, the end goes as far as the walk went on that side instead of to the meter. - The walk's first request is the ground in front of the meter. The end prompt follows the walked distance, not unbroken coverage (B-06). An aim task keeps its stretch while the lag window's middle drifts under its half-width. - The wall strip shows where an end would land (a dashed line, the stretch past it dimmed, and a line when that cuts off part of the walk), and says the ends in its VoiceOver value. The end prompt shows the reticle it marks at. - Past an unexplored end with nothing between the ends in view, photos are refused and the card says "You're past the end of the wall". - -autopilotCantGetThere replays run 1 on the synthetic wall; testCantGetThereEndsWhereThePhoneIs checks the ends and the exported scene. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The window-row assertion read "at least 3 ft" from the VoiceOver value. The B-16 fix (#34) spells lengths out there ("3 feet"), so the test would fail once both land. It now checks "The rule is at least 3". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The first real-phone run's wall already had a battery, and the scene couldn't say so. `battery` is now an object type, held to clearances.battery_ft (3 ft; Base's help page and IRC 2021 R328), measured in plan like the AC clearance under the strict rule. The cable detours over it on its route, as over a box, since it stands flush against the wall. The check appears when the scene marks a battery (or the rules make its reach larger than the AC clearance's); otherwise it would repeat the AC check's coverage, so every earlier answer is unchanged apart from the rules hash. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…go to review (#35) - A server band request whose span reaches past a marked end has no capture request (GapPlanner.reachesPastEnd): the coverage map records nothing past an end, so its bar stayed at 0 % and "I can't get there" was the only way out. Ground past a limit end is still recorded, so a ground request there stays capturable. It goes to review, like a request beyond capture. - A past_end request that ends without its end marked again no longer leaves that side without an end. Skipped, the end it cleared comes back with its kind and mark time. Met, the end moves on to the far edge of the ground the request showed, still unexplored (GapPlanner.endAfterPastEnd), so the next past_end request asks for the 2 m after it instead of the ground at the meter. - ServerGapTests: aBandRequestPastAMarkedEndIsNotACaptureRequest, aMetPastEndRequestMovesItsEndOn. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Build 3.1 reached a spot and the AR view for the first time, but the spot overlaps the meter's working space and the wall export ran from 7 ft 6 in left of the meter to 6 in right. The scan was lost, so this comes from the recording and the code at ad91620. Findings are filed as #38 to #45 and as comments on #24 to #29, #31 and #35. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Cells seen before an end was set stay in the coverage map, and the lagging-band and see-behind tasks read them: after "Can't get there" put the right end 6 in from the meter on device run 2, the walk kept asking to tilt up around 7 ft right and to tilt down at 5 ft 3 in right. Past an end nothing is observed (coveredFraction) or skipped (markSkipped), so the task could never fill and "Can't get there" didn't clear it. laggingBand, hiddenNearCamera and the see-behind check now count only cells between the ends. GuidancePlannerTests.noCameraTaskPastAMarkedEnd. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Field test run 2's working-space line read "Measured 1 ft 3 in. The rule is 0 in" for a spot 1 ft 3 in inside the meter's working space: the server's measured_ft was -1.25 and Distance.feetAndInches drops the sign. A measurement below zero now reads "Overlaps by 1 ft 3 in", and a minimum of 0 reads "The rule is no overlap". Less than half an inch of overlap still reads "Measured 0 in". The app target has no unit-test bundle (adding one means regenerating the project with XcodeGen), so the check is a screen state: -uiDemoOverlap puts run 2's line in the sample result, and result-overlap asserts the row's VoiceOver value and runs the accessibility audit on it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
vendoredCopiesMatchServer compares the kit's copy with origin/t3/server, which gained the `battery` object type at 074bf1d (#27). Every iOS run from then on fails in HouseScanKit (run 36275541111 on this PR). The copy is byte for byte from 074bf1d, and its recorded hash and the provenance follow; the other two vendored files are unchanged there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The object types gain battery (issue #27). The drift test compares with the server branch's head, so every iOS run failed until this copy moved. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two bugs from the review of 3c7b83a: - "Wall ends here" during a tilt or step-back request offered the first side without an end, left first. After walking right with the left end unmarked, it ended the LEFT side at its farthest walked point, far from the phone, while its hint says "where you're standing". It now ends the side of the meter the phone is on (WalkedEnd.side, the same s along the chain WalkedEnd.end uses), falling back to the old choice when the phone's place is lost, it stands at the meter, or its side is already ended. The strip's dashed line and VoiceOver value follow, since both read the same EndPreview. - Ending both sides at the meter without walking still gave a wall of no length. finishWalk now refuses ends closer together than WallFrame.minWallLength (0.79 m, one Base Core battery's width), clears them so the walk goes on, and the card says the ends were too close. WalkedEndTests covers both: the side choice with the phone right of the meter and the left end open, and ends at the meter being too close. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
After the check's answer asks for several views, "I can't get there" on one of them used to stop the loop: the next upload showed the result, so "2 more views to finish" delivered only one view. Now it skips only the current view: the next answer raises the next item it lists, and the result shows once none is left. Each item is raised at most once per pass (GapPlanner.nextServerRequest); a past-end item is known by its side, since raising it clears the end and it comes back with a different request. A new "Show my result" button on the follow-up request stops early, and the skip button's VoiceOver hint says where it leads. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s past (#35) aRequestSpanIsReadInTheServersFeet asked for ground 0.02 ft past an unexplored end to show it isn't met. Past the end it now has no capture request, so the test checks that, and reads the shortfall from a request planned without the end. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The server added the `battery` object type (issue #27) after this branch's last CI run, and vendoredCopiesMatchServer fails on every t3/ios-mvf run until the copy is refreshed. One enum value and its description; the app doesn't export batteries yet. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…uests Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # ios/HouseScanKit/Tests/HouseScanKitTests/JSONSchemaValidatorTests.swift
…-fixes Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # ios/HouseScan/UI/Copy/ScanCopy.swift # ios/HouseScanKit/Tests/HouseScanKitTests/JSONSchemaValidatorTests.swift
A ray nearly parallel to the wall met its line hundreds of meters away, so "Wall ends here" could put an end there and the strip's preview drew a tape that long. A hit farther along the wall from the camera than the coverage map's sighting distance is now refused. An object marked past an end that came before a corner named the wall round that corner, which the export leaves out of walls[]. It now names the nearest written wall, whose line the server continues past its end. Found by the caretaker's review of 75dea41. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # ios/HouseScan/UI/Screens/WallWalkScreen.swift
…n did not see' (#45) Co-authored-by: Cursor <cursoragent@cursor.com>
The AR result was a SwiftUI Canvas over the camera, so nothing could occlude it. On the live camera it is now RealityKit content on the meter's anchor, with scene-mesh occlusion on LiDAR phones and people occlusion where supported, both on only while the AR screen is up. Replays keep the Canvas overlay. Co-authored-by: Cursor <cursoragent@cursor.com>
- #44: among UNSURE spots, one whose best estimate is past a rule (a spot overlapping the meter's working space, UNSURE only because the meter's error is large) now ranks after every spot whose estimates all clear, whatever its cable run. The strict rule is unchanged. - #45: "N checks depend on areas the scan did not see" counts only the unseen checks, and names the rest: "and 1 needs a person to judge". - #42: an object with no footprint past an unexplored end is not put on the wall's straight extension, which may not exist; one across the end keeps its part on the scanned wall. Answers list set-aside marks in objects_not_used (result schema, additive). Moved spots in the suite and on real scenes all had a best estimate past a rule (gas at 2.96 ft of 3, openings at 0 to 2.5 ft); no decision changed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Its capsule was 14% white, composited over the camera behind the card, and CI's accessibility audit measured its contrast as nearly passing on the LiDAR replay. It is now the same colour as a solid (#2D333F), so chalk text on it is about 11:1 whatever is behind the card. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit 397b6d6)
The full-flow tests audited the walk the moment it appeared, while the autopilot was still marking features, setting the ends and tilting up over a playing replay. Each audit pass caught a different crossfade; a copy fading out has no identifier, so transient issues from two passes matched on a label or on the "no element" key and failed as one lasting issue (runs 36304552823, 36305050676, 36307476187). The app now leaves <phase>.held in the gate folder while it waits for the test to finish with a screen, and the walk is audited once that file appears, on the settled "Done with this wall" state. Every state the walk passes through is still audited frozen by ScreenStatesUITests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit ad942d8)
Port the DeveloperOptions change from Aiden's 786bcf2. The 4482 Simulator audit failed when Practice text was replaced by an icon. Keep the existing beta test isolation and Details/scrolling restart fixes. Source reviewed by the permissions Astra caretaker and Codex. Swift parsing and exact owner-file comparison pass; app build and Dynamic Type runtime acceptance remain pending.
…he floor (#163) An AC unit stands on the ground in front of the wall, so a ray aimed at it meets the ground first and the wall's plane under the floor. The #140 check refused those taps. ObjectTap.place now falls back to the ground hit for an object that stands on the ground, when the wall hit is below the ground or there is none, with the fence tap's 8 m out bound and the wall-end reach check on the ground hit. Doors, windows and gas meters keep the wall-only rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…Can't-get-there flow asks for The synthetic replay saw the wall only to 6.5 ft: its walk looks 20 degrees down, and its three tilt-up views share one position, so the 7 ft row never got a second. The server's wall request (out_ft 6.500001) could not settle. - make-synthetic-replay.swift appends 20 frames (k00042-k00061) after the existing 41, which keep their numbers and bytes: a walk along the wall 22 degrees up, 0.5 m apart. Both fixtures regenerated; additions only. The project lists the new files among those kept out of the UI test bundle. - A wall request above the walk's band plays the tilt-up run, as an overhead request does (GapPlan.asksAboveTheWalk); the autopilot lets the whole run play before it ends the walk. - The held-back window is planned over the walk's frames, and for the Can't-get-there flow against the ends that flow sets (EndPolicy.walked), so its gap is one that flow asks for. testCantGetThereEndsWhereThePhoneIs sometimes never reached the gap screen. - Tests: the first 41 frames still see 6.5 ft; all 61 see 7.5 ft over the request's span and settle it; the Can't-get-there held-back gap lies inside its ends. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit 4461f13)
A detected vertical plane standing roughly parallel in front of the wall (FarSurface) now marks where the open space ends. With LiDAR depth, a reading nearer than a sample counts as the end of the space, not as something in front of the wall, when the sample lies past that surface or what the reading met is that surface. Such rows are neither seen nor hidden, so the walk no longer asks to look past a corridor's far wall. Anything nearer the wall than the surface still hides. The engine measures the surface from ARKit's planes whenever they, the wall or the stretch seen change, on phones with and without LiDAR. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A server walk-out needs the walk to pass its clearance plus the wall's position error. In a space narrower than that, the line lies behind the far wall or fence and no walk can meet it; on build 7.1 the card sat at 0 % while the homeowner walked against the far wall. GapPlanner.walkOutBlock now finds where the line lies past the detected far surface less the room a homeowner takes behind the phone. The card then says the space ends about X out and that "I can't get there" is the answer, the dotted line behind the wall goes, and the skip is logged with the reason. While a walk-out is up the card also gives the phone's distance out and the distance that counts there, rounded to 3 in; the reply and the guidance log use the request without that reading, so it doesn't relock the reply on every step. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Without LiDAR, a narrow space's far wall or fence reached the server only as the walked path less its position error, which could never show the facing gap. Where the walk came within a homeowner's depth of a surface ARKit's planes found facing the wall, the space is known clear up to it, so the scene now carries that surface as a `facing` entry with its error written out: the wall's position error plus a detected plane's default. The server then checks the real space, as it does the mesh's facing. Only when the mesh measured no facing gap: the mesh sees the gap itself and anything standing in it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The log message is an escaping autoclosure, which can't capture the inout request. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ized one Greptile noted the deflate attempt allocated a second buffer almost as large as each entry. The encoder now streams through 64 KiB, so the only added memory is the compressed output, and stops once that output stops being a saving. The write() and byte-identical doc comments say what they now mean, and the deflate tests are behind the same canImport(Compression) as the code. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…a guessed ground (#163) Most AC taps now land on the ground, so the bracket centred on the tap sat half under the floor. It now stands on the ground under the tap. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Only the build 7.1 packet shows the planes; the build 5.1 one showed the far wall in the mesh. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit ee8cdcc)
ios: land an AC tap on the ground when its ray meets the wall under the floor
ios: bring the t3/ios-mvf UI test flake fixes to beta (reply-capsule contrast, audit the walk once the autopilot holds it, the held-back gap the Can't-get-there flow asks for).
…#164)" This reverts commit 64b9719. Held back for the demo. Replayed on the build 7.1 geometry, the far surface could be the corridor wall's own detected plane, where the chain piece runs about 11 degrees off it, and the export then sent a facing gap of under 3 ft that would fail the corridor's spots. It can come back once the own-plane rejection is proven on field data, and once it no longer reports a surface the walk went well past. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…164) Replayed on the build 7.1 corridor, FarSurface picked the corridor wall's own detected plane: the chain's piece runs about 11 degrees off it, so the plane passed the 0.5 m minimum two thirds of the way along and read as a far surface under 3 ft out. A candidate plane now has to stand at least minOut in front of every piece its outline's ends lie along, at both ends; a straight plane clear of a piece at both ends is clear of it everywhere. An end round a corner is held to no piece, so a corridor's far wall ending at the corridor's mouth still counts. The plane the meter's wall was refit to can be left out by id. Tests: a wall's own plane 10 degrees off a chained piece, crossing 0.5 m inside the requested span, is never chosen while the corridor's far wall is; a 7.6 m far wall 11 degrees off at 1.8 to 2.2 m counts and a 0.75 m plane doesn't. Also notes on the #160 path that a large flat obstacle can qualify, leaving what's behind it unseen rather than hidden. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ARKit refines its planes about ten times a second, so the headline's distance could move or come and go, re-arming the reply's lock each time. The engine now keeps where the space ends for the request once the card has shown it, and leaves out the plane the meter's wall was refit to when it measures the far surface. The walk-out and skip logs write their distances to the centimetre. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…164) Round-2 review of #168: holding each outline end to whichever piece's stretch it lay along still let the build 7.1 corridor wall's own plane through. Its near end ran past the corner, so it was judged against the meter's wall, 0.515 m in front of it. The chain's end pieces also run to infinity, which rejected a fence whose end lay along a side piece. Each end is now held to the piece nearest it in plan among those within maxAngle of parallel to the plane, measured against that piece's whole line, and the plane is rejected if either end is within minOut of it or behind it. Pieces turned further from the plane are ignored, as they are when a cell meets it, so a corridor's far wall is never held to the wall across its mouth. Tests: an own plane running 0.52 m past a convex corner is rejected; a fence parallel to the meter's wall that runs up to a side wall is kept. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ios: treat a corridor's far wall or a side yard's fence as where the space ends
…on-main # Conflicts: # CONTRIBUTING.md # Makefile
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Important Review skippedToo many files! This PR contains 746 files, which is 646 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (154)
📒 Files selected for processing (746)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Review skipped Codemetrics is installed on this repository, but it hasn't been added to your team yet. To enable AI code reviews for this repo, go to your dashboard and add house-scanning-master to your team's repositories. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+31
to
+33
| uv run python -m meter_eval.sweep --shard 0 --shards 2 & \ | ||
| uv run python -m meter_eval.sweep --shard 1 --shards 2 & \ | ||
| wait |
There was a problem hiding this comment.
wait can still report success, so make sweep may finish with missing rows. make q2 analyzes the rows it finds without checking that the sweep is complete, which can produce misleading retake thresholds.
Suggested change
| uv run python -m meter_eval.sweep --shard 0 --shards 2 & \ | |
| uv run python -m meter_eval.sweep --shard 1 --shards 2 & \ | |
| wait | |
| uv run python -m meter_eval.sweep --shard 0 --shards 2 & first=$$!; \ | |
| uv run python -m meter_eval.sweep --shard 1 --shards 2 & second=$$!; \ | |
| wait "$$first"; first_status=$$?; \ | |
| wait "$$second"; second_status=$$?; \ | |
| test "$$first_status" -eq 0 && test "$$second_status" -eq 0 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: experiments/meter-closeup/Makefile
Line: 31-33
Comment:
**Sweep failures go unnoticed** If either background shard fails, the unqualified `wait` can still report success, so `make sweep` may finish with missing rows. `make q2` analyzes the rows it finds without checking that the sweep is complete, which can produce misleading retake thresholds.
```suggestion
uv run python -m meter_eval.sweep --shard 0 --shards 2 & first=$$!; \
uv run python -m meter_eval.sweep --shard 1 --shards 2 & second=$$!; \
wait "$$first"; first_status=$$?; \
wait "$$second"; second_status=$$?; \
test "$$first_status" -eq 0 && test "$$second_status" -eq 0
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+84
to
+86
| meter-closeup: | ||
| $(call require,$(METER_CLOSEUP),#16) | ||
| $(MAKE) -C experiments/meter-closeup check |
There was a problem hiding this comment.
meter-closeup target delegates to a check that runs lint and tests but skips CI’s uv sync --locked. If the lockfile is stale, the local uv run commands can update it and let make check pass even though the meter-closeup CI workflow rejects the lockfile. This makes the root check less reliable as a local CI check.
Suggested change
| meter-closeup: | |
| $(call require,$(METER_CLOSEUP),#16) | |
| $(MAKE) -C experiments/meter-closeup check | |
| meter-closeup: | |
| $(call require,$(METER_CLOSEUP),#16) | |
| cd experiments/meter-closeup && uv sync --locked | |
| $(MAKE) -C experiments/meter-closeup check |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Makefile
Line: 84-86
Comment:
**Meter lockfile check omitted** The root `meter-closeup` target delegates to a check that runs lint and tests but skips CI’s `uv sync --locked`. If the lockfile is stale, the local `uv run` commands can update it and let `make check` pass even though the meter-closeup CI workflow rejects the lockfile. This makes the root check less reliable as a local CI check.
```suggestion
meter-closeup:
$(call require,$(METER_CLOSEUP),#16)
cd experiments/meter-closeup && uv sync --locked
$(MAKE) -C experiments/meter-closeup check
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+68
to
+70
| measure-lab: | ||
| $(call require,$(MEASURE_LAB),#7) | ||
| $(MAKE) -C experiments/measure-lab check |
There was a problem hiding this comment.
MeasureLab.xcodeproj can therefore pass make check while Measure Lab CI fails, reducing the value of the root check as a local CI check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Makefile
Line: 68-70
Comment:
**Measure Lab drift check omitted** The new root target runs Measure Lab’s tests and build, but not the workflow’s check that regenerates the Xcode project and detects drift. A stale committed `MeasureLab.xcodeproj` can therefore pass `make check` while Measure Lab CI fails, reducing the value of the root check as a local CI check.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
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
2 participants
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.
Sam's #90 (
make checkalso runs the experiment suites a branch has), with its conflicts againstbetaresolved so it merges on top of #179.Makefile: keeps build: include available experiment suites in root checks #90's experiment targets and beta'smake smoke, and lists both in one.PHONY.CONTRIBUTING.md: keeps beta's iOS row andfull-uinote, and adds build: include available experiment suites in root checks #90's Makefile-dispatch row andmake checkdescription.betacontent,make checknow also runs scoring, measure-lab, evals and meter-closeup.recon/isn't on beta, so its target is skipped.Tested on the Mac mini from this branch:
bash tests/makefile.shpasses every case, andmake -n checklists the suites above.Merge after #179, with "Create a merge commit". Until #179 lands, the diff also shows beta's commits.
🤖 Generated with Claude Code