Skip to content

Commit 999d4cc

Browse files
committed
Address AI review: bounded length nudge, docstring, test coverage
- Replace the unbounded length multiplier with a bounded nudge: length maps to a multiplier in [1 - length_nudge, 1] (length_nudge = 5%), so it can only break ties and never overturns a word-coverage difference (>= 10% per consensus word). Old 1/(1+len/n) shrank toward zero without limit, coupling its influence to absolute length. - Update the evaluate_summary_quality docstring to describe consensus vocabulary instead of the removed noun-phrase mechanism. - Add tests: length/coverage inversion regression, bounded-nudge guarantee, bytes input, dedup, unseen-summary scoring, custom vs default bad-phrase behavior, regex/dash penalties, empty best_choice (summary.py now 100%).
1 parent 9394cae commit 999d4cc

3 files changed

Lines changed: 144 additions & 5 deletions

File tree

src/palace/manager/sqlalchemy/model/identifier.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,8 +1121,8 @@ def evaluate_summary_quality(
11211121
to form an overall quality score.
11221122
We need to evaluate summaries from a set of Identifiers
11231123
(typically those associated with a single work) because we
1124-
need to see which noun phrases are most frequently used to
1125-
describe the underlying work.
1124+
need to see which content words are most frequently used to
1125+
describe the underlying work (its consensus vocabulary).
11261126
:param privileged_data_sources: If present, a summary from one
11271127
of these data source will be instantly chosen, short-circuiting the
11281128
decision process. Data sources are in order of priority.

src/palace/manager/util/summary.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,19 @@ class SummaryEvaluator:
8383
re.compile("This is"),
8484
}
8585

86+
# "Shorter is better" is only a tie-breaker, so we express it as a bounded
87+
# nudge: length maps to a multiplier in ``[1 - length_nudge, 1]`` rather
88+
# than the old unbounded ``1 / (1 + len / n)`` that kept shrinking toward
89+
# zero. ``length_nudge`` is the most a long summary can be scaled down (5%).
90+
# Keeping it below the smallest meaningful word-coverage step -- one word
91+
# out of at most ``top_words_to_consider`` (>= 10%) -- guarantees length can
92+
# never overturn a genuine coverage difference; it only decides between
93+
# summaries whose other signals are already within a few percent.
94+
length_nudge: float = 0.05
95+
# Characters at which the nudge reaches half of ``length_nudge``. Sets how
96+
# quickly the (bounded) preference ramps in; it does not change the cap.
97+
length_half_life: int = 1000
98+
8699
def __init__(
87100
self,
88101
optimal_number_of_sentences: int = 4,
@@ -196,8 +209,12 @@ def score(self, summary: str | bytes, apply_language_penalty: bool = True) -> fl
196209
if language_difference > 1:
197210
score *= 0.5 ** (language_difference - 1)
198211

199-
# All else being equal, prefer a shorter summary. This is a gentle
200-
# tie-breaker -- it only matters when the signals above are close.
201-
score *= 1 / (1 + len(summary) / 1000)
212+
# All else being equal, prefer a shorter summary. This is a bounded
213+
# tie-breaker: ``saturation`` grows from 0 toward 1 with length, so the
214+
# multiplier stays within ``[1 - length_nudge, 1]`` no matter how long
215+
# the summary is (~0.976x at 1000 chars, ~0.963x at 3000, never below
216+
# 0.95x). See ``length_nudge`` for why this can only break ties.
217+
saturation = len(summary) / (len(summary) + self.length_half_life)
218+
score *= 1 - self.length_nudge * saturation
202219

203220
return score

tests/manager/core/test_summary_evaluator.py

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,128 @@ def test_generic_marketing_copy_loses_to_on_topic_summary(self):
5151
second = "Elizabeth Bennet and Mr. Darcy overcome pride to find love."
5252
assert generic != self._best(generic, on_topic, second)
5353

54+
def test_length_tiebreaker_does_not_override_word_coverage(self):
55+
# A short summary that covers only some of the consensus vocabulary must
56+
# not beat a longer one that covers all of it: the length preference is
57+
# only a tie-breaker, not a signal strong enough to invert coverage.
58+
short_partial = "Alice meets the Rabbit."
59+
full_coverage = (
60+
"Alice follows the White Rabbit down the hole, meets the Mock "
61+
"Turtle, and the Cheshire Cat, before waking from the dream." * 3
62+
)
63+
# A third summary so the recurring words become consensus vocabulary.
64+
second = (
65+
"Alice, the White Rabbit, the Mock Turtle, and the Cheshire Cat "
66+
"appear in the dream."
67+
)
68+
assert full_coverage == self._best(short_partial, full_coverage, second)
69+
70+
def test_length_nudge_is_bounded(self):
71+
# The length preference is a bounded nudge, not an unbounded penalty:
72+
# however long a summary is, the length term scales its score by at most
73+
# ``length_nudge`` (i.e. never below ``1 - length_nudge``). This pins the
74+
# property that keeps length from ever overriding word coverage.
75+
#
76+
# Isolate the length term: four sentences (the optimal count) zeroes the
77+
# sentence penalty, "cat" carries no bad phrase, and disabling the
78+
# language penalty removes the only other factor -- so the score is
79+
# exactly the length multiplier.
80+
huge = "One cat. Two cats. Three cats. Four cats." + " cat" * 20000
81+
evaluator = SummaryEvaluator()
82+
evaluator.add(huge)
83+
evaluator.ready()
84+
assert len(huge) > 50000 # a genuinely enormous summary...
85+
score = evaluator.score(huge, apply_language_penalty=False)
86+
assert 1 - evaluator.length_nudge <= score < 1.0 # ...yet still bounded
87+
88+
def test_bytes_summaries_are_decoded(self):
89+
# Summaries may arrive as bytes; they should be decoded and ranked just
90+
# like str summaries.
91+
s1 = b"Sentence one. Sentence two. Sentence three. Sentence four."
92+
s2 = b"Only one sentence."
93+
assert s1.decode("utf8") == self._best(s1, s2)
94+
95+
def test_duplicate_summaries_are_only_counted_once(self):
96+
summary = "Alice and the White Rabbit and the Mock Turtle."
97+
evaluator = SummaryEvaluator()
98+
evaluator.add(summary)
99+
evaluator.add(summary)
100+
assert evaluator.summaries == [summary]
101+
102+
def test_scoring_an_unseen_summary(self):
103+
# score() may be called with a summary that was never add()ed; its
104+
# content words are computed on the fly rather than looked up.
105+
evaluator = SummaryEvaluator()
106+
evaluator.add("Alice meets the White Rabbit and the Mock Turtle.")
107+
evaluator.add("The White Rabbit leads Alice to the Mock Turtle.")
108+
evaluator.ready()
109+
unseen = "Alice, the White Rabbit, and the Mock Turtle."
110+
assert evaluator.score(unseen) > 0
111+
112+
def test_bad_phrases_are_penalized(self):
113+
# A summary containing a bad phrase is scored below an otherwise
114+
# equivalent summary without one.
115+
clean = "Alice meets the White Rabbit and the Mock Turtle."
116+
abridged = "An abridged version of Alice and the White Rabbit."
117+
evaluator = SummaryEvaluator()
118+
evaluator.add(clean)
119+
evaluator.add(abridged)
120+
evaluator.ready()
121+
assert evaluator.score(clean) > evaluator.score(abridged)
122+
123+
def test_best_choice_with_no_summaries(self):
124+
evaluator = SummaryEvaluator()
125+
evaluator.ready()
126+
assert evaluator.best_choice() == (None, None)
127+
128+
def test_custom_bad_phrases_replace_the_defaults(self):
129+
# Two evaluators with the same corpus but different bad-phrase sets. By
130+
# scoring the same summaries under both, every scoring term except the
131+
# bad-phrase penalty is held constant, so a score difference isolates
132+
# the effect of the bad-phrase set.
133+
custom_bad = "A forbidden summary of Alice and the White Rabbit."
134+
default_bad = "An abridged summary of Alice and the Mock Turtle."
135+
corpus = [custom_bad, default_bad]
136+
137+
default_evaluator = SummaryEvaluator()
138+
custom_evaluator = SummaryEvaluator(bad_phrases={"forbidden"})
139+
for evaluator in (default_evaluator, custom_evaluator):
140+
for summary in corpus:
141+
evaluator.add(summary)
142+
evaluator.ready()
143+
144+
# "abridged" is a default bad phrase but not in the custom set; the
145+
# custom evaluator therefore stops penalizing it.
146+
assert custom_evaluator.score(default_bad) > default_evaluator.score(
147+
default_bad
148+
)
149+
# "forbidden" is only in the custom set; only the custom evaluator
150+
# penalizes it.
151+
assert custom_evaluator.score(custom_bad) < default_evaluator.score(custom_bad)
152+
153+
def test_score_calls_ready_and_accepts_bytes(self):
154+
# score() called before ready() triggers ready() itself, and it accepts
155+
# bytes just like add().
156+
evaluator = SummaryEvaluator()
157+
evaluator.add("Alice meets the White Rabbit and the Mock Turtle.")
158+
assert evaluator.top_words is None
159+
score = evaluator.score(b"Alice and the White Rabbit.")
160+
assert evaluator.top_words is not None
161+
assert score > 0
162+
163+
def test_regex_bad_patterns_and_dashes_are_penalized(self):
164+
# The regex-based bad patterns and an excess of " -- " separators both
165+
# push a summary's score down.
166+
clean = "Alice meets the White Rabbit and the Mock Turtle."
167+
bad_regex = "Includes Alice and the White Rabbit and the Mock Turtle."
168+
dashes = "Alice -- the White -- Rabbit -- Mock -- Turtle -- again."
169+
evaluator = SummaryEvaluator()
170+
for summary in (clean, bad_regex, dashes):
171+
evaluator.add(summary)
172+
evaluator.ready()
173+
assert evaluator.score(clean) > evaluator.score(bad_regex)
174+
assert evaluator.score(clean) > evaluator.score(dashes)
175+
54176
def test_non_english_is_penalized(self):
55177
"""If description text appears not to be in English, it is rated down
56178
for its deviations from average English bigram distribution.

0 commit comments

Comments
 (0)