fix(notion-search): drop unused _flatten_property arg + lock falsy-value behavior
Code review polish: - Drop the unused `name` parameter from `_flatten_property` (it was reserved for future per-property-name special-casing but never used). - Add a regression test pinning that checkbox=False and number=0 are preserved in the enriched output. The existing empty-value filter is `if value not in (None, [], "")` which keeps falsy-but-meaningful values, but the contract wasn't tested. 22/22 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -154,7 +154,7 @@ TEXT_BEARING_BLOCK_TYPES = {"paragraph", "heading_1", "heading_2", "heading_3",
|
|||||||
"numbered_list_item", "to_do", "toggle"}
|
"numbered_list_item", "to_do", "toggle"}
|
||||||
|
|
||||||
|
|
||||||
def _flatten_property(name: str, prop: Dict):
|
def _flatten_property(prop: Dict):
|
||||||
"""Flatten a Notion property to a Python value suitable for display/rerank."""
|
"""Flatten a Notion property to a Python value suitable for display/rerank."""
|
||||||
ptype = prop.get("type")
|
ptype = prop.get("type")
|
||||||
if ptype == "title":
|
if ptype == "title":
|
||||||
@@ -224,7 +224,7 @@ def enrich_candidates(notion, candidates: List[Dict]) -> List[Dict]:
|
|||||||
for name, prop in properties.items():
|
for name, prop in properties.items():
|
||||||
if prop.get("type") == "title":
|
if prop.get("type") == "title":
|
||||||
continue
|
continue
|
||||||
value = _flatten_property(name, prop)
|
value = _flatten_property(prop)
|
||||||
if value not in (None, [], ""):
|
if value not in (None, [], ""):
|
||||||
flat_props[name] = value
|
flat_props[name] = value
|
||||||
excerpt = _fetch_excerpt(notion, c["id"])
|
excerpt = _fetch_excerpt(notion, c["id"])
|
||||||
|
|||||||
@@ -360,6 +360,29 @@ def test_enrich_truncates_long_excerpt():
|
|||||||
_assert(len(enriched[0]["excerpt"]) == 200, "excerpt truncated to 200 chars")
|
_assert(len(enriched[0]["excerpt"]) == 200, "excerpt truncated to 200 chars")
|
||||||
|
|
||||||
|
|
||||||
|
def test_enrich_keeps_falsy_but_meaningful_values():
|
||||||
|
"""checkbox=False and number=0 are meaningful, not 'empty' — must survive the filter."""
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
import notion_search
|
||||||
|
|
||||||
|
notion = MagicMock()
|
||||||
|
notion.blocks.children.list.return_value = {"results": []}
|
||||||
|
|
||||||
|
candidate = {
|
||||||
|
"id": "page-a", "url": "https://notion.so/a",
|
||||||
|
"properties": {
|
||||||
|
"Name": {"type": "title", "title": [{"plain_text": "Test"}]},
|
||||||
|
"Active": {"type": "checkbox", "checkbox": False},
|
||||||
|
"Score": {"type": "number", "number": 0},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
enriched = notion_search.enrich_candidates(notion, [candidate])
|
||||||
|
_assert(enriched[0]["properties"].get("Active") is False,
|
||||||
|
"checkbox=False kept (not filtered as 'empty')")
|
||||||
|
_assert(enriched[0]["properties"].get("Score") == 0,
|
||||||
|
"number=0 kept (not filtered as 'empty')")
|
||||||
|
|
||||||
|
|
||||||
def run_all():
|
def run_all():
|
||||||
tests = [
|
tests = [
|
||||||
test_call_claude_dispatches_to_sdk_when_available,
|
test_call_claude_dispatches_to_sdk_when_available,
|
||||||
@@ -383,6 +406,7 @@ def run_all():
|
|||||||
test_enrich_extracts_first_paragraph_excerpt,
|
test_enrich_extracts_first_paragraph_excerpt,
|
||||||
test_enrich_falls_back_to_empty_excerpt,
|
test_enrich_falls_back_to_empty_excerpt,
|
||||||
test_enrich_truncates_long_excerpt,
|
test_enrich_truncates_long_excerpt,
|
||||||
|
test_enrich_keeps_falsy_but_meaningful_values,
|
||||||
]
|
]
|
||||||
for t in tests:
|
for t in tests:
|
||||||
print(f"\n{t.__name__}")
|
print(f"\n{t.__name__}")
|
||||||
|
|||||||
Reference in New Issue
Block a user