ModelSaar

Independent review findings F01–F18

Rendered from docs/review-2026-10-11-findings.md at commit b9d69f7299ed. The repository file is authoritative.

The review grouped its confirmed findings under eight headings (privacy, dispatch contract, resources, embeddings, evidence, evaluator, first run/files, verification). This table keeps the reviewer's 18 grouped findings with stable ids, in the order the review stated them, and maps each to the implementation (branch hardening/review-2026-10-11), the regression test that asserts the corrected behaviour, and the evidence that it was executed.

Regression tests live in tests/test_hardening_2026_10_11.py (28 tests) unless noted. They assert the corrected behaviour. scripts/reproduce_review_baseline.sh is the diagnostic that runs the same tests against 1122e1f: 26 of 28 fail there — 22 on the behavioural assertion (the reviewed defect) and 4 because they use a config field or symbol the fix introduced (memory_margin_gb/unknown_memory_policy, identity, EVIDENCE_SCHEME; each has a sibling test that fails on the behavioural assertion). The two that pass are positive controls — replicas sharing an alias, evidence surviving load/unload — i.e. behaviour the fix had to preserve. The only adaptation needed to run them at the baseline is copying in the branch's fakebackend.py/conftest.py (new recorder and failure-mode hooks); the baseline's gateway code is untouched. Per-test outcomes with the failing assertion: docs/live-runs/2026-10-11-baseline/ (website branch).

Evidence keys: U = python -m pytest on the branch (see docs/verification-report.md for the exact revision and totals of each run); B = baseline reproduction above; L = live run on the GX10 (docs/live-runs/2026-10-11-gx10-hardening/); M = Mac test run (sandbox-exec; mac-tests-<commit>.txt, see verification report); CI = GitHub Actions on the PR revision — a CI definition exists for every item; a CI run is recorded here only once the branch has been pushed and the run has completed, which has not happened yet.

Reopened by the second independent review (2026-10-11, of 80276bb) and re-closed on 2026-10-12. The second review kept its own identifiers (R1–R8, REVIEW.txt; H01–H05, HANDOFF.txt); its tests are in tests/independent_review_2026_10_11/ verbatim. The table marks the F entries whose coverage it found incomplete as fixed; reopened by Rn, re-closed 2026-10-12 — "all fixed" at 80276bb was not true for those entries. A third-party verification of the corrected revision is pending.

id finding (as confirmed by the review) disposition implementation regression test(s) evidence
F01 _exact_counts ran before hard eligibility and sent the whole message to every deployment with /tokenize, including a remote one under local_only fixed two-pass eligibility; tokenizer counts only for policy-allowed deployments, one deadline, label tokenize:<backend>+template-estimate (routing/router.py, routing/eligibility.py, config.py: exact_count_deadline_s) test_local_only_sends_no_message_text_to_remote_tokenizer, test_exact_counts_only_for_policy_allowed_and_labelled_as_estimate U, B
F02 audio_url/video_url parts bypassed fetch_remote_media=false and reached incapable backends; nested content loosely typed fixed typed content parts; unknown types/keys → 400 unsupported_content_type before dispatch (protocol/openai.py) test_unsupported_content_part_types_rejected_before_any_dispatch U, B, L (client-checks.json: unsupported_part_400)
F03 tool-result turn replayed on another candidate after HTTP 500 (also applies to reasoning continuation and pinned sessions) fixed Decision.failover_allowed/failover_note; no pre-commit failover when bound (routing/router.py, server.py) test_no_failover_for_tool_result_turn, test_no_failover_for_pinned_session U, B, L (tool_result_continuation: failover_allowed=false)
F04 a timeout released the reservation while the backend kept computing; classifier timeout did the same fixed; reopened by R1/R4 (streaming read timeout, probe timeout and embedding call still released the slot; embeddings had no deadline), re-closed 2026-10-12 AdapterError(kind="timeout") → mark_uncertain(); streaming read timeout typed; classifier timeout holds its slot (server.py, adapters/base.py, routing/classifier.py, routing/router.py) test_timeout_holds_slot_uncertain_then_recovers (one-slot pool, 300 ms timeout, hold expiry, recovery) U, B
F05 HTTP 200 headers reset the circuit, so four truncated streams left failures=1 fixed; reopened by R4 (a stream of malformed data frames + [DONE] still counted as a validated completion and pinned the session), re-closed 2026-10-12 circuit success only after validated completion (non-stream envelope / completed stream) (server.py) test_circuit_opens_on_truncated_streams U, B
F06 response_model_field=deployment named the failed model after failover fixed attribution computed per attempt (server.py) test_response_model_follows_failover_target U, B, L (client-checks.json: model = requested alias in body and in every SSE frame)
F07 upstream 200 HTML became downstream 200 with an error-shaped body and outcome=ok fixed envelope validation + text/event-stream check pre-commit → 502 upstream_bad_response, no pin, no circuit success (server.py) test_html_200_is_a_typed_502_and_never_pins U, B
F08 memory_gb=1 dispatched a 20.7 GB deployment; memory setting unused fixed; reopened by R2 (completed dispatch dropped the model's weights from the count until the next poll; reject ignored unknown residents), re-closed 2026-10-12 (budget, not OOM guarantee) pool memory budget enforced atomically with the slot: resident + distinct reserved + candidate (+classifier) + memory_margin_gb ≤ memory_gb; weights-only estimates with provenance; unknown_memory_policy; /admin/status shows holders (routing/admission.py, config.py, routing/router.py) test_pool_memory_budget_is_enforced_before_dispatch, test_pool_memory_budget_counts_distinct_reservations_and_unknown_policy, test_unknown_memory_estimate_policy U, B (the live config sets no memory_gb, so L does not exercise it)
F09 modelsaar probe ran inference through a separate Router while the gateway held a one-slot pool fixed POST /admin/probe under gateway admission; CLI uses it when a gateway listens, refuses direct probing next to a listener without --allow-concurrent; offline probes reserve through their own admission; live eval refuses to start next to a listener (server.py, cli.py, evaluation/harness.py) test_admin_probe_runs_under_gateway_admission, test_cli_probe_coordinates_through_running_gateway U, B, L (23-admin-probe-via-gateway.txt)
F10 one alias interchanged two distinct models with equal dimensions as residency changed; "same revision" comment unimplemented fixed; reopened by R3 (an alias whose first member had an unknown identity was anchored only after a response, so concurrent first use could return two spaces), re-closed 2026-10-12 deployment identity (Ollama digest / declared identity); alias members must share the effective identity; unknown identity is never a replica; changed identity requires embeddings[].identity after re-indexing; prefixes rejected (registry.py, adapters, routing/router.py, config.py) test_alias_never_interchanges_distinct_models_with_equal_dimensions, test_known_identical_replicas_share_an_alias (positive control), test_unknown_identity_is_not_a_verified_replica U, B (fixture backend; no real embedding model was pulled — NOT RUN live)
F11 8-d contract returned 200 for 4-d vectors; prefix/normalization declared but unenforced fixed request dimensions/encoding_format guarded (400 unsupported_contract_option); output count/indices/dims/finite/unit-norm validated (502 embedding_contract_violation) (server.py, routing/router.py) test_wrong_output_shape_is_a_contract_error, test_request_level_contract_changes_rejected U, B (fixture; NOT RUN live)
F12 changing a generic backend URL under the same id/model preserved the fingerprint; a replacement inherited a stored tools probe fixed fingerprint = backend + endpoint + model + digest + identity + runtime(+version) + template + quant + pool; unknown inputs visible; evidence scheme 2 with migration (old rows kept, unused, doctor reports); measurements carry conditions; load/unload still not an identity change (registry.py, adapters, store.py, probes.py, doctor.py) test_endpoint_change_invalidates_stored_probe, test_evidence_scheme_versioning_invalidates_old_rows, test_load_unload_keeps_evidence (preserved behaviour) U, B; scripts/install-check.sh (scheme-1 DB migration PASS)
F13 scorer defects: code-05 trailing space; print-then-raise passed code-04; tool call without id/required arg scored; 16 satisfied 6 by substring fixed; reopened by R7 (flushed expected output + public marker then os._exit(0) scored as success; tool arguments were not checked against the declared schema), re-closed 2026-10-12 symmetric whitespace rule, completion sentinel + exit 0, strict tool_call (id, name, JSON-object args, required keys, args_contains), typed number with tolerance; positive/negative controls per scorer (evaluation/scoring.py, fixtures/README.md) tests/test_evaluation.py: test_code05_gold_rle_passes_with_trailing_space_output, test_code_exec_print_then_raise_fails, test_code_exec_print_then_exit_zero_fails, test_tool_call_without_id_fails, test_tool_call_missing_required_argument_fails, test_number_rejects_superstring_match, … (tests/test_evaluation.py, 42 tests incl. sandbox, selector and checkpoint tests) U, M (code-exec controls executed under sandbox-exec), L (scorer 2026.10.11-2 used for the live numbers)
F14 live evaluator executed generated Python with ordinary host authority (wrote outside its temp dir) fixed; reopened by R7 (bwrap bound the whole host root read-only and the macOS profile allowed host reads, contrary to the no-host-authority wording), re-closed 2026-10-12 (sandbox-or-skip) bubblewrap / sandbox-exec / Docker (--network none --read-only) with CPU/memory/output/time limits; no backend → task skipped and denominators reported; no unsafe opt-in (evaluation/sandbox.py) test_sandbox_run_refuses_without_backend, test_sandbox_has_no_unsafe_opt_in, test_sandbox_blocks_writes_outside_work, test_sandbox_blocks_network, test_sandbox_reference_solution_passes, test_sandbox_timeout_fails_cleanly, test_code_exec_skipped_without_backend U (4 isolation tests skipped in the cloud container: no backend), M (executed with sandbox-exec), L (sandbox: sandbox-exec, 0 skipped)
F15 --deployments bare names entered the fixed sweep but excluded all auto candidates fixed selectors resolved once to canonical ids; unknown/ambiguous rejected early; same candidate set for calibration and every policy (evaluation/harness.py: resolve_selectors) test_resolve_selectors_bare_and_full, test_resolve_selectors_unknown_and_ambiguous, test_run_live_resolves_bare_selector_everywhere, test_run_live_rejects_unknown_selector_early U, L (bare names qwen3.6:latest gemma4:31b qwen3-coder:latest resolved; auto restricted to the same three)
F16 classifier output not validated against its closed schema (extra fields, non-numeric confidence, trailing content) fixed parse_classifier_output strict schema, deterministic fallback, confidence kept distinct from quality (routing/classifier.py) tests/test_classifier_schema.py (27 parametrised cases), test_classifier_requires_local_deployment_under_local_only U, B; classifier disabled in L (not exercised live)
F17 client-config --apply changed a 0600 secret-bearing file to 0644; uninstall recursively deleted arbitrary contents under keys//state/ fixed mode-preserving apply, private temp files (O_EXCL), unique backups; uninstall removes only owned files, refuses symlinked boundaries, leaves the rest (clientconfig/__init__.py, cli.py) test_apply_patch_preserves_mode_private_tmp_unique_backups, test_uninstall_removes_only_owned_files_and_refuses_symlinks, tests/test_review_regressions.py::test_uninstall_preserves_foreign_files (refined from the earlier "refuse" behaviour) U, B
F18 fresh Compose-equivalent entrypoint exited for missing keys; init --profile fast left auto on quality fixed; reopened by R5 (the first-use sequence shown after the installer failed — init refuses an existing config; modelsaar test sent the key to any 200 listener), re-closed 2026-10-12 modelsaar keys ensure (idempotent, never rotates, touches only key files) + container CMD; scripts/compose-smoke.sh (empty volume, read-only config, non-root, keys unchanged across restart) in CI; init --profile sets aliases.auto.profile (cli.py, Dockerfile, .github/workflows/ci.yml) test_keys_ensure_is_idempotent_and_serve_points_to_it, test_init_profile_applies_to_auto_alias U, B; compose smoke: CI only (no Docker daemon in the build container) — see verification report for the CI run

Review §8 — reproducible live evidence (not a numbered finding)

requirement disposition where
missing eval-live.json; interrupted console log tracked as if a result fixed: .gitignore /eval-*.json; both 0.1.0 artefacts tracked and labelled docs/live-runs/2026-10-11-gx10/README.md
resumable one-command verification with sanitised records scripts/live_verify.sh (+ scripts/live_client_checks.py, scripts/eval_aggregate.py); --checkpoint/--resume run folder MANIFEST.md lists commands, revision, config, identities, settings, failures
real clients, real-model embeddings, vLLM/llama.cpp, matched reasoning budget NOT RUN, labelled with exact commands docs/verification-report.md

Deferred / not done in this pass