Revise PR #291: The headline field of this card is dead on the real ingest path, and the test suite cannot see it.

[jaylfc]

CARD TITLE (intent, not commit subject): Revise PR #291: The headline field of this card is dead on the real ingest path, and the test suite cannot see it.

Autonomous build of board card tsk-x6ph7n.

BLOCKER 1: _format_hit now reads doc_id, version, and review_by from the
inner user-metadata dict (where ingest_folder stores them), not from the
outer envelope where they were never present on a real hit. The three
new tests built a metadata shape ingest_folder did not produce; the tests
now use hits that match the real double-wrapped envelope.

BLOCKER 2: Replaced the bare assert in the request path with an explicit
if plus logging.warning. The assert was stripped under -O and converted
metadata-shape anomalies into 500 errors instead of a degraded result.
Also guarded the new metadata mutation with isinstance(user_md, dict) so
non-dict metadata degrades to {} instead of raising TypeError, matching
master defensive behavior.

BLOCKER 3: is_current is now derived from the presence of hidden_by on the
outer envelope, so superseded/history rows are False while current rows
remain True.

DEFECT 4: Added _parse_front_matter unit tests and end-to-end tests
covering front-matter surviving ingest_folder to search to _format_hit.

DEFECT 5: version is always int or absent (non-int values dropped at
parse time); as_of is always float (coerced at the boundary); review_by
is validated as an ISO date before storage.

DEFECT 6: front-matter parsing now supports both .md and .markdown
extensions (matching _loader_for); thematic-break documents that open
with --- are not scanned as front matter via a closing-delimiter line
budget; unknown keys are always skipped.

Full suite: 1420 passed, 12 skipped.

Refs: PR #291, PR #295

Files:
changelog.d/tsk-x6ph7n-collection-doc-currency.md | 2 +
taosmd/api.py | 36 ++++++
taosmd/collections.py | 97 +++++++++++++-
tests/test_api.py | 136 +++++++++++++++++++-
tests/test_collections_ingest.py | 150 ++++++++++++++++++++++
5 files changed, 413 insertions(+), 8 deletions(-)

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Warning

Review limit reached

@jaylfc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 19 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ed677426-e41a-4151-a431-fc9340509292

📥 Commits

Reviewing files that changed from the base of the PR and between b8c3d0e and 5ef5596.

📒 Files selected for processing (5)

• changelog.d/tsk-x6ph7n-collection-doc-currency.md
• taosmd/api.py
• taosmd/collections.py
• tests/test_api.py
• tests/test_collections_ingest.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitar-bot]

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

_Gitar

[kilo-code-bot]

WARNING: text.startswith("---\n") fails for markdown files with Windows line endings (\r\n), causing front matter to be silently skipped.

The check requires the file to begin with exactly ---\n. A file that starts with ---\r\n (common on Windows) does not match, so _parse_front_matter returns {} and the document's doc_id, version, and review_by are never extracted. Since splitlines(keepends=True) handles \r\n correctly, only this initial guard is wrong.

Consider normalizing line endings before the check, e.g. text = text.replace("\r\n", "\n"), or matching both ---\n and ---\r\n.

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
WARNING	1

Issue Details (click to expand)

WARNING

File	Line	Issue
taosmd/collections.py	521	text.startswith("---\n") fails for markdown files with Windows line endings (\r\n), causing front matter to be silently skipped

Files Reviewed (5 files)

• changelog.d/tsk-x6ph7n-collection-doc-currency.md
• taosmd/api.py
• taosmd/collections.py - 1 issue
• tests/test_api.py
• tests/test_collections_ingest.py

Fix these issues in Kilo Cloud

---

_{Reviewed by step-3.7-flash · Input: 129.6K · Output: 32K · Cached: 555K}

[jaylfc]

BLOCKED on 1. The fix for DEFECT 5 reintroduces the exact failure class BLOCKER 2 was raised about: an unguarded coercion on the search request path.

Reviewed against card tsk-x6ph7n. Trial merge with current master b8c3d0e, full suite 1420 passed, 12 skipped, 0 failed. The defect below is live on that same green tree. Every probe below ran with a control in the same command, and each probe module asserted its own __file__ was inside the tree under test.

---

First, what this PR actually got right. Do not re-do any of it.

BLOCKER 1 is genuinely discharged, end to end, on the real shape. This was the headline defect and the fix is correct. Real ingest_folder, real search, real envelope, with a no-front-matter control in the same run:

readme.md: doc_id='doc-widget' version=7 review_by='2020-01-01' is_past_review=True  is_current=True as_of=1787000236.9
plain.md : doc_id=None        version=None review_by=None       is_past_review=None  is_current=True as_of=1787000236.9

Same command against master as the control, where the field is absent entirely:

readme.md: doc_id=None version=None review_by=None is_past_review=None is_current=None as_of=None

Reading the doc keys off user_md after the unwrap is the right fix, and test_ingest_folder_front_matter_reaches_search is a properly shaped end-to-end test built on what ingest actually writes, not on the inverted shape the closed PR's tests assumed. That also discharges DEFECT 4.

BLOCKER 2 is discharged. The assert is now a logger.warning, logger is really defined (api.py:31), so the guard survives -O instead of evaporating in the one deployment that wants it. The isinstance(user_md, dict) guard is restored, and non-dict metadata degrades exactly as master does:

str metadata  -> metadata={}     (master: metadata={})
list metadata -> metadata={}     (master: metadata={})

BLOCKER 3 is discharged. A superseded row no longer contradicts itself:

{"hidden_by": "row-99"} -> {'is_current': False, 'superseded_by': 'row-99'}

DEFECT 6 is discharged for the case that was filed. The document that opens with a thematic break now yields {}.

---

THE BLOCKER: float(as_of) turns ordinary caller metadata into a ValueError out of search()

api.py, in the new block:

as_of = user_md.get("indexed_at")
if as_of is None:
    as_of = timestamp
as_of = float(as_of)

timestamp is resolved 12 lines above from user_md.get("timestamp"), which is arbitrary caller-supplied metadata. float() on a non-numeric string raises. Measured through the real public path, ingest_batch then search, with master as the control in the same probe:

--- ISO-string timestamp via ingest_batch (public migration API) ---
  this branch : RAISE ValueError: could not convert string to float: '2026-08-17T20:00:00Z'
  master      : OK, search returned 1 hits

The item was ordinary:

await ingest_batch(
    [{"text": "...", "id": "ext-1",
      "metadata": {"timestamp": "2026-08-17T20:00:00Z", "source_system": "taOS"}}],
    agent="dev", data_dir=...)
await search("sprocket wrench", agent="dev", mode="bm25", data_dir=...)   # raises

ingest_batch's own docstring calls it "the migration path for external memory stores (taOS user memory)". An ISO-8601 timestamp on an imported row is the normal case there, not an exotic one, and catalog_pipeline.py:274 already carries event.get("timestamp", ...) straight out of a JSONL file written by an external producer. So this is not a hypothetical shape.

Master returns the hit. This branch raises. That is a straight regression, and it is on the search request path, which is the precise thing BLOCKER 2 asked this PR to stop doing. The assert was correctly removed at line 452 and an unguarded coercion was added at line 489.

The same class again, two lines below. is_past_review = has_review_by and review_by < time.strftime("%Y-%m-%d") compares caller metadata against a str with no type check:

int review_by   2020            -> RAISE TypeError: '<' not supported between instances of 'int' and 'str'
float review_by 2020.0          -> RAISE TypeError
list review_by  ['2020-01-01']  -> RAISE TypeError
str review_by   '2020-01-01'    -> OK, is_past_review=True

Front matter always yields a str here because _parse_front_matter validates with date.fromisoformat, so this one is only reachable through caller metadata. It is the same shape and wants the same treatment.

Fix: coerce defensively rather than optimistically. try: as_of = float(as_of) except (TypeError, ValueError): as_of = 0.0 with a logger.warning, and gate the review_by comparison on isinstance(review_by, str). DEFECT 5 asked for one representation per field, and a field that raises on the second kind has not achieved that, it has just moved the failure from the consumer to the request.

Why the suite could not see it

1420 tests pass on this tree. Every new test builds review_by and timestamp from well-formed literals, so the coercion is only ever handed input it can convert. This is the third time this session that a green suite has shipped a live defect on the request path (#304's revocation bypass, #305's gate crash, this one), and all three share one shape: the tests exercise the branch that already worked. A test that only feeds a coercion well-formed input has measured the literal, not the coercion.

Non-blocking, worth a line in the follow-up

Front-matter over-capture is narrowed rather than closed. The key allowlist, not _FRONT_MATTER_MAX_LINES, is what fixed DEFECT 6, so prose after a thematic break is still captured when it happens to use a contract key:

"---\n\n# Title\n\nversion: 99\n\n---\n\nmore\n"  ->  {'version': 99}

Low severity, and the allowlist keeps the blast radius to three keys, but the closing delimiter should be required to sit at the top of the document rather than anywhere in the first 50 lines.

Verdict

BLOCKED on 1. Blockers 1, 2 and 3 and defects 4 and 6 are all discharged and the end-to-end test is the right one. The single remaining item is the unguarded coercion pair on the request path. Closing this PR under the close-on-block policy and filing a revision card that carries the credit forward, since the ingest half and the end-to-end test should be kept as they are.