Skip to content

fix(ml): resolve Finding and dict type mismatch in deduplication #259 - #346

Open
prasiddhi-105 wants to merge 4 commits into
ionfwsrijan:mainfrom
prasiddhi-105:fix/deduplication-type-mismatch-259
Open

fix(ml): resolve Finding and dict type mismatch in deduplication #259#346
prasiddhi-105 wants to merge 4 commits into
ionfwsrijan:mainfrom
prasiddhi-105:fix/deduplication-type-mismatch-259

Conversation

@prasiddhi-105

Copy link
Copy Markdown

Linked issue

Closes #259

What this PR does

Fixes a type annotation and attribute access mismatch in the deduplication pipeline where dict inputs could cause silent empty string extraction in embed_findings(). Also widens exception handling during SentenceTransformer initialization in embedder.py to prevent crashes caused by network errors, GPU memory exhaustion, or model load failures.

Type of change

  • Bug fix
  • New feature
  • ML model / training pipeline
  • Refactor (no behaviour change)
  • Documentation
  • Tests only

ML tier (if applicable)

  • Tier 1 — Triage
  • Tier 2 — Predictive
  • Tier 3 — Autonomous
  • Not ML-related

Stack affected

  • Backend
  • Frontend
  • Both

Changes

Backend

  • backend/app/ml/embedder.py:
    • Refactored _extract_text() to safely handle both Pydantic Finding objects and raw Python dict instances.
    • Widened exception handling around SentenceTransformer loading to catch all errors (e.g., OSError, ConnectionError, OutOfMemoryError) and fail gracefully.
    • Updated embed_findings() type hints to List[Union[Finding, Dict[str, Any]]].
  • backend/app/ml/deduplicator.py:
    • Updated type signatures for deduplicate() and internal wrapper embed_findings() to explicitly allow Union[Finding, Dict[str, Any]].

Frontend

  • No frontend changes.

New dependencies

  • No new dependencies added.

Database / schema changes

  • No database schema changes.

Testing

How did you test this?

  • Ran unit tests locally with python -m pytest (109 passing tests).
  • Verified _extract_text returns non-empty strings for both Finding model instances and standard dictionary inputs.

Checklist

  • Tested locally end-to-end (upload ZIP or GitHub URL → scan → findings returned correctly)
  • New ML model falls back gracefully when model file is absent
  • No new console.error or unhandled Python exceptions introduced
  • Added or updated tests where applicable
  • requirements.txt / package.json updated if new dependencies added
  • New model files (.pkl, .pt, etc.) are gitignored, not committed

Anything reviewers should focus on

Please check the dual Finding / dict handling in _extract_text() and the broadened Exception catch during SentenceTransformer initialization.

Screenshots (if UI changed)

N/A (Backend logic fix)

@github-actions

Copy link
Copy Markdown

🎉 Thank you @prasiddhi-105 for submitting a Pull Request!

We're excited to review your contribution.

✅ Before Review

  • Ensure all CI checks pass.
  • Complete the PR template.
  • Link the related issue.

Want faster reviews and contributor support?

Join our Discord community:

🔗 https://discord.gg/FcXuyw2Rs

Maintainers and mentors are active there and can help resolve blockers quickly.

Happy Contributing! 🚀

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.

Deduplication executes on empty embeddings due to Pydantic/dict type mismatch - all deduplication meaningless

1 participant