Skip to content

feat: make similarity thresholds configurable for judge and temporal search#213

Merged
ved015 merged 9 commits into
XortexAI:mainfrom
Pratyush426:feat/configurable-similarity-thresholds
Jun 1, 2026
Merged

feat: make similarity thresholds configurable for judge and temporal search#213
ved015 merged 9 commits into
XortexAI:mainfrom
Pratyush426:feat/configurable-similarity-thresholds

Conversation

@Pratyush426
Copy link
Copy Markdown
Contributor

Context

Currently, the cosine similarity thresholds used for summary memory updates in JudgeAgent (0.4) and temporal event search in Neo4jClient (0.3) are statically hardcoded. This prevents adjustability of the memory retrieval layer sensitivity when adopting custom or dynamic embedding configurations (e.g., 384-dimensional FastEmbed, 768-dimensional Gemini, or 1536-dimensional OpenAI embeddings), which naturally produce different similarity distributions.

Changes

  1. Configuration: Added summary_judge_similarity_threshold (default 0.4) and temporal_search_similarity_threshold (default 0.3) settings using pydantic-settings in src/config/settings.py.
  2. Judge Agent: Refactored SUMMARY_JUDGE_SIMILARITY_THRESHOLD, _has_summary_judge_candidates, and _deterministic_summary_add in src/agents/judge.py to read dynamically from settings, ensuring generated reasons output custom values.
  3. Database Client: Refactored search_events_by_embedding in src/graph/neo4j_client.py to fallback to settings.temporal_search_similarity_threshold when no override parameter is specified.
  4. Verification: Implemented a comprehensive test suite in tests/test_configurable_thresholds.py using isolated, mocked connection wrappers.

Testing Proof

  • All existing tests pass without regression:
    python -m pytest tests/test_deterministic_memory_layer.py (5 passed)
  • Dynamic settings configurations pass:
    python -m pytest tests/test_configurable_thresholds.py (2 passed)

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request makes similarity thresholds configurable by introducing summary_judge_similarity_threshold and temporal_search_similarity_threshold settings, updating the judge agent and Neo4j client to use them, and adding corresponding unit tests. Feedback suggests avoiding module-level evaluation of the judge threshold to prevent stale values, adding validation constraints (ge/le) to the Pydantic fields, and refactoring tests to use pytest's monkeypatch fixture instead of manual try...finally blocks.

Comment thread src/agents/judge.py Outdated
Comment thread src/config/settings.py
Comment thread tests/test_configurable_thresholds.py Outdated
Comment thread tests/test_configurable_thresholds.py Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 1, 2026

Greptile Summary

This PR replaces two hardcoded cosine similarity thresholds (0.4 in JudgeAgent and 0.3 in Neo4jClient) with configurable pydantic-settings fields, making the memory retrieval layer tunable for different embedding models without code changes.

  • settings.py: Adds summary_judge_similarity_threshold (ge=0.0, le=1.0, default 0.4) and temporal_search_similarity_threshold (ge=-1.0, le=1.0, default 0.3); the asymmetric lower bounds are correct — summary matching is semantically bounded to non-negative scores while raw cosine can be negative.
  • judge.py: Removes the frozen module-level constant SUMMARY_JUDGE_SIMILARITY_THRESHOLD and replaces all three usage sites with live reads from settings, eliminating the previously reported threshold-drift risk.
  • neo4j_client.py: Converts similarity_threshold to Optional[float] = None with a settings fallback; the import is correctly placed at module level.

Confidence Score: 5/5

Safe to merge — all three threshold read sites in judge.py now consistently use live settings values, the new fields carry appropriate range validation, and backward-compatible defaults preserve existing behaviour.

The change is a straightforward configurability refactor with no functional regressions: defaults are identical to the old hardcoded values, the module-level frozen constant has been fully removed, all call sites read the live settings object, and Pydantic field constraints prevent misconfiguration at startup.

No files require special attention.

Important Files Changed

Filename Overview
src/config/settings.py Adds two new validated threshold fields; intentional asymmetry in lower bounds (0.0 vs -1.0) is appropriate given their distinct semantic domains.
src/agents/judge.py Removes the module-level frozen constant and consistently reads the live settings value in all three call sites (_has_summary_judge_candidates, _filter_matches_by_threshold, _deterministic_summary_add).
src/graph/neo4j_client.py Converts similarity_threshold to Optional[float] with settings fallback; import correctly placed at module level.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    S[Settings singleton] -->|summary_judge_similarity_threshold| HA[_has_summary_judge_candidates]
    S -->|summary_judge_similarity_threshold| FM[_filter_matches_by_threshold]
    S -->|summary_judge_similarity_threshold| DA[_deterministic_summary_add]
    S -->|temporal_search_similarity_threshold| NE[Neo4jClient.search_events_by_embedding]

    HA -->|no candidates| DA
    HA -->|candidates exist| FM
    FM --> LLM[LLM judge call]
    DA --> RET1[Return deterministic ADD]
    LLM --> RET2[Return JudgeResult]
    NE --> RET3[Return matching events]
Loading

Reviews (7): Last reviewed commit: "remove test file" | Re-trigger Greptile

Comment thread src/graph/neo4j_client.py
Comment thread tests/test_configurable_thresholds.py Outdated
Comment thread src/config/settings.py
@ved015
Copy link
Copy Markdown
Contributor

ved015 commented Jun 1, 2026

hi @Pratyush426 thank you for the contribution will review it but please resolve all the conversation and leave comment on them it makes us easy to track for future PR's

@ved015
Copy link
Copy Markdown
Contributor

ved015 commented Jun 1, 2026

@Pratyush426 please revert the last commit its not needed i will be taaking care of the failing CI myself and please remove the comments from your changes rest looks fine

@ved015
Copy link
Copy Markdown
Contributor

ved015 commented Jun 1, 2026

Once you are done with this please ping me i will do a final review and merge

@Pratyush426 Pratyush426 force-pushed the feat/configurable-similarity-thresholds branch from 2c41817 to 71c4607 Compare June 1, 2026 11:22
Comment thread tests/test_configurable_thresholds.py Outdated
@github-actions github-actions Bot removed the tests label Jun 1, 2026
@github-actions github-actions Bot added the tests label Jun 1, 2026
@ved015
Copy link
Copy Markdown
Contributor

ved015 commented Jun 1, 2026

@Pratyush426 please stop adding new commits i have to run the workflow again and again now please dont push anything i will do a final review and take care myself

@Pratyush426
Copy link
Copy Markdown
Contributor Author

Apologies for the multiple pushes. I will not push anything else to the branch. The codebase and test suite are completely clean and ready for your final review whenever you are ready. Thank you.

@github-actions github-actions Bot removed the tests label Jun 1, 2026
@ved015
Copy link
Copy Markdown
Contributor

ved015 commented Jun 1, 2026

lgtm

@ved015 ved015 merged commit 647c9b7 into XortexAI:main Jun 1, 2026
10 of 13 checks passed
@ved015
Copy link
Copy Markdown
Contributor

ved015 commented Jun 1, 2026

@Pratyush426 thank you for your contribution keep on raising such fruitful PR's

@Pratyush426
Copy link
Copy Markdown
Contributor Author

Thanks for merging this. I really enjoyed working on the codebase and collaborating on the review process.

I saw that you are hiring Founding Software Engineer Interns (via your recent LinkedIn post) and I have submitted my application. Given my familiarity with the XMem codebase and my interest in memory architectures, I would love to be considered.

linkedin post: https://www.linkedin.com/posts/hiring-softwareengineering-internship-share-7466563297442643968-n8gZ/?utm_source=share&utm_medium=member_desktop&rcm=ACoAAEep3lUBYeU3eSvDkxRCTzCExOlteRECksw
form: https://docs.google.com/forms/d/e/1FAIpQLSd3rIH3X6-xypLnFxng3lBcNzIETRYG2c3O_JtKRgBGV7P1Eg/viewform

@ved015
Copy link
Copy Markdown
Contributor

ved015 commented Jun 1, 2026

Sure thank you for your interest please fill the form we will have a look

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants