TASK 18 - Deduplicate the responder factory¶
Partly superseded by TASK 19
The "Behaviour change: none" section below reasons at length about
near_threshold. TASK 19 then removed the near-relation subsystem
outright, so near_threshold, DEFAULT_NEAR_THRESHOLD and the
test_log_config_omits_near_threshold test referenced here no longer
exist. The reasoning is what led to that removal; everything else in this
report still describes the current code.
Purpose¶
Collapse the copy-pasted perception-stack construction in _build_responder
into one code path, and give the env-var parsing its first test coverage.
Problem (before)¶
The perception and scene_gemini branches of
src/xiao_hei_vln/app/main.py each built the same perception stack from
scratch — 52 identical lines, including two copies of:
- the same eight lazy imports,
- reads of the same nine env vars (
XIAO_HEI_PERCEPTION_*,XIAO_HEI_SCAN_*,XIAO_HEI_OBJECT_MAP), each with its default repeated, HTTPPerceptionClient+wait_until_ready(),PointLifter,ScanAccumulator,Vocabulary, and the conditionalObjectMap,- the
PerceptionResponderconstruction itself.
The failure mode this invites is silent: a default tuned in one branch and not the other gives two responders that disagree about how perception behaves, with nothing failing to signal it. The branches had already drifted — see below.
Compounding it, _build_responder has never had a test. It blocks on the
sidecar's /healthz and issues a live Gemini warmup(), so it isn't
unit-testable as written, and the env parsing — the part most likely to carry a
config bug — was the least protected code in the file.
What changed¶
Two new module-level pieces in app/main.py:
_PerceptionSettings— a frozen dataclass withfrom_env()andas_log_config(). Pure: no I/O, no network, so it is directly testable. The perception-package imports stay lazy insidefrom_env(), preserving the property that the package imports without theperceptionextra installed._build_perception_responder(scene, settings, ...)— does the wiring, parameterised by the only three things that actually differed between the branches:trajectory_path,take_waypoint_reached_signals,logger.
Result:
| before | after | |
|---|---|---|
_build_responder |
222 lines | 75 lines |
| env vars read in both branches | 10 | 1 (XIAO_HEI_VLM_LOG_DIR) |
| tests covering this code | 0 | 20 |
app/main.py net: 676 → 647 lines, despite the added helper and docstrings.
Behaviour change: none¶
session.json comes out byte-identical to before for both responders.
An earlier revision of this refactor got that wrong, and the reason is worth
recording. The two logger configs look like they had drifted — the
perception branch recorded near_threshold_m, scene_gemini did not — so
the first attempt unified them and added the key to both. That was incorrect:
near_threshold is consumed in exactly one place,
PerceptionResponder._compute_output (the Phase-B answer path). But
scene_gemini never calls respond() on its perception responder — it drives
it purely through ingest() and answers via Gemini (TASK 14). The threshold
therefore has no effect whatsoever in the submission path, and the scene
graph handed to Gemini carries no derived near edges at all.
So the asymmetry was right, and unifying it would have documented a setting
that does nothing. as_log_config() now carries only the four knobs that apply
to any responder building the perception stack; the perception branch adds
near_threshold_m and trajectory_json itself, both being Phase-B-only.
tests/test_perception_settings.py::test_log_config_omits_near_threshold pins
the reasoning so it does not get "fixed" again.
The general lesson: an asymmetry between two copies is not automatically drift. Here it encoded a real difference in which code path each responder runs.
Verification¶
uv run pytest -q→ 427 passed, 1 skipped (407 + 20 new). No existing test changed.- New
tests/test_perception_settings.pyasserts defaults against the imported constants (DEFAULT_NEAR_THRESHOLDetc.) rather than literals, so bumping a default in the perception package cannot silently desync the factory. Also covers numeric coercion (env gives strings; these feed arithmetic), all seven truthy spellings ofXIAO_HEI_OBJECT_MAP, the off-by-default rule for unrecognised values, and thatas_log_config()stays JSON-serialisable and carries no key/token field. - Wiring confirmed end-to-end with
wait_until_readypatched: every knob (near_threshold,score_threshold, liftermin_inliers, scanmax_keyframes,ObjectMapenablement) reaches its destination object, and the/healthzblock is still issued. ruff checkonapp/main.py: identical 4 findings before and after (line numbers shifted only) — all pre-existing (E501×3,SIM115), none introduced. New test file is clean.
Not done¶
_build_responderstill isn't testable as a whole — thescene_geminibranch'sGeminiEngine.warmup()and the sidecar/healthzblock are real I/O in a factory function. Injecting those would make the whole factory unit-testable; the pure part is now carved out and covered._PerceptionSettingslives inapp/main.py. Moving it toxiao_hei_vln/perception/settings.pywould put the env schema next to the defaults it reads, at the cost of the perception package owning its own configuration surface. Left as-is; the app is the only consumer.