Skip to content

test(corpus): regression coverage for per-line JSONL loading in the auditor#222

Merged
himanshu231204 merged 4 commits into
OpenAgentHQ:mainfrom
Nitjsefnie-OSC:test/corpus-jsonl-coverage
Jul 23, 2026
Merged

test(corpus): regression coverage for per-line JSONL loading in the auditor#222
himanshu231204 merged 4 commits into
OpenAgentHQ:mainfrom
Nitjsefnie-OSC:test/corpus-jsonl-coverage

Conversation

@Nitjsefnie

Copy link
Copy Markdown
Contributor

Closes #57 (assigned — thanks!)

As noted on the issue, the production fix already landed in 4e6fbbb (dup of #127), so this is the missing regression coverage: 7 tests in tests/unit/test_corpus/test_auditor.py locking the merged _load_jsonl_documents behavior, each verified against a live probe of the code before writing —

  • N non-empty lines → N documents (via audit() totals)
  • per-line document shape: content = stripped line, doc_id = {path}:L{n}, metadata extension/filename/line_number
  • blank/whitespace lines skipped
  • malformed JSON line skipped silently (the merged except json.JSONDecodeError: continue policy — asserted as a skip, no invented logging)
  • .json files still load as a single document (routing regression guard)
  • max_documents capped across JSONL lines (locked via the single-file path, which exercises the in-loop break)

Gates: uv run pytest tests/unit — 941 passed, 4 skipped; target file 19 passed (+7). Zero new ruff findings (the one import-ordering error in the file header is pre-existing on main, verified by diffing against pristine).

Generated by Claude Fable 5 (brief, review), Claude Opus 4.8 (implementation)

Nitjsefnie and others added 2 commits July 23, 2026 19:29
Locks the merged behavior of commit 4e6fbbb (per-line JSONL loading via
_load_jsonl_documents): one CorpusDocument per non-empty valid line,
blank/whitespace lines skipped, malformed JSON lines skipped silently,
.json still loaded as a single document, and max_documents enforced
across JSONL lines. Tests only; no production changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@himanshu231204

Copy link
Copy Markdown
Member

/oc review pr

@opencode-agent

Copy link
Copy Markdown
Contributor

Review: test/corpus-jsonl-coverageLGTM — ship it.

Summary

7 well-scoped regression tests that lock the merged _load_jsonl_documents behavior from 4e6fbbb. Every assertion maps 1:1 to the production code.

Verification

Test Implementation Status
test_jsonl_one_document_per_line _load_jsonl_documents iterates splitlines() → N docs
test_jsonl_document_shape content=stripped, doc_id=f"{path}:L{n}", metadata keys
test_jsonl_skips_empty_and_whitespace_lines if not stripped: continue
test_jsonl_skips_malformed_line_silently except json.JSONDecodeError: continue
test_json_file_loads_as_single_document routing checks == ".jsonl" (.json misses → _load_single_file)
test_json_file_document_extension _load_single_file uses file_path.suffix".json"
test_jsonl_respects_max_documents if len(documents) >= self.max_documents: break

Design choices worth noting (none blocking)

  1. Sync vs async split — Tests that call audit() are async; tests that call _load_documents() directly are sync. Both correct, the latter avoids the async fixture machinery for unit-level assertions.

  2. extension hardcoded as ".jsonl" in _load_jsonl_documents rather than derived from file_path.suffix. This is a normalization decision — matches the lowercase routing in _load_documents (path.suffix.lower() == ".jsonl"). Consistent.

  3. No directory-level multi-JSONL test — The break in _load_documents only fires in the non-JSONL else branch, so a directory with multiple .jsonl files could load up to max_documents per file. This is a pre-existing design gap (not introduced by this PR) and out of scope.

Nitpicks

  • test_jsonl_skips_malformed_line_silently could more explicitly assert the error didn't propagate with pytest.raises(SomeException) as a negative check (e.g., with pytest.raises(...): ... for the absence of a raise). The current approach (just verifying docs are returned) is fine and consistent with the rest of the test suite.

New%20session%20-%202026-07-23T18%3A04%3A46.651Z
opencode session  |  github run

@himanshu231204

Copy link
Copy Markdown
Member

@Nitjsefnie test_jsonl_skips_malformed_line_silently could more explicitly assert the error didn't propagate with pytest.raises(SomeException) as a negative check (e.g., with pytest.raises(...): ... for the absence of a raise). The current approach (just verifying docs are returned) is fine and consistent with the rest of the test suite.

@himanshu231204
himanshu231204 merged commit 74dc906 into OpenAgentHQ:main Jul 23, 2026
8 checks passed
@github-actions

Copy link
Copy Markdown

🎉 Congratulations @Nitjsefnie!

Your pull request has been successfully merged into main. 🚀

Thank you for contributing to OpenAgentHQ and helping improve the project.

We truly appreciate your contribution and hope to see you back with more amazing PRs!

Happy Open Sourcing! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

corpus(auditor): .jsonl files loaded as single documents instead of one per line

2 participants