Skip to content

Conversation

@AlexanderOnischenko
Copy link
Owner

No description provided.

AlexanderOnischenko and others added 28 commits January 23, 2026 12:03
…tures-from-events.jsonl

Guard planner_input dependency when emitting replay_point requires
…com:AlexanderOnischenko/fetchgraph into codex/reorganize-fixtures-and-migrate-paths
…com:AlexanderOnischenko/fetchgraph into codex/reorganize-fixtures-and-migrate-paths
…com:AlexanderOnischenko/fetchgraph into codex/reorganize-fixtures-and-migrate-paths
…tures-and-migrate-paths

Clarify fixture help defaults in Makefile
…com:AlexanderOnischenko/fetchgraph into codex/reorganize-fixtures-and-migrate-paths
…tures-and-migrate-paths

Codex/reorganize fixtures and migrate paths
@AlexanderOnischenko
Copy link
Owner Author

@codex, please review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d9076ec05a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 31 to 35
bucket_dir = FIXTURES_ROOT / bucket
if not bucket_dir.exists():
continue
for path in bucket_dir.rglob("*.json"):
paths.append((bucket, path))

Choose a reason for hiding this comment

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

P2 Badge Ignore resource JSONs when collecting replay fixtures

When a replay fixture is exported with --with-requires, export_replay_fixture/write_bundle copies dependent resource files into the same output directory, and those resources can be .json files. _iter_fixture_paths currently glob/rglob’s every *.json under tests/fixtures/replay_points/{fixed,known_bad}, so resource payloads get parameterized as fixtures and then test_replay_fixture fails at assert event.get("type") == "replay_point" because resource files don’t contain a replay payload. This breaks tests as soon as any fixture includes JSON resources; consider filtering by type in _iter_fixture_paths or storing resources in a subdirectory that isn’t globbed.

Useful? React with 👍 / 👎.

@AlexanderOnischenko
Copy link
Owner Author

@codex, please review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: be0ea0f3e2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 394 to 395
events_path = run_dir / "events.jsonl"
case_logger = event_logger.for_case(case.id, events_path) if event_logger else EventLogger(events_path, run_id, case.id)

Choose a reason for hiding this comment

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

P2 Badge Respect --events off when creating case logger

Because run_one always instantiates an EventLogger when event_logger is None, per-case events.jsonl emission becomes unconditional. That means batch.handle_batch’s --events off path (which intentionally passes event_logger=None) no longer disables event logging, and the agent still records events to disk; this can unexpectedly write large trace files or capture data that users explicitly opted out of logging. Consider only creating a case logger when event_logger is provided (or add an explicit flag) so the CLI semantics for --events still work as advertised.

Useful? React with 👍 / 👎.

…een-to-ensure-validation

fixture-green: validate by default, add --expected-from, rollback on mismatch with improved diffs
* Allow explicit disabling of event logging

* Refine event logger sentinel typing
* Fix relational selector validation

* Add replay validator checks for fixtures

* Use shared replay validators in known_bad tests

* Require replay validators in fixture-green and fixed tests
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