From 67e7fe0c2fb872079b3de32cdac44d96bdbc049d Mon Sep 17 00:00:00 2001 From: jaylfc Date: Mon, 17 Aug 2026 21:46:01 +0000 Subject: [PATCH] Fix unguarded coercion on search request path: as_of and review_by - Move timestamp definition before _format_hit if-block to fix UnboundLocalError - Defensive coercion: as_of uses try/except (TypeError, ValueError) with logger.warning so ISO-8601 timestamps from caller metadata degrade to 0.0 instead of raising - review_by comparison gated on isinstance(review_by, str) to prevent TypeError when non-string metadata arrives through ingest_batch --- ...rp6s-unboundlocalerror-and-coercion-fix.md | 4 ++ taosmd/api.py | 54 +++++++++++++++++-- tests/test_api.py | 2 +- 3 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 changelog.d/tsk-22rp6s-unboundlocalerror-and-coercion-fix.md diff --git a/changelog.d/tsk-22rp6s-unboundlocalerror-and-coercion-fix.md b/changelog.d/tsk-22rp6s-unboundlocalerror-and-coercion-fix.md new file mode 100644 index 00000000..f9a149d3 --- /dev/null +++ b/changelog.d/tsk-22rp6s-unboundlocalerror-and-coercion-fix.md @@ -0,0 +1,4 @@ +### Fixed +- Fixed `UnboundLocalError` in `_format_hit` when `timestamp` was referenced before definition +- Defensive coercion: `as_of` now uses `try/except (TypeError, ValueError)` with `logger.warning` instead of unguarded `float()`, so ISO-8601 timestamps from caller metadata degrade to `0.0` rather than raising +- `review_by` comparison gated on `isinstance(review_by, str)` to prevent `TypeError` when non-string metadata arrives through `ingest_batch` \ No newline at end of file diff --git a/taosmd/api.py b/taosmd/api.py index 70334389..f59fe2ec 100644 --- a/taosmd/api.py +++ b/taosmd/api.py @@ -449,12 +449,16 @@ def _format_hit(hit: dict) -> dict: user_md = dict(user_md) # copy: never mutate the stored row metadata for key, value in preserved.items(): user_md.setdefault(key, value) + # Warn (not assert) if critical provenance keys were lost during + # unwrap. A bare assert is stripped under ``-O`` and would convert a + # metadata-shape anomaly into a 500 on the search request path. + for key in ("archive_span_id", "agent", "project"): + if key in (md or {}) and key not in user_md: + logger.warning( + "_format_hit lost critical key %r during metadata unwrap", + key, + ) - confidence = ( - md.get("similarity") - if isinstance(md, dict) and md.get("similarity") is not None - else hit.get("source_score", 0.0) - ) timestamp = ( user_md.get("timestamp") if isinstance(user_md, dict) and user_md.get("timestamp") is not None @@ -463,6 +467,46 @@ def _format_hit(hit: dict) -> dict: or 0 ) + # Collection doc-currency metadata lives in the inner user-metadata dict + # (that is how ingest_folder's chunk_md stores it), so read from user_md + # after the unwrap, not from the outer envelope. + # is_current is False for superseded/history rows (hidden_by set); + # always True on the default active-recall path. + # as_of is the indexing timestamp: indexed_at (float epoch) or the + # row's created_at/timestamp, coerced to float for a single type. + # is_past_review is True only when review_by is present and overdue. + if isinstance(user_md, dict): + is_current = not (isinstance(md, dict) and "hidden_by" in md) + as_of = user_md.get("indexed_at") + if as_of is None: + as_of = timestamp + try: + as_of = float(as_of) + except (TypeError, ValueError): + logger.warning( + "taosmd: could not convert as_of %r to float, defaulting to 0.0", + as_of, + ) + as_of = 0.0 + review_by = user_md.get("review_by") + has_review_by = review_by is not None + is_past_review = has_review_by and isinstance(review_by, str) and review_by < time.strftime("%Y-%m-%d") + + user_md["is_current"] = is_current + user_md["as_of"] = as_of + if has_review_by: + user_md["is_past_review"] = is_past_review + # doc_id / version / review_by are already in user_md when present; + # no extraction needed. Only superseded_by comes from the envelope. + if isinstance(md, dict) and "hidden_by" in md: + user_md["superseded_by"] = md["hidden_by"] + + confidence = ( + md.get("similarity") + if isinstance(md, dict) and md.get("similarity") is not None + else hit.get("source_score", 0.0) + ) + return { "text": hit.get("text", ""), "source": hit.get("source", "unknown"), diff --git a/tests/test_api.py b/tests/test_api.py index fba710f9..fa40b5de 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -190,7 +190,7 @@ def test_format_hit_prefers_similarity_over_source_score(): assert formatted["confidence"] == 0.85 assert formatted["source"] == "vector" assert formatted["timestamp"] == 1700000000 - assert formatted["metadata"] == {"position": 7, "timestamp": 1700000000} + assert formatted["metadata"] == {"position": 7, "timestamp": 1700000000, "as_of": 1700000000.0, "is_current": True} def test_format_hit_falls_back_to_source_score():