Repository navigation
server: stop requests at the wall's ends, keep placeholder distances out of them, name things without ids - #122
AidenJohnston wants to merge 15 commits into
Conversation
Scene.coverable let every band reach the extension piece past a limit
end, so a spot beside a "Something blocks it" end asked for wall past
the end (to 26 ft 2 in on a wall that ends at 19 ft 9 in), which the
app can never show and drops. Only the ground is coverable past a limit
end now. _missing_json also clips the views checks take straight from
the 1D bands (the wall above the battery, the gap in front, the space
overhead) to the chain's ends. A check whose radius crosses a limit end
stays UNSURE for a person.
The ground request past a limit end now says what the view looks for
("to check for an AC unit or a pool"), from the checks whose views
reach past the end.
Fixes #78.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
_missing_json merged every UNSURE check's unseen views per band and asked for the deepest, so the pool's 10 ft and the driveway's 5 ft, both placeholders with no public source, set the ground request: 14-16 ft out over the whole wall in every 4.1 field run, which nobody could show. A clearance whose distance is a placeholder still in effect now asks for no view (Check.asks_for_views). It still reads the area and stays UNSURE until it is seen, and the summary names it for a person rather than counting it among the checks more views settle. A value the private rules set counts as real even though the public flag merges under it, so the requests come back under those rules. Scoped to check_clearance (pool, driveway). headroom and wall_equipment_above are placeholders too but keep asking: headroom is the only check that asks for the overhead band, which the app answers with a tilt-up, and wall_equipment_above's views lie inside the gas check's. The tests that pinned a pool-sized request now use real pool and driveway values. Part of #75. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reasons were built from the ids in `subject`: "objects[1] ac is 3 ft 0 in
(± 6 ft 0 in) from the battery", "facing[1] ... may or may not be over
the battery", "ground[0] drive", and the route's blockers. The unseen
reason hardcoded "a {noun}" ("a AC unit"), and solve() appended the
demo-rules notice to the summary the app shows.
- `subject` stays the id (siteplan keys geometry by it). Reasons name
the thing by what it is and roughly where: "the AC unit about 13 ft
right of the meter", "what faces the wall about 6 ft right of the
meter", "the stretch with no wall about 3 ft right of the meter",
"the driveway". check_clearance's items carry the description.
- _a() picks the article ("an AC unit", "an electrical box").
- Check labels keep their initialisms inside a sentence ("distance from
AC units"), and the reject summary names checks by label, not id.
- The summary no longer ends with policy.notice; the notice stays in
policy.notice for the team.
test_homeowner_text scans every answer and every weighed position's
reasons over the examples, scenes that reach each kind of reason, and
random cluttered scenes: no id, no underscore, no "a AC", no notice in
the summary.
Fixes #74.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
AidenJohnston
left a comment
AidenJohnston
left a comment
There was a problem hiding this comment.
Adversarial review of draft #122 at a22c2d4 (U8: #78, #74, server half of #75).
What I ran
- PR head, detached checkout:
ruff checkclean,ruff format --checkclean,pytest -qgave 372 passed and 2 failed. The 2 failures are the known Windows-only ones. - The PR merged over the current
t3/servertip (9176125).t3/serverhas moved on 2 commits since the 7133283 base the body names. The merge is clean, andpytest -qon the merged tree gave 378 passed and the same 2 Windows-only failures. gh pr checks 122: Server checks pass, Label PR size passes, CodeRabbit skipped (draft). No iOS files changed, so the missing iOS run doesn't matter here.- A two-round probe on the PR's own
spot_beside_a_limit_end()fixture: solve, add exactly thecoverage.observedentries thatmissing_evidenceasks for, then solve again. Result below.
Must fix
-
server/solver.py:1729-1745: after #78, the summary still asks for "more views" for checks that no request can settle. The PR clips wall, facing and overhead views at the end marks and leavesunobserved_wall_linesunclipped. So a wall check whose radius crosses a limit end stays UNSURE withunsure_cause: "unobserved"andasks_for_views == True, but_missing_jsondrops the part past the end. Probe, round 2, after the homeowner supplied every requested view:missing_evidenceis[]gas_clearanceandopening_clearanceare stillunobservedsummary: "More views are needed around the best spot, 10 ft 3 in right of the meter: 2 checks depend on areas the scan did not see; a person also needs to check distance from the driveway; distance from a pool."
So the homeowner is told to capture more with nothing on the "Still needed" list. The #49/U9 gap loop reaches this state as soon as it captures the newly capturable clipped wall item, which is the flow this PR unblocks. It is the same inconsistency the PR fixed for the placeholder checks (#75), and it contradicts the README and commit text ("a check whose radius crosses a limit end stays UNSURE for a person"): the summary doesn't name those checks for a person.
Fix: count a check as "more views needed" only if it appears in a band request that is actually emitted. For example,asked = {i for m in missing if m["kind"] == "band" for i in m["checks"]}andunseen = [c for c in best.checks if c.outcome == UNSURE and c.unsure_cause == "unobserved" and c.id in asked], with every other UNSURE check going into the "a person also needs to check" list. That also coversasks_for_views. Add a round-2 test totests/test_requests.py: supplyspot_beside_a_limit_end()'s requests, then assertmissing_evidence == []and that the summary starts with "A person needs to check".
Should fix
server/solver.py:449:_a()on a mass noun. "The footprint stands on {_a(_noun(g.type))}" becomes "stands on a gravel", "a mulch" or "a concrete" whenever a rules file leaves those surfaces out ofground.allowed. Under the public rules onlydriveanddeckfail, so the tests don't reach it, but a privateground.allowedcan. Before this PR the text read "stands on gravel". Only count nouns should get an article; keep the bare noun for concrete, gravel and mulch.server/solver.py:1830:objects_not_usedmessages still use the raw type: "The ac marked 3 ft 0 in right of the meter…" and "The elec box marked…".test_homeowner_textscans these messages, but it only rejects underscores, ids and "a AC", so both pass. The new README line says every homeowner message names things by what they are. Use_noun(kind)here. The app doesn't show this field today, so this is not a blocker.server/solver.py:1578with_either: the hint can read "to check for a gas meter or pipe or an AC unit" (probe, round 1). The double "or" reads badly. Either give gas the hint noun "a gas meter" or join with commas.
Notes
- Scope deviation, surfaced properly. The brief said to drop the views of every check on a placeholder rule. The PR limits that to
check_clearance(pool and driveway), soheadroomandwall_equipment_abovekeep asking. The reason given (headroom is the only check that asks for the overhead band, which the app's tilt-up answers) is sound, and the body calls it a team decision. Nothing else looks like a product decision made silently. Dropping the notice fromsummaryfollows the issue's fix direction. Solver._placeholderagrees withoverridden_keysand_placeholders_leftfor a cited value, including the stricter group handling in 6200098. Theprivate-fileparametrisation oftest_a_real_pool_rule_brings_its_request_backpins it.- Private tests. The Base-derived tests in
private/testsare not on this machine, so I couldn't run them. They may assert on the old reason text (ids) or on pool-sized ground requests. Runuv run pytest ../private/tests -qbefore this leaves draft. - Clipped wall item vs the app. The wall item ends at
scene.s_max, rounded outward to 1e-6 ft.GapPlanner.reachesPastEndallows 0.01 ft, so the app should keep the item. That still needs a device check, and the body already lists it as device-only. subjectstill holds the id, andsiteplan._dimensionskeys geometry by it. The new"what"key on route crossings isn't serialized, and_route_jsonpicks its fields one by one, so there's no schema change. I grepped every call site of_report_rangeandcheck_clearance, and all are updated.- Public-repo check: diff, body and the three commit messages are clean. They contain no names, meter numbers or serials, addresses, private values or media. The distances quoted come from the public issues.
- PR body: it has Fixes #78, Fixes #74 and Part of #75, the base, verified vs CI/device-only, files touched, dependencies and the attribution line. The base SHA (7133283) is now 2 commits behind the
t3/servertip. The merge is clean and passes (see above), but consider rebasing so the Linux CI runs on the combined code.
Verdict: blocked (one must-fix: the summary says "More views are needed" with no request to settle it, in the limit-end flow this PR opens up)
🤖 Generated with Claude Code
After the end clipping (#78), a check whose radius crosses a limit end stays unseen once every emitted request is met, so the summary kept saying "More views are needed" with nothing left to show. Only checks named in an emitted band request now count as needing views; every other unsure check is named for a person. Also from the review: ground types that are materials read without an article ("stands on gravel"), set-aside marks use the homeowner noun, and the past-end hint names "a gas meter" so it reads with one "or". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Addressed the review of a22c2d4 in c8d3c9d. Must fix
Should fix
Notes
Checks (in
|
|
Verdict: ready
🤖 Generated with Claude Code |
|
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8d3c9d5d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| a, b = max(view.a, scene.s_min), min(view.b, scene.s_max) | ||
| if b - a < COVERAGE_TOLERANCE_FT: | ||
| continue | ||
| view = replace(view, a=a, b=b) |
There was a problem hiding this comment.
When a clearance view crosses a limit end, clipping it here still leaves the check ID attached to the shortened request, but submitting exactly that request cannot settle the check because the unseen portion beyond the end remains. This contradicts schemas/result.schema.json, which guarantees that supplying a band request's span_ft and out_ft settles it, and can break clients that use missing_evidence as a recapture protocol. Either distinguish this as partial evidence requiring manual review or avoid advertising the clipped request as settling that check.
Useful? React with 👍 / 👎.
 |
Conflicts: - scene.py coverable: the ground keeps the base's coverable_span() and house() difference; the wall bands keep this branch's stop at both end marks (#78). - solver.py route_for: the base's route error over every wall the cable runs along, with this branch's maybe_blockers holding crossings (their homeowner name) rather than ids. - test_policy_and_auth: the unsure example asks only for past_end; both comments kept. After the merge the ground request is the smallest depth that covers the region (5682a67), which the seen ground's tolerance can put a hair under D + 3, so the two D + 3 bounds allow COVERAGE_TOLERANCE_FT. The base's one-round property now counts only checks that ask for views, as test_coderabbit_review already did: placeholder distances ask for none (#75). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
view_to_cover bisects to the depth at which the unseen area left falls to its bound, so a speck just under the bound always remains. At 1e-9 sq ft one sat where the gas and AC clearances' 3 ft arc meets the view's edge, inside their radius: capturing exactly the request left both unseen, with nothing more to ask for. test_coderabbit_review's property found it after the merge, once the placeholder pool and driveway stopped sizing the request (#75) and the request shrank to the gas and AC reach. The bound is now 1e-12 sq ft (the request there grows from 3.362958 to 3.363014 ft). The scene is pinned in test_requests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
After #78 a request for the wall, the gap in front of it or the space overhead stopped at a limit end but still named the check whose radius crossed the end. That stretch still counts as unseen, so capturing exactly the request left the check UNSURE. The app reads a request's `checks` as the checks it settles (ResultReading.evidenceIndex(settling:) on beta), and result.schema.json promises that supplying a request settles it. Beside a limit end the gas and opening clearances were named on the wall request, and facing_gap and headroom on theirs, and all four stayed unseen after the capture (Codex review of c8d3c9d). settled_by_views() now says whether views can settle a check: not one on a placeholder distance (#75), nor one whose view of the wall, gap or overhead reaches past a limit end by more than the coverage tolerance. Such a check asks for none of its views, so they neither size nor name a request, and the summary names it for a person from the first answer. Scene.coverable keeps the base's wall stretch past a limit end so that view shows up to be tested; views past an unexplored end are still clipped (walking on settles those). test_requests pins it beside a limit end for the wall and for facing and overhead: those checks aren't named, the wall request stays inside the end, and capturing every request leaves no named check unseen. The one-"or" hint is tested on _either directly, since the gas clearance no longer asks there. The one-round properties in test_final_review and test_coderabbit_review count checks by settled_by_views. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Updated for review at f6f6089. Merge of
|
7f5ddc4 bounded the unseen area left at 1e-12 sq ft instead of 1e-9. That moved the speck rather than removing it: a verifier's sweep still found a sliver 4.25e-7 ft inside a radius, against the checks' 1e-6 ft tolerance. view_to_cover now takes the check's own test. _missing passes one that asks Solver._covered whether anything the check reads there, that a view can show, is left strictly within its radius. The bisection then stops exactly where the check is settled. The area bound is gone. Requests come out up to SEEN_GROWTH_FT + SEAM_FT (0.005 ft) shallower than the depth a check needs, as seen ground grows by that. The depth bounds in test_requests and test_capture_contract allow that and no more (before: the full 0.01 ft tolerance, and exact D + 10 for the pool). The scene pinned in 7f5ddc4 still passes, and a sweep of 303 gap positions around it leaves no named check unseen after the capture. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ests leave
Beside an unexplored end, round 1 named the AC, gas and opening clearances on band requests.
After capturing exactly those, all three were still unseen: part of what they read lies past
the end, where only walking on (the past_end request) shows it. Round 2 had only the past_end
request, and the summary sent the three to a person ("A person needs to check..."), because
c8d3c9d counted only checks named on band requests. On a726199 it read "More views are
needed... 5 checks depend".
- walk_on_settles() finds the unexplored ends past which a check's unseen area lies. That is
a wall, gap or overhead view reaching past the end, or ground or wall within the radius
that Scene.coverable leaves out (Solver._past_unexplored, deferred like missing_later).
- A past_end request in a manual review now lists those checks in `checks`, which the schema
already allows. solve() counts a check as needing more views when any request names it.
So the contract holds for band requests together with walking on, and the app can find
the request that settles a check the same way for both kinds.
- _missing skips a part whose unseen area within the check's exact radius is empty. With the
depth search stopping at the check's own test (2f93263), the sliver between the radius and
the polygon that circumscribes it gave a ground request 1e-6 ft deep in round 2.
- Check.all_missing() is cached, since the summary now reads it again.
test_capturing_the_requests_settles_every_check_they_name gains the unexplored variant: round 2
has only the past_end request, it names exactly the three checks still unseen, and the
summary counts them as needing more views.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… names it result.schema.json described `unobserved` as "more photos fix it". That is no longer true. Placeholder distances (#75) and checks that need the wall past a limit end (#78) are unobserved but named by no request, and the summary sends them to a person. The unsure_cause description now says that more views settle an unobserved check only when a missing_evidence request names it, and that a check no request names needs a person. The missing_evidence description and the README say that every band request, together with walking on for any past_end request, settles every check the requests name. No enum value or key is added. The app vendors result.schema.json and checks its hash (JSONSchemaValidatorTests on beta). Its copy needs refreshing once this lands on t3/server. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
test_s4_round's property found a scene where the check's test from 2f93263 raised "Overlay input is mixed-dimension". Overlaying the unseen ground after a capture can leave line fragments, which the next overlay refuses. The test and Solver._past_unexplored now keep only the areas (polygonal), as view_to_cover already did. The scene is pinned as an @example on test_a_captured_request_never_comes_back, and it fails without the fix. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
t3/server gained da6d308 and 0ac96a3: the battery's errors in plan and along the walls, and check_clearance items that say whether each was placed in plan. solver.py conflicted: - check_clearance's items keep this branch's homeowner description and gain the base's in-plan flag: (subject, description, geometry, error, counts, in plan). _items adds o.in_plan, and the driveway item passes True with the base's along_err and this branch's noun. - _missing keeps this branch's skip of parts no view can show within the radius, and the base's polygonal() on the ground region. - route_for takes the base's error (the meter's one-sided, and the slide), with this branch's maybe_blockers holding crossings rather than ids. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Follow-up to the verifier's review of f6f6089. New head: ee882e4.
Checks at ee882e4:
🤖 Generated with Claude Code |
…es there
An unexplored end out of cable reach raises no past_end request, but settled_by_views didn't
know that. With the wall ending 22 ft right of the meter and the lawn pad just before it,
round 1 named the AC, gas and opening clearances on band requests ("7 checks depend"). After
capturing exactly those, all three stayed unobserved with nothing naming them, and round 2
sent them to a person. A verifier hit this in 7 of 400 random scenes.
- open_ends(solver) gives the unexplored ends within reach, the ones past_end requests ask
to walk on past; solve() uses it throughout.
- settled_by_views(chk, scene, reachable) is false when part of what the check reads lies past
an unexplored end not in `reachable`. Such a check asks for no views and goes to a person
from round 1. walk_on_settles is cached per check.
- The unobserved_area reason is built after the past_end requests, so a round whose only
request is past_end still lists the checks it names, as the summary does.
test_capturing_the_requests_settles_every_check_they_name gains a beyond-reach variant, which
fails on ee882e4, and the unexplored variant checks the reason. The one-round properties pass
the reachable ends in.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The missing_evidence description said walking on for a past_end request "settles" the checks it names. It doesn't: it lets the next answer ask for their views. It now says that, matching the README and the unsure_cause description, and both of those also list an unexplored end beyond reach among the cases no request names. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brings in 3baa338 (the route's meter error bounded exactly, a PASS held strictly). No conflicts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Follow-up to the round-2 verifier's review of ee882e4. New head: 7046c93.
Checks at 7046c93:
🤖 Generated with Claude Code |
|
@greptileai review Please review the current head |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Only the ground is asked for past an end mark: past an unexplored end only | ||
| # walking on settles the wall (a past_end request), and a view past a limit end | ||
| # by less than the tolerance is rounding. | ||
| a, b = max(view.a, scene.s_min), min(view.b, scene.s_max) |
There was a problem hiding this comment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/solver.py
Line: 1874
Comment:
**Clipped view repeats indefinitely** When wall-position error makes a facing or overhead check reach less than 0.01 ft past a limit end, the check is still treated as one a view can settle. This line clips the request at the wall end, leaving that short stretch unseen. Capturing the requested view can therefore produce the same request again instead of sending the check to a person.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
@AidenJohnston, verified at 7046c93b. The request generator does repeat. Full solve has not been shown to pick a candidate that hits it, though.
How it happens: wall error widens a candidate at [9, 11.583] to [8.578, 12.005], which crosses the right limit end at 12 ft by 0.005 ft. _sides_past uses COVERAGE_TOLERANCE_FT (0.01 ft, solver.py:1811-1814), so settled_by_views still treats facing and overhead as settleable. The request is then clipped to 12 ft (:1874). After the capture, seen_to still reports no depth in the last 0.005 ft (scene.py:378-384), so both checks stay unsure/unobserved (solver.py:900-908) and the same requests come back.
The reproduction below uses the repository's own test helpers on a synthetic scene, with nothing mocked. I extracted the tree with git archive 7046c93b server and ran it with the server's venv:
import sys
sys.path[:0] = ['<tree>/server', '<tree>/server/tests']
from helpers import golden_rules, pads_ground, parsed, run
from test_requests import facing_and_overhead_past_the_end, captured_exactly
from solver import Solver, evaluate_start, _missing_json, open_ends
loaded = golden_rules()
raw = facing_and_overhead_past_the_end()
raw['walls'][0]['plus_minus_ft'] = 0.4216666666666667 # 0.005 ft past the 12 ft limit
raw['ground'] = pads_ground([(9, 17)], hi=32)
for round_no in range(3):
scene = parsed(raw, loaded)
candidate = evaluate_start(scene, loaded, 9)
requests = _missing_json(candidate, scene, open_ends(Solver(scene, loaded)))
print(round_no + 1, [(m['band'], m['span_ft']) for m in requests if m.get('band') in ('facing', 'overhead')])
raw = captured_exactly(raw, {'missing_evidence': requests})Output: all three rounds request facing [8.578333, 12.0] and overhead [8.578333, 12.0] for facing_gap and headroom. Both checks stay unsure/unobserved after each round's requests are captured exactly. As controls, with error 0.415 (the span stops short of the end) the checks pass after one capture, and with 0.43 (more than 0.01 ft past) no view is requested. On the same scene, run(raw, loaded) chooses [8.995, 11.578333] and leaves no requests, so a homeowner-facing loop has not been shown, and P1 severity isn't established.
A possible fix: have settled_by_views reject a non-ground view that goes past a limit end by more than a numerical epsilon, and keep the 0.01 ft coverage tolerance where it is. Add this 0.005 ft tail as a fixed-candidate regression that asserts neither check is offered as capturable, and keep the no-tail control. I have not changed this branch.
There was a problem hiding this comment.
Thanks for the detailed reproduction. This confirms the underlying correctness issue in _missing_json: a non-ground view can be clipped at a limit end while settled_by_views() still claims the check is capturable, so capturing the exact request leaves the same checks unseen and regenerates the request. The 0.005 ft case is especially useful because it is within COVERAGE_TOLERANCE_FT; the tolerance should remain useful for ordinary rounding, but it must not make a clipped wall/facing/overhead request appear settleable.
I agree that the reproduction establishes the generator defect, but not yet a homeowner-facing loop or P1 impact: it calls _missing_json on a fixed candidate, while solve() chooses the candidate itself. I would keep the finding as a correctness issue and add the fixed-candidate regression described. If the candidate can also be selected by solve(), an end-to-end assertion would establish the stronger impact. The fix should make settled_by_views() reject any non-ground missing view that crosses a limit end by any positive geometric tail, using a numerical epsilon rather than the 0.01 ft coverage tolerance.
This branch has not been deployed
2 participants
Fixes #78
Fixes #74
Part of #75 (the server half; the app's span copy is a separate unit)
Base branch:
t3/server, merged in at 3baa338 (52a7fd5, ee882e4, then 7046c93). This PR doesn't depend on another open PR.Base owner's OK needed: 7f5ddc4 and 2f93263 change
Scene.view_to_cover, and f6f6089 changesScene.coverable. Both are inserver/scene.py, whicht3/serverowns (see "Files touched").What changed
In the build 4.1 field runs, the upload's "Still needed" list asked homeowners for views nobody could give, and the result screen showed internal ids and the team's demo-rules notice.
Requests stop at the wall's ends (#78)
unobserved_wall_linesis unclipped, see "Why"), so no capture could settle the check. The summary names it for a person from the first answer: "More views are needed … ; a person also needs to check distance from gas equipment".settled_by_viewsinsolver.py). The app relies on this:ResultReading.evidenceIndex(settling:)treatschecksas the checks a request settles. Earlier revisions of this PR clipped the request at the end but still named those checks, which broke that rule (the Codex review of c8d3c9d).past_endrequest in a manual review now names, inchecks, the checks whose unseen area lies past its end (walk_on_settles). Walking on shows that area, and the next answer asks for the views. The summary counts a check as "more views needed" only if some request names it. An unexplored end beyond cable reach raises nopast_endrequest, so a check that needs anything past it asks for no views and goes to a person from the first answer (open_ends,settled_by_views).Placeholder distances don't size the requests (#75, server half)
_missing_jsonmerged the unseen views of every UNSURE check in a band and asked for the deepest. The pool's 10 ft and the driveway's 5 ft therefore set the ground request (13 to 16 ft out in the three runs). Both values areplaceholder: true, with no public source.check_clearancesetsCheck.asks_for_views = Falsewhen its distance is a placeholder still in effect. Those checks ask for no views. They still read the area and stay UNSURE until it is seen, and the summary names them for a person.placeholder: truemerges under it (Solver._placeholderchecksprivate_keys). Under private rules, the pool and driveway requests come back.Homeowner text names things, not ids (#74)
subjectstill holds the id (objects[1] ac), becausesiteplankeys geometry by it. Reasons name the thing and roughly where it is, for example "The AC unit about 13 ft right of the meter is 3 ft 1 in (± 0 ft 2 in) from the battery…" or "the stretch with no wall about 3 ft right of the meter". A driveway is "the driveway" with no position._a()picks the article ("an AC unit"). Surfaces that are materials take none ("stands on gravel", but "stands on a driveway"). Marks set aside past an end use the same nouns ("The electrical box marked…").summaryno longer ends withpolicy.notice. The notice is still inpolicy.notice.Ground request depth (found while merging)
test_coderabbit_review's property found that capturing a small ground request exactly could leave the gas and AC clearances unseen.view_to_cover(5682a67 ont3/server) bisected until the unseen area left was under 1e-9 sq ft, which left a speck inside the check's radius.Solver._coveredover what a view can show) intoview_to_cover, so the search stops exactly where the check is settled. As a result, requests come out up to 0.005 ft (SEEN_GROWTH_FT + SEAM_FT) shallower than the depth the check needs, because seen ground grows by that much.test_s4_round's property found an overlay that left line fragments.Why
headroom(6.5 ft) andwall_equipment_above(0 ft).headroomis the only check that asks for the overhead band, which the app answers with a tilt-up, so dropping it would switch that flow off under the demo rules.wall_equipment_aboveasks only for views the gas check already asks for. So the change is limited to the clearance distances Ground requests driven by placeholder rules ask for 14–16 ft of ground nobody can show, and the copy misdescribes the span #75 is about (pool and driveway). Ifheadroomshould stop asking too, that is a one-line change and a team decision.unobservedno longer means "more photos fix it". A check isunobservedwhenever the area it reads wasn't seen, but views settle it only when a request names it.result.schema.json'sunsure_causeandmissing_evidencedescriptions and the README now say so. No enum value or key was added.unobserved_wall_linesalso stop at a limit end? Clipping it would let the checks above pass without anyone seeing that stretch, so this PR leaves it unclipped and sends them to a person. That's a team decision.Verification
Commands run in
server/at 7046c93:uv run ruff check .: "All checks passed!"uv run ruff format --check .: "38 files already formatted"CI=1 uv run pytest -q: 424 passed, 1 xfailed, 2 failed. The 2 failures are the known Windows-only tests:test_zip_entry_escaping_the_bundle_is_400[..\\x]andtest_deeply_nested_input_is_a_scene_error. GitHub's Linux Server checks job runs them.New tests (all synthetic scenes):
tests/test_requests.py("Still needed" asks for wall past a limit end (wall to 26 ft 2 in; the wall ends at 19 ft 9 in) #78, Ground requests driven by placeholder rules ask for 14–16 ft of ground nobody can show, and the copy misdescribes the span #75):_either.test_capturing_the_requests_settles_every_check_they_name): after capturing exactly the band requests, no check they named is still unseen, for a wall case, a facing/overhead case, an unexplored-end case and an unexplored end beyond reach. In the unexplored case, round 2 has only thepast_endrequest. It names exactly the checks still unseen, theunobserved_areareason lists them, and the summary counts them as needing more views. Beyond reach, round 1 names none of them and round 2 asks for nothing.tests/test_homeowner_text.py(Result text shows internal ids ("objects[1] ac"), says "a AC unit", and shows the demo-rules notice to homeowners #74): this test scanssummary, reasons, check reasons, request messages andobjects_not_usedmessages. It rejects ids, underscores, a wrong article, and the notice in the summary. It runs over the examples, the fixture, scenes that reach each kind of reason, and 40 random cluttered scenes.tests/test_s4_round.py: the mixed-dimension scene is pinned as an@example.Updated tests:
test_capture_contractandtest_s4_roundpin pool-sized requests under rules with real pool and driveway values. The depth bounds allowSEEN_GROWTH_FT + SEAM_FT.test_coderabbit_reviewandtest_final_reviewcount only checks for whichsettled_by_viewsis true.test_policy_and_auth,test_s4_roundandtest_field_run_2match the new wording.Not run, or only CI or a device can verify:
private/testsisn't on this machine.past_enditem now carrieschecks.policy.noticeno longer appears in the summary. On beta the app reads the notice frompolicy.notice(rulesNotice), andsummaryWithoutNoticeis a no-op.App-side follow-ups (not in this PR)
PlacementCheck.needsPersonis false for everyunobservedcheck, so the result screen shows that line under a summary that says a person needs to check. The fix isneedsPerson = unsureCause != .unobserved || evidenceIndex(settling: id) == nil.result.schema.jsonwith a hash (JSONSchemaValidatorTests). The copy needs refreshing once this lands ont3/server.Files touched
server/solver.py:Check.asks_for_views,Check.past_ends_later,Check.all_missing(cached),Solver._placeholder,Solver._items(with the base's in-plan flag),check_ground,check_clearance,check_along_wall,check_measured,_report_range,route_for,evaluate,Solver._missing,_missing_json,_past_end_hint, andsolve(summary,past_endchecks, theobjects_not_usedmessage).open_ends,settled_by_views,walk_on_settles,_sides_past,Solver._unseen_parts,Solver._past_unexplored,_noun,_a,_surface,_about,_describe,_sentence,_phrase,_either.server/scene.py(base-owned):Scene.coverable(docstring; the wall band keeps the base's stretch past a limit end so thatsettled_by_viewscan see it) andScene.view_to_cover(takes the check's test instead of an area bound).server/schemas/result.schema.json: the descriptions ofmissing_evidenceandunsure_causeonly.server/README.md: limit-end requests, placeholder requests, what settles a check,subjectcompared with homeowner text, and where the notice appears.server/tests/test_requests.py,server/tests/test_homeowner_text.py.test_capture_contract.py,test_coderabbit_review.py,test_final_review.py,test_policy_and_auth.py,test_s4_round.py,test_field_run_2.py.No change to
rules.yaml, reason codes or the error model. The schema's shape is unchanged:past_endrequests may already carrychecks.Dependencies
solve()andcheck_clearance, so either will need a merge after this PR.Risk
summaryfor the demo notice will no longer find it there. None in this repo do.Checklist
🤖 Generated with Claude Code