Skip to content

feat: Phase D1 replay split preservation and cartesian input-filtered case alignment - #62

Merged
txmed82 merged 4 commits into
mainfrom
feat/phase-d1-replay-split-alignment
Sep 9, 2026
Merged

feat: Phase D1 replay split preservation and cartesian input-filtered case alignment#62
txmed82 merged 4 commits into
mainfrom
feat/phase-d1-replay-split-alignment

Conversation

@txmed82

@txmed82 txmed82 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

What: Passes canonical split into replay_job; filters cartesian independent cases by actual evaluated input items and declared split order; ensures runner and cartesian case counts remain in lockstep.

Why: Prevent replay head mismatches on non-default splits and ensure cartesian independent case counting exactly reflects evaluated inputs rather than full split inventory.

Verify: Full test suite green (1569 passed, 10 skipped); new tests in tests/test_split_manifest.py; ruff + format + mypy strict clean.

@factory-droid

factory-droid Bot commented Sep 9, 2026

Copy link
Copy Markdown

Droid finished @txmed82's task —— View job


Security Review

Review summary: The split-preservation and cartesian alignment changes look correct and are well covered by tests. One small performance issue remains in cartesian filtering where set(split_items) is rebuilt inside the list comprehension.

Comment thread src/or_audit/eval/cartesian.py Outdated
evaluated_items = set(split_items[:trials] if trials is not None else split_items)
all_inputs = load_items(root / task.environment.inputs_path)
split_order = {item_id: idx for idx, item_id in enumerate(split_items)}
filtered_inputs = [item for item in all_inputs if str(item["id"]) in set(split_items)]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Avoid rebuilding the split-item set per input

filtered_inputs = [item for item in all_inputs if str(item["id"]) in set(split_items)] rebuilds set(split_items) for every input item, which can turn this into O(len(all_inputs)*len(split_items)) work and add avoidable allocations on large datasets; compute the set once and reuse it.

Suggested change
filtered_inputs = [item for item in all_inputs if str(item["id"]) in set(split_items)]
split_set = set(split_items)
filtered_inputs = [item for item in all_inputs if str(item["id"]) in split_set]

@txmed82

txmed82 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@droid review

@factory-droid

factory-droid Bot commented Sep 9, 2026

Copy link
Copy Markdown

Droid finished @txmed82's task —— View job


LGTM — I did not find any high-confidence, actionable issues in this diff. The split preservation and cartesian/input alignment changes look correct and are backed by tests.

@txmed82
txmed82 merged commit a3a5b0e into main Sep 9, 2026
5 checks passed
@txmed82
txmed82 deleted the feat/phase-d1-replay-split-alignment branch September 9, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants