Skip to content

fix(batch_routing): remove dead build_jsonl_body - #967

Merged
seonghobae merged 1 commit into
mainfrom
fix/hollow-path-build-jsonl-body
Sep 1, 2026
Merged

seonghobae merged 1 commit into
mainfrom
fix/hollow-path-build-jsonl-body

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

Removes a hollow/dead code path flagged by an org-wide audit responding to "빈 깡통 경로 너무 많다.." (too many empty-can/hollow-shell code paths).

build_jsonl_body (contextual_orchestrator/batch_routing.py) had zero callers outside its own dedicated test, and unlike its embeddings twin build_embeddings_jsonl_body, it was never re-exported from __init__.py. The real batch-submission path, PgLlmBatchBackend._assemble_payload, builds the JSONL lines directly via a list comprehension over BatchRequest.to_jsonl_line(...) — it never calls build_jsonl_body. So the function was pure unreferenced dead weight, not a stub for an unwired path.

Independent verification before fixing

  • Confirmed via grep -rn "build_jsonl_body" across the whole repo: only contextual_orchestrator/batch_routing.py (definition) and tests/test_batch_routing.py (its own dedicated test) reference the name.
  • Confirmed contextual_orchestrator/__init__.py re-exports build_embeddings_jsonl_body, cheapest_upstream, etc., but never build_jsonl_body — matching the finding's exact asymmetry claim.
  • Read PgLlmBatchBackend._assemble_payload and confirmed the production JSONL-assembly path is independent of this function ([request.to_jsonl_line(self._endpoint) for request in requests]), so nothing needs wiring — removal is correct rather than wiring-in.

Changes

  • Removed build_jsonl_body from contextual_orchestrator/batch_routing.py.
  • Removed its dedicated test (test_build_jsonl_body_uses_openai_batch_line_shape) and now-unused import from tests/test_batch_routing.py.

Scoped to exactly this one finding — no other cleanup bundled in.

Verification

Run from an isolated clone, Python 3.12 (the fast_mlsirm private-repo optional dependency, gated to python_full_version >= "3.12", could not be fetched in the sandbox — network-blocked for private GitHub archives — so it and its one dependent test were excluded; unrelated to this change, see the full test run below):

python -m pytest tests --deselect tests/test_psychometric_routing.py::test_fast_mlsirm_fit_uses_judge_acceptance_item_for_context_score -q
# 2855 passed, 1 skipped, 1 deselected in 808.23s

coverage run -m pytest tests ... && coverage report --show-missing
# contextual_orchestrator/batch_routing.py: 310 stmts, 1 miss, 99% (the one miss is a pre-existing,
# unrelated branch in PgLlmBatchEmbeddingBackend.submit, unchanged by this PR)

interrogate -v contextual_orchestrator
# RESULT: PASSED (minimum: 100.0%, actual: 100.0%); batch_routing.py 41/41 100%

Also ran the unmodified full suite once (before excluding the private-dep test) to confirm the only failure is the pre-existing fast_mlsirm import gap: 1 failed, 2855 passed, 1 skipped — the single failure is ModuleNotFoundError: No module named 'fast_mlsirm' in tests/test_psychometric_routing.py, unrelated to batch_routing.py.

🤖 Generated with Claude Code


Generated by Claude Code

build_jsonl_body had zero callers outside its own test and, unlike its
embeddings twin build_embeddings_jsonl_body, was never re-exported from
__init__.py -- the real submission path (PgLlmBatchBackend._assemble_payload)
builds JSONL lines directly via to_jsonl_line() and never calls it. Remove
the hollow function and its dedicated test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 57 minutes.

View limit details

Limit details: You’ve used the included review currently available.

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0fa1179b-07e8-4504-9677-9f754433ee58

📥 Commits

Reviewing files that changed from the base of the PR and between c6c3a0c and 2488f2c.

📒 Files selected for processing (2)
  • contextual_orchestrator/batch_routing.py
  • tests/test_batch_routing.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

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

@seonghobae
seonghobae marked this pull request as ready for review August 31, 2026 17:07

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment thread contextual_orchestrator/batch_routing.py
@seonghobae

Copy link
Copy Markdown
Contributor Author

Cross-PR integration contract: routing identity is provider-neutral model_group only; do not add or preserve a provider-family abstraction. OpenRouter discovery must retain concrete free model IDs, while the aggregate openrouter/free router is not a serving candidate. OpenCode, Noema, and Strix must call contextual-orchestrator. Do not impose fixed wall-clock deadlines on inference, initial ping, readiness/health, provider discovery, or OpenRouter ZDR-list retrieval; use explicit cancellation and evidence-backed transport failure instead. Reconcile this PR with #971 and central .github #1508 before merge.

Copy link
Copy Markdown
Contributor Author

Two things on this PR:

opencode-review required check (failed, head 2488f2c8): same structural race already diagnosed on other PRs this window — the job polls pulls/967/reviews for up to 90 minutes waiting for a current-head verdict from opencode-agent, but no verdict landed in that window, so it exited non-zero. Not caused by this PR's diff (a two-file, 14-line dead-code removal). Re-queued the failed job just now; the merge-scheduler dispatch step at the top of the job will fire a fresh review request for this exact head. Will keep watching.

Cross-PR model_group/OpenRouter/deadline contract note: checked — this PR's diff is contextual_orchestrator/batch_routing.py (removes the dead, zero-caller build_jsonl_body) and tests/test_batch_routing.py only. Neither file is touched by #971 (fix(routing): select concrete free model groups, 25 files — confirmed batch_routing.py/test_batch_routing.py are not among them), and this PR carries no routing-identity, OpenRouter-discovery, or deadline logic to reconcile against that contract. No changes needed here on that front; flagging in case the broadcast was meant for a different PR in the routing family.


Generated by Claude Code

@seonghobae

Copy link
Copy Markdown
Contributor Author

Contextual-Orchestrator와 관계한 것들을 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. Timeout은 적어도 3시간으로 잡으세요. 120초 같은 건 당황스럽군요.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Contextual-Orchestrator와 관계한 것들을 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. Timeout은 적어도 3시간으로 잡으세요. 120초 같은 건 당황스럽군요. Strix가 6시간 이상 동작해서 취약점 잡는 것도 본 일이 있습니다. Opencode와 Noema 는 Coderabbitai 및 Devin 수준으로 실제로 리뷰를 하게 하시오. Strix도 보안 리뷰를 꼼꼼하게 하도록 하시오. 특히 보안 리뷰는 전체 코드로 수행하는 것입니다. Contextual-Orchestrator는 실시간으로 빠르면서 능력이 좋은 모델에 요청을 보내어 시간을 당기시오. @opencode-agent 라고 부르면 호출되는 기능도 인터넷 가이드에는 /oc 라고 나와있기 때문에 이 점도 확인해 보는 게 좋겠습니다.

Copy link
Copy Markdown
Contributor Author

opencode-review failed after its 90-minute poll never saw an APPROVED/CHANGES_REQUESTED verdict from opencode-agent on this head — likely the same class of review-dispatch/wake bug ContextualWisdomLab/.github#1507 (just merged) fixed (e.g. required-review wake authorization, large check-rollup pagination gaps). Queued a re-run now that the fix is in main; will check back once it completes (expect up to ~90 minutes given the poll window).


Generated by Claude Code

@seonghobae
seonghobae merged commit 1efcd42 into main Sep 1, 2026
36 of 39 checks passed
@seonghobae
seonghobae deleted the fix/hollow-path-build-jsonl-body branch September 1, 2026 07:28
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.

2 participants