Skip to content

fix(hotel_receptionist): pair near-miss rows in the expected-state diff #6799

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
u9g wants to merge 1 commit into
base: main
Choose a base branch
from
+42 −4
Open

fix(hotel_receptionist): pair near-miss rows in the expected-state diff #6799

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 42 additions & 4 deletions examples/hotel_receptionist/benchmark.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,37 @@ def _rows(
return cols, counter


def _changed_fields(
cols: list[str], expected_row: tuple[Any, ...], actual_row: tuple[Any, ...]
) -> list[str]:
return [
f"{col}: {want!r} != {got!r}"
for col, want, got in zip(cols, expected_row, actual_row, strict=True)
if want != got
]


def _pair_rows(
cols: list[str], missing: list[tuple[Any, ...]], unexpected: list[tuple[Any, ...]]
) -> list[tuple[tuple[Any, ...], tuple[Any, ...]]]:
"""Greedily pair each missing row with its nearest unexpected row.

A row the agent got *almost* right is one row, not one missing plus one
unexpected: pairing keeps the reported diff at the field that actually differs.
"""
pairs: list[tuple[tuple[Any, ...], tuple[Any, ...]]] = []
remaining = list(unexpected)
for want in list(missing):
if not remaining:
break
got = min(remaining, key=lambda row: len(_changed_fields(cols, want, row)))
remaining.remove(got)
missing.remove(want)
unexpected.remove(got)
pairs.append((want, got))
return pairs
Comment on lines +120 to +130

@devin-ai-integration devin-ai-integration Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Unrelated missing and extra records get merged into a single misleading difference report

Every absent record is paired with some extra record no matter how dissimilar (min(remaining, ...) at examples/hotel_receptionist/benchmark.py:125), so a truly absent record and a completely unrelated extra one are reported as one "row differs" line instead of two separate findings.

Impact: Grading reports can claim a record was merely edited when in fact one record is missing and a different, unrelated one was created, hiding real failures from whoever reads the report.

Greedy pairing has no similarity threshold

_pair_rows (examples/hotel_receptionist/benchmark.py:112-130) pairs while remaining is non-empty, with no bound on len(_changed_fields(...)). If the expected DB contains booking A that the agent never created, and the agent instead created an unrelated booking B for a different guest, missing == [A] and unexpected == [B], so they get paired and emitted as "row differs on guest_name: ... ; check_in: ... ; ..." listing essentially every compared column — the fact that A is absent and B is spurious is lost. The author's tested case ("a genuinely absent row still reports as missing") only holds when the unexpected list happens to be empty.

A threshold (e.g. only pair when the number of differing fields is small relative to the column count, or when key identity columns match) would preserve the intent while avoiding bogus pairings.

Prompt for agents 
In examples/hotel_receptionist/benchmark.py, _pair_rows greedily pairs every missing row with the nearest unexpected row without any similarity threshold. When a row is genuinely absent from the agent's DB and an unrelated extra row exists, the two get paired and reported as a single 'row differs on <every column>' line, which conceals that one row is missing and another is spurious. Consider only accepting a pairing when the candidate is actually a near miss — e.g. when the number of differing fields is at most some fraction of the compared columns, or when a set of identity-ish columns matches — and leaving non-matching rows to print as plain missing/unexpected.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.



def diff_databases(
expected: apsw.Connection,
actual: apsw.Connection,
Expand All @@ -112,10 +143,17 @@ def diff_databases(
ecols, exp = _rows(expected, sql)
acols, act = _rows(actual, sql)
cols = ecols or acols
for row, n in (exp - act).items():
diffs.append(f"{table}: missing {n}x {dict(zip(cols, row, strict=True))}")
for row, n in (act - exp).items():
diffs.append(f"{table}: unexpected {n}x {dict(zip(cols, row, strict=True))}")
# repr key: rows mix None with str/int in the same column, so tuples aren't
# directly orderable.
missing = sorted((exp - act).elements(), key=repr)
unexpected = sorted((act - exp).elements(), key=repr)
for want, got in _pair_rows(cols, missing, unexpected):
fields = "; ".join(_changed_fields(cols, want, got))
diffs.append(f"{table}: row differs on {fields}")
for row in missing:
diffs.append(f"{table}: missing {dict(zip(cols, row, strict=True))}")
for row in unexpected:
diffs.append(f"{table}: unexpected {dict(zip(cols, row, strict=True))}")
return diffs


Expand Down
Loading