Skip to content

test(log): cover search result summary redaction - #2223

Open
WeiminLee wants to merge 1 commit into
dev-v2.0.29from
fix/2103-search-summary
Open

test(log): cover search result summary redaction#2223
WeiminLee wants to merge 1 commit into
dev-v2.0.29from
fix/2103-search-summary

Conversation

@WeiminLee

Copy link
Copy Markdown
Collaborator

Summary

Refs #2103.
Supersedes #2110.

Verification

  • make format: failed because poetry is not installed in this environment
  • uv run --with ruff ruff check --fix tests/test_log.py: All checks passed
  • uv run --with ruff ruff format tests/test_log.py: 1 file left unchanged
  • uv run --with pytest pytest tests/test_log.py -q: 8 passed, 1 warning

@Memtensor-AI Memtensor-AI added area:core MOS 编排层 / 框架底座 / 跨模块问题 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 5, 2026
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2223
Task: d401551f78cdcd5e
Base: dev-v2.0.29
Head: fix/2103-search-summary

🔍 OpenCodeReview found 1 issue(s) in this PR.


1. tests/test_log.py (L161-L163)

The assertions assert "embedding" not in rendered, assert "0.123456" not in rendered, and assert "do-not-log" not in rendered are vacuously true and provide no meaningful test coverage. The summarize_search_results function returns a fixed-shape dict ({"bucket_counts": ..., "item_counts": ..., "total_items": ...}) that never contains the input keys or values, so these assertions would pass even if the function were completely broken and leaked payload. To actually validate that private payload is excluded from the logged output, the test should assert on the absence of the raw input dict's string representation, or use a mock/spy to verify the argument passed to the logger. Alternatively, if a buggy implementation were introduced that passed results directly to the logger instead of log.summarize_search_results(results), the assertion assert "private memory payload" not in rendered and assert "private preference note" not in rendered would catch it, but the "embedding", "0.123456", and "do-not-log" assertions add nothing on top of those.

💡 Suggested Change

Before:

    assert "embedding" not in rendered
    assert "0.123456" not in rendered
    assert "do-not-log" not in rendered

After:

    # These values are private inputs that must not appear in the logged summary.
    # The assertions below already cover the key privacy guarantees:
    assert "private memory payload" not in rendered
    assert "private preference note" not in rendered
    # Note: "embedding", "0.123456", "do-not-log" would never appear in the summary
    # dict regardless of input, so those assertions are redundant and not meaningful.

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (8/8 executed). memos_python_core/changed-repo-python: 8/8. Duration: 5s [advisory, non-gating] AI-generated tests on branch test/auto-gen-d401551f78cdcd5e-20260805163841: 43/43 passed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/2103-search-summary

@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core MOS 编排层 / 框架底座 / 跨模块问题 status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants