fix: prevent CitationMixin crash when context missing 'context' key - #2460
fix: prevent CitationMixin crash when context missing 'context' key#2460truecallerabreham wants to merge 1 commit into
Conversation
ErenAta16
left a comment
There was a problem hiding this comment.
Correcting my own review on #2492: I described that as one of two PRs for #2459 when there are actually four, and this one predates all of them by eight days. My duplicate check was too shallow, apologies.
The fix here is right, and identical in substance to #2488 and #2492: info.context.get("context", None) returns None when the validation context exists but has no context key, and both downstream uses assume a string:
regex.search('(quote){e<=0}', None) -> TypeError: expected string or buffer
with text_chunks[span[0]:span[1]] failing on None right after. Returning early is the correct shape, and it matches the info.context is None guard already two lines up.
Comparing the four for whoever triages:
| PR | date | test | notes |
|---|---|---|---|
| #2460 (this) | Jul 18 | yes, new tests/dsl/test_citation.py |
earliest |
| #2471 | Jul 20 | - | closed |
| #2488 | Jul 26 | yes, in existing coverage file | adds CHANGELOG entry |
| #2492 | Jul 27 | no |
This one has priority by date and ships a test. The only thing #2488 carries that this doesn't is the CHANGELOG line, which is trivial to add here. I'd land this one.
|
Closed as a duplicate implementation. The citation-context fix and regression coverage shipped in #2495 via the consolidated canonical patch. |
Summary
Fix
CitationMixin.validate_sourcescrash whenvalidation_contextis provided but doesn't contain the expected"context"key.Problem
validate_sourcescallsinfo.context.get("context", None)which returnsNoneif the key is missing. ThisNoneis passed toget_spans(None), which calls_get_span(quote, None), which callsregex.search(pattern, None)— raisingTypeError: expected string or bytes-like object.Fix
Add
if text_chunks is None: return selfafter the.get()call (1 line).Test
Added
tests/dsl/test_citation.pywith 3 regression tests covering:"context"key in validation_contextIssue
Closes #2459