Skip to content

feat(backend): bridge base-scene/v0 metres to the placement wire scene - #272

Merged
SamGu-NRX merged 1 commit into
alphafrom
obv/burst7-bs-scene-converter
Oct 16, 2026
Merged

SamGu-NRX merged 1 commit into
alphafrom
obv/burst7-bs-scene-converter

Conversation

@obvious-autobuild

Copy link
Copy Markdown

What

Adds backend/server/placement/scene_bridge.py — a pure convert_scene(document: dict) -> dict that turns the backend base-scene/v0 scene contract (metres, the meter_anchored/v0 frame, pydantic Scene-validated) into the placement wire scene the unchanged top-level POST /v1/placements server accepts (server/schemas/scene.schema.json, feet). This is the pinned consumer interface the placement stage's placement/scene.json input has been waiting on — until now backend/server/placement/adapter.py could only fall back to a committed reference fixture (#268) because nothing published reconstructed scenes to the solver.

Also adds backend/server/placement/tests/test_scene_bridge.py (14 tests).

Mapping

  • Units: every length converts by dividing by exactly 0.3048 (correctly rounded division, never a rounded reciprocal multiplier). Both contracts measure s along the wall chain from the meter, positive to the right seen from outside, so spans convert without rebasing: wire_s_ft = source_s_m / 0.3048.
  • meter — the frame anchor object (point geometry keeps its measured position; wall_rect geometry sits on the wall at span centre, mid-height), position and error in feet.
  • walls — the single chain wall: scaled baseline, height_ft, plus_minus_ft; no wire source guess (the source contract records how wall points were supported, not how the line was found).
  • objects — observed wall_rect hazards with the exact class map (window/door/garage_door/gas_meter/vent/downspout/pool/battery identity, ac_unit→ac, gas_regulator/gas_pipe→gas_meter, disconnect/panel/wall_box/meter_can→elec_box), source kinds vlm/tap/tape, conf unchanged (recognition confidence in both contracts), attrs carrying operable/well when present — a missing attribute stays missing, read as unknown by both sides.
  • ground / overheads / facing — level plane surface plus observed regions with wire materials; observed overhead and facing measurements converted with the lengths.
  • coverage — chain ends and every explicit band (coverage.bands and sensitivity[].bands); an absent band stays unobserved.

Refusals

Everything raises SceneBridgeError, a ValueError with code and a JSON-pointer pointer naming the offending input:

  • placement_scene_unsupported — valid scene, outside the bounded scope: more than one wall, non-house/non-observed wall roles, generated or inferred evidence on anything, unmapped classes/sources/materials, stand-off wall_rects, heightfield or non-level ground, sloped regions, facing lower_bound, grid coverage, keyframes, generated regions. Nothing is promoted or guessed.
  • placement_scene_invalid — anything Scene itself rejects, plus cross-checks of every declared s value against the plan geometry at a 1e-6 m float-noise bound: wall s_range_m vs the baseline and meter anchor, chain ends at the wall's ends, the meter's wall_rect span centred on s=0, bounded hazard spans inside the wall, reversed spans, non-finite numbers (pointer at the exact list element), negative/inverted heights.

Purity

convert_scene reads its argument and nothing else — loads no fixture, reads no environment, calls no solver — never mutates its argument, and returns an equal dict for an equal document on every call (tested, including at depth).

Tests

Every expected value in the test module is computed from first principles — metres ÷ 0.3048 written out in the test file, plan geometry with math.hypot — never from the bridge's own constants. Each converted scene is additionally validated through the real root parser (server/scene.py via placement/adapter.load_solver, the code behind POST /v1/placements) and scene.schema.json, including a rotated and translated wall with a nonzero s origin, where the test independently recomputes where the hazard span must land in plan coordinates and checks the parser finds exactly that point from the converted feet.

Refusal expectations were verified case-by-case against the actual behavior (56 refusal cases): e.g. non-wall_rect geometry kinds are refused upstream by the pydantic discriminator union with the union tag in the error location, and an unknown chain wall reference is caught by the source contract's consistency check before the bridge's own chain check.

Verification

  • New tests: 14 passed.
  • Full backend suite: 1248 passed / 17 skipped / 5 failed — the failures are exactly the documented baseline (1 SSR-hygiene overlap test + 4 expired eval-gate tests in server/eval/tests/test_gates.py), identical on base.
  • ruff check and ruff format --check clean across the backend.

Base: stacked on obv/burst6-bs-status-evidence (head of #270), like #271. Draft; not merging.

Add server/placement/scene_bridge.py: a pure convert_scene(document) that
turns the backend base-scene/v0 contract (metres, meter_anchored/v0 frame,
Scene-validated) into the placement wire scene the unchanged top-level
POST /v1/placements server accepts (feet, server/schemas/scene.schema.json).
Lengths convert by dividing by exactly 0.3048; s coordinates are already
measured from the meter in both contracts, so spans convert without
rebasing — and every declared s value (wall s_range_m, chain ends, bounded
object spans, the meter anchor's s=0 centre) is cross-checked against the
plan geometry at a 1e-6 m float-noise bound, so a self-contradictory scene
is refused, never silently accepted.

The initial supported scope is one straight observed house wall, an
observed meter anchor (point or wall_rect) and level ground; everything
outside it raises SceneBridgeError with code placement_scene_unsupported
(valid scene, outside the scope) or placement_scene_invalid (the source
contradicts itself or the schema), each carrying a JSON pointer naming the
offending input. Unmappable provenance (generated/inferred evidence,
unmapped classes, sources, materials) and unrepresentable geometry
(stand-offs, heightfields, sloped ground, grid coverage, keyframes,
generated regions) are refused rather than guessed at; nothing converts to
a default silently.

The new test module computes every expected value from first principles
(metres / 0.3048 written out, plan geometry with math.hypot) and validates
each converted scene through the real root parser (server/scene.py via
placement/adapter.load_solver) and scene.schema.json, including a
rotated/translated wall with a nonzero s origin. Full backend suite:
1248 passed, 17 skipped, 5 failed — the documented baseline (SSR hygiene
+ 4 eval gates) identical on base; ruff check and format clean.

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f89140f7-2710-4bd2-be33-70c824ba8af8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@obvious-autobuild

Copy link
Copy Markdown
Author

Independent review (Obvious, Autobuild + Max)

Verdict: ready — as a draft, nothing blocks ready-for-review. Every load-bearing claim in the body checks out on a fresh checkout of 48de156c; the findings below are low/informational and none of them blocks the conversion path this PR pins.

Findings (ranked)

  1. [low] A ground coverage band without out_m converts to a wire-invalid scene instead of being refused. Band.out_m is optional in the source contract (server/contracts/scene.py, Band), but the wire schema makes out_ft required for band: "ground" (server/schemas/scene.schema.json, the if/then on coverage.observed[]). _coverage/band_out (backend/server/placement/scene_bridge.py:597-604) copies any band through, so a schema-valid backend scene with a ground band lacking out_m passes convert_scene and is then rejected by the consumer's parse_scene (server/scene.py validate_schema) — a downstream schema error, not the bridge's own placement_scene_unsupported with a pointer. This is the one hole I found in an otherwise fail-closed refusal set; it fails loudly, never silently. Smallest fix: in band_out, refuse with _unsupported("a ground coverage band needs out_m", f"{pointer}/out_m") when band.band == "ground" and band.out_m is None.

    Confirmed by running it: convert_scene accepts a schema-valid backend scene carrying {"band": "ground", "s_m": [-2.0, 4.0]} with no out_m, and the wire schema and parse_scene both refuse the result with 'out_ft' is a required property at /coverage/observed/0.

  2. [informational] Two silent drops the docstring doesn't name. coverage.rule (the CoverageRule object: min_views, max_incidence_deg, observed_share) and each coverage.sensitivity[] wrapper's surface / max_incidence_deg / rule_id / max_footprint_m are dropped without a wire counterpart, but the module docstring only documents the band-level views/best_incidence_deg drops. Dropping them is right — the wire cannot carry them — the docstring could just say so, matching its usual "every drop is documented" standard.

  3. [nit] Body says "56 refusal cases"; the tests execute 62. Counted at runtime with a wrapper around convert_scene over the two refusal tests: 38 placement_scene_unsupported + 24 placement_scene_invalid = 62. Conservative direction — the suite covers more than the body claims.

What I verified (commands and results)

Environment: fresh clone at 48de156c (git merge-base HEAD origin/obv/burst6-bs-status-evidence = 715c2438 = base tip, so the head is exactly base + one commit — the stacking claim is true). Backend env via uv sync; uv-invoked per backend/CONTRIBUTING.md.

  • New tests: uv run pytest server/placement/tests/test_scene_bridge.py -q → 14 passed in 1.11s (the body's count: 12 test functions, one ×3-parametrized = 14 items).
  • Placement suite: uv run pytest server/placement/tests -q → 14 passed.
  • Full backend suite (head): uv run pytest -q → 5 failed / 1248 passed / 17 skipped in 268s — exactly the body's numbers. The 5 failures are exactly the documented baseline: tests/test_ssr_overlap_check.py::test_repo_has_no_ssr_shingle_overlap + 4 server/eval/tests/test_gates.py gates.
  • Full backend suite (base 715c2438): uv run pytest -q → 5 failed / 1234 passed / 17 skipped in 285s; the FAILED list is byte-identical to head's (diff of the two lists is empty) — head is base + exactly the 14 new passing tests, and the 5 failures are pre-existing, untouched by this PR.
  • Lint: uv run ruff check . → clean; uv run ruff format --check . → 472 files already formatted.
  • Revert check (tests bind to the change): moved scene_bridge.py out of the tree → test_scene_bridge.py fails at collection (import error); restored.
  • Mutation check, conversion precision: replaced / 0.3048 with a rounded reciprocal multiplier (* 0.328084) → 4 failed (happy path, wall-rect meter, rotated/translated placement, stub scene all catch it). The tests really pin exact division, as the body claims.
  • Mutation check, refusal logic: deleted the s=0-centre check for wall-rect meters → test_invalid_refusals_carry_code_and_pointer failed (the off-centre [0.2, 0.5] case is no longer refused). Restored; tree clean after every mutation.

Cross-checks against the pinned consumer (read, not run): the wire schema's object/wall/meter/ground/overhead/facing/coverage field names and enums match the bridge's output field-for-field (_CLASS targets are exactly the wire type enum; _MATERIAL values exactly the wire ground type enum; End.kind and band enums identical on both sides, so passthrough is safe). parse_scene sets s=0 at the meter's projection onto its wall, which is the semantics the bridge's metre→feet s-passthrough assumes; the wire's "observed spans may extend past the chain's ends" justifies bounded=False for coverage bands while objects/overheads/facing are bounded. The docstring's format-doc citation checks out (backend/docs/scene-and-verdict-format.md §1: "on the meter wall s = x").

What I couldn't verify

  • Nothing exercises the bridge outside its tests yet: no stage publishes placement/scene.json (the adapter still falls back to the reference fixture), so there is no end-to-end production run to inspect — by design, per the body.
  • GitHub-side evidence is label-only (Label PR size = success; Aikido skipped; no backend CI on this repo) — the local runs above are the only test evidence, same as every recent PR here.

Body accuracy otherwise: every numeric claim I could measure matched (14 tests, 1248/17/5 full suite, ruff clean, stacked-on-#270, draft). Refusal-case count is the one mismatch (finding 3).

@SamGu-NRX
SamGu-NRX changed the base branch from obv/burst6-bs-status-evidence to alpha October 16, 2026 04:32
@SamGu-NRX
SamGu-NRX marked this pull request as ready for review October 16, 2026 04:32
@SamGu-NRX
SamGu-NRX merged commit 9c21d72 into alpha Oct 16, 2026
3 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T04:34:32.523801Z 48de156 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codemetrics-ai

Copy link
Copy Markdown

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 BaseScanning to your team's repositories.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 3/5

[Medium impact] Adds a coordinate conversion bridge for placement scene data.

Fix coverage selection and missing ground depths before merging.

Fix All in Claude CodeFindings

  1. P1 Coverage alternatives become extra observations ▶
  2. P1 Ground bands lack required depth ▶
  3. P2 Band counts exceed wire limits ▶
Fix with agent prompt
### Issue 1
backend/server/placement/scene_bridge.py:599-601
`_coverage` combines the main bands with every alternative in `sensitivity`. These alternatives describe coverage under different viewing-angle rules, not extra observations. A ground strip absent under the main rule but present under a looser alternative therefore becomes observed in the output. The placement parser joins these strips, so checks can treat unseen ground as clear.

Select one declared rule, or refuse alternatives the wire scene cannot represent, rather than combining them.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

### Issue 2
backend/server/placement/scene_bridge.py:593-595
The source contract allows a ground band without `out_m`, but the wire schema requires `out_ft` for every ground band. The bridge returns that band without a depth, so `convert_scene` succeeds while the real placement parser rejects its result.

Refuse a ground band without a measured depth using `SceneBridgeError`; do not guess one. Add a test that checks this case against the root parser.

### Issue 3
backend/server/placement/scene_bridge.py:603-604
The source contract puts no size limit on `coverage.bands`, but the wire scene allows at most 500 observed bands. The bridge copies them all without merging or checking the count. A valid source with 501 bands therefore converts successfully but cannot be read by the placement server.

Merge equivalent neighboring bands where possible, or return a pointed `SceneBridgeError` when the output would exceed the limit.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds a pure bridge from metre-based base-scene/v0 documents to the feet-based placement scene, plus tests against the real placement parser.

  • Backend scenes can now be converted into placement scenes in feet.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["base-scene/v0 · metres"] --> B["Scene validation"]
  B --> C["Scope and geometry checks"]
  C --> D["Convert lengths and fields"]
  D --> E["Placement wire scene · feet"]
  E --> F["Root placement parser"]
  B --> G["SceneBridgeError"]
  C --> G
Loading

Reviews (1) · Last reviewed commit: "feat(backend): bridge base-scene/v0 metr..." · Reviewed by Greptile

Comment on lines +599 to +601
for sensitivity_index, sensitivity in enumerate(scene.coverage.sensitivity or []):
for band_index, band in enumerate(sensitivity.bands):
band_out(band, f"/coverage/sensitivity/{sensitivity_index}/bands/{band_index}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Coverage alternatives become extra observations

_coverage combines the main bands with every alternative in sensitivity. These alternatives describe coverage under different viewing-angle rules, not extra observations. A ground strip absent under the main rule but present under a looser alternative therefore becomes observed in the output. The placement parser joins these strips, so checks can treat unseen ground as clear.

Select one declared rule, or refuse alternatives the wire scene cannot represent, rather than combining them.

Knowledge Base Used:

Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/server/placement/scene_bridge.py
Line: 599-601

Comment:
**Coverage alternatives become extra observations**

`_coverage` combines the main bands with every alternative in `sensitivity`. These alternatives describe coverage under different viewing-angle rules, not extra observations. A ground strip absent under the main rule but present under a looser alternative therefore becomes observed in the output. The placement parser joins these strips, so checks can treat unseen ground as clear.

Select one declared rule, or refuse alternatives the wire scene cannot represent, rather than combining them.

**Knowledge Base Used:**
- [Analysis API and schemas](https://app.greptile.com/s-gu/-/custom-context/knowledge-base/samgu-nrx/basescanning/-/docs/analysis-api-and-schemas.md)
- [Site analysis solver](https://app.greptile.com/s-gu/-/custom-context/knowledge-base/samgu-nrx/basescanning/-/docs/site-analysis-solver.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

Comment on lines +593 to +595
if band.out_m is not None:
record["out_ft"] = _ft(band.out_m)
observed.append(record)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Ground bands lack required depth

The source contract allows a ground band without out_m, but the wire schema requires out_ft for every ground band. The bridge returns that band without a depth, so convert_scene succeeds while the real placement parser rejects its result.

Refuse a ground band without a measured depth using SceneBridgeError; do not guess one. Add a test that checks this case against the root parser.

Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/server/placement/scene_bridge.py
Line: 593-595

Comment:
**Ground bands lack required depth**

The source contract allows a ground band without `out_m`, but the wire schema requires `out_ft` for every ground band. The bridge returns that band without a depth, so `convert_scene` succeeds while the real placement parser rejects its result.

Refuse a ground band without a measured depth using `SceneBridgeError`; do not guess one. Add a test that checks this case against the root parser.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

Comment on lines +603 to +604
if observed:
out["observed"] = observed

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Band counts exceed wire limits

The source contract puts no size limit on coverage.bands, but the wire scene allows at most 500 observed bands. The bridge copies them all without merging or checking the count. A valid source with 501 bands therefore converts successfully but cannot be read by the placement server.

Merge equivalent neighboring bands where possible, or return a pointed SceneBridgeError when the output would exceed the limit.

Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/server/placement/scene_bridge.py
Line: 603-604

Comment:
**Band counts exceed wire limits**

The source contract puts no size limit on `coverage.bands`, but the wire scene allows at most 500 observed bands. The bridge copies them all without merging or checking the count. A valid source with 501 bands therefore converts successfully but cannot be read by the placement server.

Merge equivalent neighboring bands where possible, or return a pointed `SceneBridgeError` when the output would exceed the limit.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL 500-999 effective changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant