Skip to content

fix(llm): reject nonconforming structured payloads - #1553

Draft
seonghobae wants to merge 6 commits into
developfrom
codex/goal21-json-schema-current
Draft

seonghobae wants to merge 6 commits into
developfrom
codex/goal21-json-schema-current

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

목적

Naruon이 소유하는 구조화 의미 payload의 검증 경계를 강화합니다. 이 PR은 provider/model routing 또는 OpenAI-compatible transport를 승인하거나 구현하는 PR이 아닙니다.

Exact identity

  • protected base: develop@042b0c70531b229af3acbd0421a2f23098d848b3
  • branch: codex/goal21-json-schema-current
  • predecessor: c05948278b1b23932bb73215a4235d4ed855f4bb
  • exact head: 4643e3404e01a3c860cdbddb225000493357bb07
  • fresh compare: behind_by=0; product/test delta remains exactly six files
  • lifecycle: Draft; mechanical mergeability is not merge authorization

Retained product delta

The useful delta is provider-agnostic schema hardening:

  • ExtractionResult, ExtractionPayload, its nested object/relation payloads, and GroundedAnswerPayload use ConfigDict(extra="forbid", strict=True).
  • Unknown fields therefore fail validation instead of being silently dropped by Pydantic's default extra=ignore behavior.
  • Strict scalar validation prevents machine-readable ids/scores from being accepted only through coercion.

These payload models are Naruon product/domain contracts and can be hardened independently of transport ownership.

Architecture repair after live review

The predecessor test file used a real AsyncOpenAI client, synthetic provider URL/API key, literal orchestrator/free model selection, and direct _call_llm(...) seams to prove an OpenAI-compatible wire envelope. That characterization may describe current protected-base behavior, but it also encoded the wrong authority: Naruon must not define provider URL, credential, model, group, pool, or fallback semantics for production LLM work.

A fresh 2026-09-04 owner read still returns zero GitHub Releases for ContextualWisdomLab/contextual-orchestrator, so there is no immutable released consumer API/client/schema/runtime contract that this PR can truthfully bind to.

Direct non-force child 9efd4425323dfa9f768569ca085a0ec335601308 replaces those transport tests with provider-agnostic Pydantic contract tests. They verify top-level additionalProperties=false, runtime rejection of unknown top-level and nested project-graph fields, and strict scalar rejection without asserting any provider/model/base URL/API-key/OpenAI-SDK transport.

Direct non-force child 13754d679eba8c997e27cf42cfed6e1ae60cefc6 removes the predecessor's unrelated parenthesis-indentation churn from llm_service.py; protected-base comparison now shows that file's effective delta is only the ConfigDict import/config needed for payload validation.

Current non-force child 4643e3404e01a3c860cdbddb225000493357bb07 removes a later, unrelated migration from client.beta.chat.completions.parse to the stable SDK namespace and its private openai.lib._parsing assertion. The installed OpenAI SDK supports both paths, but choosing Naruon's future wire contract is outside this payload-hardening slice and remains blocked on a released contextual-orchestrator consumer contract.

Existing direct AsyncOpenAI production seams visible on protected develop are migration debt owned by the broader contextual-orchestrator consumer work; this PR does not treat them as a valid architectural exception merely because it touches their payload classes.

Relation to #1529

This PR does not fully supersede #1529. #1529 retains useful structured-output wire characterization but is correctly Draft pending an immutable contextual-orchestrator consumer contract or fail-closed production migration. Closing it as fully inherited would lose its transport fixtures/evidence before they are recreated at the canonical released boundary.

The valid portion inherited here is only the fail-closed structured payload requirement. Any eventual wire/E2E successor must exercise the released contextual-orchestrator contract rather than reconstructing provider routing in Naruon.

Verification boundary

On exact head 4643e3404e01a3c860cdbddb225000493357bb07, 71 focused payload/service/extractor contracts and the full backend suite (1820 passed, 33 skipped) pass; git diff --check also passes. Fresh exact-head hosted checks/reviews must still be read after this push. Keep Draft until one unchanged exact head has every then-live protected repository/organization check terminal-success, zero valid unresolved findings/threads, and the qualifying independent post-last-push approval required by live governance.

No self-approval, bypass/admin merge, force-push, destructive rebase, mutable owner dependency, direct-provider fallback, dummy/no-op requeue commit, or predecessor-evidence transfer.

Summary by CodeRabbit

  • Validation

    • Structured AI responses now reject unknown fields and invalid data types.
    • Project graph confidence values must be finite and within the 0–1 range.
    • Invalid confidence values are no longer automatically corrected before storage.
  • Tests

    • Added coverage for strict field validation, identifier types, and confidence boundaries.
    • Updated project graph tests to verify valid confidence values are preserved.

Signed-off-by: Seongho Bae <me@seonghobae.me>
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 4e8e6842-cba8-4802-96c4-6f586c890b15

📥 Commits

Reviewing files that changed from the base of the PR and between 042b0c7 and b424258.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • backend/services/llm_service.py
  • backend/services/project_graph/llm_extractor.py
  • backend/services/rag_service.py
  • backend/tests/test_llm_structured_output_contract.py
  • backend/tests/test_project_graph_llm_extractor.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Pydantic LLM payload models now reject unknown fields and implicit type conversions. Project graph confidence values must be finite and within [0, 1]. Manual confidence clamping was removed, and tests cover the stricter contracts.

Changes

Structured output contracts

Layer / File(s) Summary
Provider response validation
backend/services/llm_service.py, backend/services/project_graph/llm_extractor.py, backend/services/rag_service.py, backend/tests/test_llm_structured_output_contract.py, CHANGELOG.md
LLM payload models reject extra fields and implicit type conversions. Project graph confidence fields reject values outside [0, 1] and non-finite values. Tests validate the schemas and provider-boundary failures.
Confidence propagation
backend/services/project_graph/llm_extractor.py, backend/tests/test_project_graph_llm_extractor.py
Object and edge construction now stores validated confidence values directly. Extractor tests preserve a valid confidence of 0.75 and no longer test clamping.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to b4242

Structured LLM payloads now reject unknown fields, coercions, and invalid confidence values before graph persistence. The validated confidence values are propagated directly, with no remaining concrete merge-blocking risk identified.

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting nonconforming structured LLM payloads.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/goal21-json-schema-current

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 enabled auto-merge (squash) September 4, 2026 09:19
@seonghobae
seonghobae marked this pull request as draft September 4, 2026 09:48
auto-merge was automatically disabled September 4, 2026 09:48

Pull request was converted to draft

@seonghobae seonghobae changed the title fix(llm): reject nonconforming structured output fix(llm): reject nonconforming structured payloads Sep 4, 2026
Assisted-by: OpenAI Codex
Signed-off-by: Seongho Bae <me@seonghobae.me>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head repair b4242580e93ada697b405f8c48eb822daadd6de9 fixes a remaining contradiction in this PR's fail-closed structured-output contract. ExtractedObjectPayload and ExtractedRelationPayload previously accepted negative, greater-than-one, NaN, and Infinity confidence values; downstream code then clamped finite outliers, which could turn provider value 7.5 into maximum confidence and allowed non-finite data to cross the schema boundary. Both payloads now require finite 0.0–1.0 confidence, and downstream projection consumes the validated value without heuristic clamping. Production payload classes also carry boundary docstrings and the change is recorded in the Unreleased changelog.\n\nEvidence on this exact head: 8/8 new invalid-confidence cases RED before the fix; 71 focused structured-output/project-graph/LLM tests GREEN with warnings as errors; Ruff and git diff --check pass. The deleted historical Strix run exposed only a generic exit-code annotation, so no clean or source-finding claim is inferred from it. Fresh hosted checks and current-head independent review remain required.

@seonghobae
seonghobae marked this pull request as ready for review September 4, 2026 17:15
coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 4, 2026
Signed-off-by: Seongho Bae <me@seonghobae.me>
Remove the unrelated OpenAI SDK wire-path migration and private serializer assertion from the structured-payload repair. Preserve strict product-owned payload validation while leaving transport migration to the released contextual-orchestrator contract.\n\nAssisted-by: OpenAI Codex <codex@openai.com>
@seonghobae
seonghobae marked this pull request as draft September 4, 2026 20:38
auto-merge was automatically disabled September 4, 2026 20:38

Pull request was converted to draft

seonghobae added a commit that referenced this pull request Sep 4, 2026
Record Naruon PR #1553 exact-head validation and the removal of unrelated OpenAI SDK transport churn from the product-owned payload hardening slice.\n\nAssisted-by: OpenAI Codex <codex@openai.com>
@seonghobae seonghobae added bug Something isn't working priority: medium Normal-priority or P2 work type: bug Defect or incorrect behavior labels Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Normal-priority or P2 work type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant