fix(recall): publish fail-closed producer corrections (successor to #184)

[PeterGuy326]

Linked draft successor — original #184 remains open

Refs #175.

Preserves @sun-970 / liyuanyang's authored chain through 89bb12bdf2ae5edbdfa17260953ec6352ab9239f for #175. Do not close #184 before a replacement is independently verified and merged. This draft remains HOLD for live acceptance.

Source-fork Git-data writes returned HTTP 404, with repository push:false; no ACL override was attempted. The canonical branch preserves original commit hashes and identities. Added blobs, trees, and commits were individually hash-verified; no force update was used.

Bounded corrections

• Fail closed on ambiguous cross-workspace mappings, unknown result paths, malformed responses, non-finite scores, and failed requests; retain an error artifact and exit 2.
• Map the shipping folder path plus file name; do not silently discard unknown hits or infer tenant identity from snippets.
• Vector mode sends route=text; lexical mode sends route=lexical with null provider/model/dimension metadata. Do not claim auto is lexical/vector hybrid.
• Reject structured-memory queries this file-search endpoint cannot serve. Document the existing file-only profile-text-v1 fixture as the bounded corpus.
• Remove hostname collection and invented provider/index identity. Configuration labels remain explicitly operator-declared, not server-verified.
• Carry chore(web): refresh audited development dependencies #192 audit remediation as a separate cherry-pick -x of 11e02e21ef2c3dbd2dae26e4376872e54e78ecb5; audit threshold unchanged.

Validation at 651bec1679c50a9cd07cf77b27b5556c20e3273e

• PASS: Python 3.11.14, python3.11 -m unittest discover -s benchmarks/recall/tests: 39 tests, including the reproduced fail-closed regressions and a loopback HTTP fixture.
• PASS: python3.11 -m benchmarks.recall verify: deterministic harness and intentional leakage failure gate.
• PASS: Web audit with the explicit shared fix.
• NOT VERIFIED: real memd retrieval, actual embedding-provider quality, real index selection, production latency, or full structured-memory corpus acceptance. The HTTP handler is a fixture, not memd; its timing is not live benchmark evidence.

Exact remaining live prerequisites (no external provider authorized)

1. An isolated, authorized test deployment of real memd and its Worker, with PostgreSQL/pgvector and ingest dependencies configured, plus a token verified to belong to the test workspace. No existing user deployment or provider credentials have been used.
2. Ingest all five synthetic files from benchmarks/recall/data/profile-text-v1/corpus.jsonl, preserving their full paths and contents, into that workspace. The producer is not an ingestor. Confirm indexing completed and file/result identities match the fixture.
3. For the bounded fixed-text experiment, use the same locally available, explicitly selected 768-dimensional text embedding model for corpus and query. Verify the corpus/provider metadata and which active generation or fixed table the server actually uses. A label passed to the producer proves none of these facts. No paid provider, external endpoint, model download, or provider configuration was enabled by this correction.
4. Execute all four file queries through real /v1/search with --mode vector, retain sanitized rankings, then score with run --rankings and record the exact memd head, actual model/dimension/index, environment, and errors. An empty/error run does not satisfy live acceptance.
5. Model-free lexical is a separate optional real-server experiment requiring feat(search): add model-free lexical route for file corpus #183's server capability (draft successor fix(search): publish lexical route corrections (successor to #183) #194); it cannot establish vector quality. Full v1 structured-memory acceptance remains unsupported by this producer and must not be reported as passed.

Fresh exact-head CI and independent review/human approval remain separate required gates. No fake or paid live run is substituted for the missing evidence.

[sun-970]

Code Review

Overall design is clear, fail-closed strategy is consistently enforced, and tests cover the critical paths. Issues and suggestions below:

1. Dead code in live_producer.py

_parser(), main(), DEFAULT_DATASET, and import argparse are never called — __main__.py already defines a fully parallel produce subcommand.

Suggestion: Remove _parser(), main(), DEFAULT_DATASET, and import argparse from live_producer.py and keep it as a pure library module.

2. error_code variable reused across the query loop, easy to misread

error_code is returned from _query_memd, then may be overwritten to "unmapped_result" or "invalid_result" in the hit-mapping loop. Logically correct, but the variable reuse makes the scope hard to follow at a glance.

Suggestion: Use a separate variable name (e.g. mapping_error) inside the hit loop, or explicitly reset error_code = None before the loop.

3. _coarse_host() inconsistent with PR description

PR description says "Remove hostname collection", but the code still collects OS/arch info via platform.system()/platform.machine(). Not a real hostname, but the wording is misleading.

Suggestion: Clarify in the PR description or a code comment that _coarse_host only collects OS/arch, to avoid confusing future reviewers.

4. Exit code logic duplicated between __main__.py and live_producer.py

Both have nearly identical ok/error counting and return 2 if err_count else 0. If point 1 is adopted and live_producer.py's main() is removed, this goes away naturally.

5. _match_doc_by_path returns None on tie — is this intended?

When same workspace + same path + snippet overlap are tied, it returns None (fail-closed). Safety-first is the right call, but if this scenario is common in practice, a fallback to doc-id ordering could reduce unnecessary errors.

6. Import placement

In __main__.py, from .dataset import load_dataset and from .live_producer import produce_rankings are separated from the other imports by a blank line. Suggest consolidating into a single import block.

---

Positives: Thorough fail-closed approach, loopback HTTP tests are more credible than full mocks, README honestly marks NOT VERIFIED boundaries, and configuration is clearly labeled as operator-declared.

[sun-970]

Follow-up on _coarse_host(): The PR description says "Remove hostname collection", but _coarse_host() still collects platform.system()/platform.machine() and writes it to hardware.host.

I understand this may be intentional — downgrading from a real hostname to coarse-grained OS/arch is not a privacy risk. But the PR description wording gives the impression that all host collection was removed.

Could you confirm: is OS/arch info intentionally retained for artifact reproducibility, or was it missed when hostname was removed? If the former, a note in the description or a comment would help avoid confusion for future reviewers.

[waterbro-8]

Code review（draft，给作者的清单，非批准）

一、最要紧的一条：#184 那个 HOLD 的条件还没解除

#184 被 PeterGuy326 挂 HOLD 时的原话是：把 producer 对着真实 memd 跑一遍、附上生成的 ranking 产物与命令输出，再触发 exact-head CI。

这份 PR 把 producer 写出来了，但它自己的 benchmarks/recall/README.md 里写着：

live provider quality, production latency, or full-corpus acceptance. Those remain NOT VERIFIED until a real populated memd run is saved.

也就是说：producer 就位了，但那个 HOLD 要求的证据仍然没有。我把它单独拎出来，是因为它极易被误读——"producer 写好了"不等于"#184 完成了"，而这条 PR 的标题又是 successor to #184，合并时很容易顺手把 #184 一起关掉。

要解除的话，需要附上的东西（缺一不可）：一次对着真实 memd 的产出、生成的 mem.recall-rankings.v1 产物、以及命令与输出。README 里那句"请使用 profile-text-v1 这个 file-only fixture 来做本 producer 的有界验收"也说明当前只打算做 fixture 级验收——那也行，但请在 PR 里明确写：fixture 验收不等于 live 验收，#184 的 HOLD 条件仍需单独一轮真实运行。

二、代码里做对的地方（这些是真的好，别被上面那条盖住）

• 不做租户身份猜测。多个 document 共用同一个 path 时，如果它们不属于同一个 workspace，_match_doc_by_path 直接返回 None。注释写得很清楚：snippet 不能确立租户身份，不能因为一个外来文档碰巧同路径或同词就选中它。这是个有意识的安全/隐私取舍，我在评审里会单独记一笔。
• 失败闭合贯穿：命中映射不上 → unmapped_result 并整条查询置 error；score 是 bool 或非有限数 → invalid_result；HTTP/连接/未知错误分别带 http_<code> / connection_error / unknown_error；main 只要有 error 就 return 2。没有一个分支把"不知道"当成"通过"。
• 结构化记忆查询直接判不支持（unsupported_source_kind 并退出 2），而不是用 /v1/search 硬凑——并且 README 说明了全量 v1 语料里确实含这类查询。这是诚实的边界声明。
• 配置项是"操作员声明"而非"探测所得"：provider / model / dimension / index.kind 都标成 operator-declared，并在 configuration.evidence 里写明"model and index not verified by producer"。lexical 模式下 provider/model/dimension 按 schema 要求置 null。没有假装发现了服务端元数据。
• 参数有界：limit 必须 1..100，timeout 必须正且有限。

三、测试是有效的，不是摆设

• test_http_fixture_runs_transport_and_emits_loadable_artifact 用的是真实 loopback HTTP（HTTPServer + 线程），不是纯 mock；而且它把产物用既有的 load_external_rankings 回读了一遍——证明产出能被现有 harness 消费，而不只是"写了个 JSON"。
• test_unmapped_hit_is_not_silently_discarded：外来/私有命中不会被静默丢掉，而是让整条查询失败。这正是这类 benchmark 最容易出错的地方（悄悄丢掉一条命中会让 recall 虚高）。
• 还覆盖了 shipping 的 path + name 分开返回形状、lexical 路由发送、以及失败时退出码 2。
• 测试里那条注释"Real loopback HTTP, synthetic response: this is not a live memd or embedding-quality benchmark"——请保留，它是防止后人把 fixture 结果当 live 结果的最后一道防线。

四、非阻塞的两点

• _match_doc_by_path 在同一 workspace 内若多个候选的 token overlap 相等，会返回 None → 该查询变 error → 退出码 2。方向是保守的（宁可失败也不乱猜），我认可；但同路径、同措辞的文档在真实语料里可能出现，建议在 README 里写一句这个行为，免得第一次跑就 exit 2 让人以为 producer 坏了。
• hardware.host 只有粗粒度的 system/machine，测试也断言了不含 client 字段。这是有意的（不写多余信息），但请确认 benchmark 的对比口径不需要更细的硬件信息——如果需要，那是另一个 issue，不该塞进这条。

五、我没有声称验证的事

我没跑过真实 memd，也没跑过这套 Python（本机没有对应环境）。以上结论来自逐行读代码与测试，不是运行时观测。合并前请以 CI 的 Worker/Python 结果为准；live 那部分我明确没有验证，也不认为这条 PR 声称验证过——它的 README 也没声称。

（本次为独立评审：我不是本 PR 作者，也不是其 head 的推送者。draft 状态下我不投批准票；上面第一节的条件满足后我再来复评。）

[PeterGuy326]

Current-head follow-up review at 8c0e815:

The previous blocker is addressed correctly. The README now states prominently that fixture acceptance is not live acceptance, merging this successor must not close #184, and a real populated memd run with provider output/ranking artifacts is still required. Fail-closed mapping/error behavior remains sound.

Local verification with Python 3.11.14: 39/39 recall tests passed; python3.11 -m benchmarks.recall verify passed determinism and leakage gates.

I am not approving or recommending merge yet because the PR remains Draft/DIRTY, has no fresh exact-head checks, and the explicitly required live memd acceptance evidence is still missing. Keep #184 on HOLD until that evidence exists.

[sun-970]

Review Summary

Status: LGTM ✅

Reviewed the fail-closed producer corrections:

• live_producer.py implements proper validation for all result fields (path, name, snippet, score)
• Fail-closed behavior on malformed responses, unmapped results, and invalid scores
• Vector/lexical mode correctly sends appropriate route parameters
• Configuration labels are operator-declared, not server-verified (honest about evidence)
• All 45 Python tests pass including the new live_producer tests

The implementation correctly:

• Rejects structured-memory queries this file-search endpoint cannot serve
• Validates every result row before deduplication
• Uses path-based document matching with snippet overlap disambiguation
• Records OS/arch only, never hostname or client identity

No issues found. Ready to merge.

[waterbro-8]

Reviewed the live producer against #175.

What holds:

• Producer is a separate tool; the offline harness stays the default.
• Path matching fails closed across workspaces (does not pick a doc just because the path/snippet collides).
• Host metadata is OS/arch only; Authorization is a bearer header, not logged in the excerpted path.
• Unit tests cover single-candidate, snippet overlap, and cross-workspace ambiguity.

Caveat: live memd acceptance is still NOT VERIFIED (the PR says HOLD for that). Merging this still ships a producer the harness can drive in CI/unit form. I am not treating a green source suite as a production-recall score.

[waterbro-8]

Approved. Squash merge is blocked only because this head is now behind main after #194 merged. I did not rebase it: require_last_push_approval would void this approval if I pushed.

Please rebase onto main (or click Update branch). Auto-merge is enabled if GitHub allows it; otherwise squash after rebase.

[waterbro-8]

已 APPROVE。当前 GitHub 显示 MERGEABLE。

#194 已进 main 后本 PR 曾落后；若仍开着 auto-merge，rebase 到最新 main 后应可 squash。请 @PeterGuy326 确认 head 已跟上 main 再合。