Drop the duplicate fixture keys the loops/runs merge left behind - #206
Merged
Conversation
#205 merged two sides that had each added owner/visibility/posture to the same Python fixtures. Unlike TypeScript, where the compiler rejected the duplicates as TS1117, Python silently keeps the last key, so nine repeated keys landed on main and ruff's F601 was the only signal. In test_create_or_adopt.py, test_smoke.py and test_transcript.py the losing copy was the one main had established, so the fixtures quietly changed shape — an agent fixture went from team/organization/team to person/private/ only_you, and two session fixtures likewise. Removing the later duplicates restores the values main had before the merge. test_artifacts.py already resolved to main's values; only the dead lines go. No behavior change beyond the fixtures, and no assertion reads these fields. Also reflows the #205 changelog bullets to the four-line limit in the repo's changelog guidelines. Verified: ruff F601/F602 clean, tsc clean, make test green — TypeScript 135/135, Python 134 passed and 2 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LbC7p6PaNNwijM86bWqEB4
|
Warning Review limit reached
On-demand reviews are free for the next 20 days. After that, they cost $0.25 per reviewed file. Or wait 51 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 64 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #205, on CodeRabbit's review of it.
What happened
#205 merged two sides that had each added
owner/visibility/posturetothe same fixtures. In TypeScript the compiler caught this as
TS1117and I fixedit before merge. Python has no such check — it silently keeps the last key — so
nine duplicate keys landed on
main, with ruff'sF601as the only signal.Why it matters
In three of the four files the losing copy was the one
mainhad established, sothe fixtures quietly changed shape:
test_create_or_adopt.pytest_smoke.pytest_transcript.pytest_artifacts.pyNothing asserts on these fields today, so no test was failing — but the fixtures
had stopped describing the contract
mainintended, which is exactly the kind ofdrift that makes a later ownership test wrong for invisible reasons.
Removing the later duplicates restores the pre-merge values.
Also
Reflows the #205 changelog bullets to the four-line limit in the repo's changelog
guidelines, the other thing CodeRabbit flagged.
Verification
ruff check --select F601,F602 python/— all checks passedtsc --noEmit— cleanmake test— Go ok, TypeScript 135/135, Python 134 passed and 2 skipped🤖 Generated with Claude Code
https://claude.ai/code/session_01LbC7p6PaNNwijM86bWqEB4