fix resume#548
Conversation
WalkthroughThe resume/deduplication logic in the response regeneration script now uses consistent identifier format. The ChangesResume Key Format Alignment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Couldn't analyze Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/response_regeneration/script.py`:
- Around line 119-121: load_seen() currently adds fallback keys from
metadata.idx as plain digits but main() expects fallback keys in the form
"sample_{idx}", causing dedup resume to fail; update the logic where key =
obj.get("id") or obj.get("metadata", {}).get("idx") to normalize the fallback by
wrapping numeric metadata.idx values as f"sample_{idx}" before adding to the
seen set (ensure the same normalization is applied in all places that add to
seen, including the other occurrence around lines 300-302) so both load_seen()
and main() use identical key formats.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 22949fdb-97d5-4240-98a3-6476c8754e4e
📒 Files selected for processing (1)
scripts/response_regeneration/script.py
| key = obj.get("id") or obj.get("metadata", {}).get("idx") | ||
| if key is not None: | ||
| seen.add(str(key)) |
There was a problem hiding this comment.
Normalize metadata.idx fallback to sample_{idx} to make resume work on legacy files.
load_seen() stores fallback keys from metadata.idx as "123", but main() checks fallback keys as "sample_123". That still skips dedup for files missing top-level id.
Suggested fix
def load_seen(path: str):
@@
- key = obj.get("id") or obj.get("metadata", {}).get("idx")
- if key is not None:
- seen.add(str(key))
+ key = obj.get("id")
+ if key is None:
+ idx = obj.get("metadata", {}).get("idx")
+ key = f"sample_{idx}" if idx is not None else None
+ if key is not None:
+ seen.add(str(key))Also applies to: 300-302
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/response_regeneration/script.py` around lines 119 - 121, load_seen()
currently adds fallback keys from metadata.idx as plain digits but main()
expects fallback keys in the form "sample_{idx}", causing dedup resume to fail;
update the logic where key = obj.get("id") or obj.get("metadata", {}).get("idx")
to normalize the fallback by wrapping numeric metadata.idx values as
f"sample_{idx}" before adding to the seen set (ensure the same normalization is
applied in all places that add to seen, including the other occurrence around
lines 300-302) so both load_seen() and main() use identical key formats.
1 participant
Purpose
This PR fixes a bug with the --resume flag where it does it correctly locate the ids of rows with already generated responses
Description
The load_seen() function tries to read:
key = obj.get("uuid") or obj.get("idx")
But the output file actually writes:
output = {
"id": item.get("uuid") or f"sample_{idx}", # Top-level "id", not "uuid"
}
This means that the response generation starts from the beginning as the ids do not exist
Related Issue
Tests
I have filled in: