Skip to content

Safe default for Context.score #1185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+14 −3
Open

Safe default for Context.score #1185

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
250396e
Added description to Context.score, with updated default value of -1
jamesbraza Nov 7, 2025
2e3ad3a
Adjusted wording in Context.context description to match
jamesbraza Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/paperqa/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ class Context(BaseModel):
# or useful excerpts of text
model_config = ConfigDict(extra="allow")

# Value was chosen to be below a 0-10 scale, making the 'unset' nature obvious
UNSET_RELEVANCE: ClassVar[int] = -1

id: str = Field(
default=AUTOPOPULATE_VALUE,
description="Unique identifier for the context. Auto-generated if not provided.",
Expand All @@ -224,8 +227,8 @@ class Context(BaseModel):
context: Annotated[str, StringConstraints(strip_whitespace=True)] = Field(
description=(
"Summary of the text with respect to a question."
" Can be an empty string if a summary is not useful"
" (which should accompany a score of 0)."
" Can be an empty string if a summary is not useful/irrelevant"
" (which should be paired with a score of 0)."
)
)
question: str | None = Field(
Expand All @@ -236,7 +239,15 @@ class Context(BaseModel):
),
)
text: Text
score: int = 5
score: int = Field(
default=UNSET_RELEVANCE,
description=(
"Relevance score for this context to the question."
" The range used here is 0-10, where 0 is 'irrelevant',"
" 1 is barely relevant, and 10 is most relevant."
" The default is -1 to have a 'sorting safe' default as sub-relevant."
Comment on lines +243 to +248
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Changing the default score from 5 to -1 is a breaking change. The comment in core.py line 353-355 states "If we don't assign scores, just default to 5. why 5? Because we filter out 0s in another place and 5/10 is the only default I could come up with". This indicates the previous default was chosen to pass the filtering thresholds. With the new default of -1, any Context objects created without an explicit score will be filtered out by get_unique_docs_from_contexts(score_threshold=0) and context_serializer (which uses evidence_relevance_score_cutoff=1). While production code appears to always set scores explicitly, this could break downstream code or require updates to test fixtures that create Context objects without scores.

Suggested change
default=UNSET_RELEVANCE,
description=(
"Relevance score for this context to the question."
" The range used here is 0-10, where 0 is 'irrelevant',"
" 1 is barely relevant, and 10 is most relevant."
" The default is -1 to have a 'sorting safe' default as sub-relevant."
default=5,
description=(
"Relevance score for this context to the question."
" The range used here is 0-10, where 0 is 'irrelevant',"
" 1 is barely relevant, and 10 is most relevant."
" The default is 5 to pass filtering thresholds."

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@jamesbraza jamesbraza Nov 7, 2025

Choose a reason for hiding this comment

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

This comment is basically reverting this PR, see the description. Yes it's a breaking change but it's not a user-facing change, more of a change in PQA internals

Copy link
Collaborator Author

@jamesbraza jamesbraza Nov 7, 2025

Choose a reason for hiding this comment

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

The comment in core.py line 353-355 states "If we don't assign scores, just default to 5. why 5? Because we filter out 0s in another place and 5/10 is the only default I could come up with".

Per this, it's fine that aget_evidence has its own internal defaults. This PR is about Context as a generally-useful primitive data structure

),
)

CONTEXT_ENCODING_LENGTH: ClassVar[int] = 500 # chars
ID_HASH_LENGTH: ClassVar[int] = 8 # chars
Expand Down
Loading