Skip to content

fix(dedupe): make edge contradiction reasoning explicit - #1784

Open
Whxuan0701 wants to merge 1 commit into
getzep:mainfrom
Whxuan0701:codex/issue-1666-edge-contradiction
Open

Whxuan0701 wants to merge 1 commit into
getzep:mainfrom
Whxuan0701:codex/issue-1666-edge-contradiction

Conversation

@Whxuan0701

Copy link
Copy Markdown

Closes #1666

Summary

  • Add a reasoning field to the EdgeDuplicate structured response before the final index arrays.
  • Require prompt reasoning to cite continuous fact indices before returning duplicate/contradiction lists.
  • Keep the field optional for backward compatibility with providers that omit it.

Design

Structured-output field order gives non-reasoning small models an explicit place to analyze facts before emitting indices. The existing duplicate and contradiction arrays remain unchanged, so downstream validation and invalidation behavior is preserved.

Tests

  • uv run --with pytest --with pytest-asyncio pytest --confcutdir=tests/prompts tests/prompts/test_dedupe_edges.py tests/utils/maintenance/test_edge_operations.py -q (11 passed)
  • Ruff check and format check on changed files.

Compatibility/Risks

Additive response-model field with an empty default. This improves the prompt/schema contract but cannot guarantee model accuracy for every provider or model. No external LLM calls were required.

@zep-cla-assistant

Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. For privacy information, see our Privacy Notice. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA behalf on myself, e-mail: example@example.com

or

I have read the CLA Document and I hereby sign the CLA behalf of my company, e-mail: example@example.com

Signature is valid for 6 months.


XD seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
This bot will be retriggered when the Contributor License Agreement comment has been provided. Posted by the CLA Assistant Lite bot.

@linhongyu510 linhongyu510 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new reasoning field is optional (default=""), so the raw Pydantic schema does not include it in required. That undercuts the mechanism this PR and #1666 rely on: OpenAIGenericClient and AnthropicClient pass model_json_schema() directly, and Gemini likewise consumes the model/schema, so those providers may legally emit the two arrays without first generating reasoning. The issue’s measured variant used Field(...); the new test checks only declaration order and therefore misses this. Please make reasoning required and assert that reasoning is present in EdgeDuplicate.model_json_schema()["required"] (ideally also cover a provider-facing schema path). I reproduced the current schema on 8f559931: required == ["duplicate_facts", "contradicted_facts"], while the focused 11 tests still pass. Also please remove the stray literal apostrophes embedded at the ends of the new prompt lines (reasoning field. ', reasoning ', the '); the current test only checks substrings and does not catch the malformed prompt text.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

2 participants