Fix/pr168 sidecar pair reverify - #177
Conversation
Closes fmagent-project#167 - Clarify required sidecar filename format in spec generation prompts (md/system_prompt.md, md/workflow_spec_step4_batch.md, src/generate_batch_prompts.py) - Add normalize_spec_filenames() to detect and rename sidecar files where the LLM omitted the source extension (src/file_utils.py) - Wire normalization into Stage 6 after spec generation completes (src/spec_generation_and_verification.py) - Report RuntimeError for ambiguous filename mappings - Add tests for normal, extension-drop, and ambiguous cases (tests/test_normalize_spec_filenames.py)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a46004a26f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
haoran-ding
left a comment
haoran-ding
left a comment
There was a problem hiding this comment.
It is not reasonable to directly raise AmbiguousSidecarError. If that error happens, the code should delete illegal files and re-generate sidecars rather than directly crash.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae31cf3161
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| replacements = ( | ||
| (alt_spec, expected_spec), | ||
| (alt_info, expected_info), |
There was a problem hiding this comment.
When only the canonical spec from a previous attempt is valid and the canonical info is missing or invalid, this branch replaces the spec before the info. In the normal streaming path, run_spec_generation_and_verification runs streaming_reasoner while these Futures are active, so is_file_ready() can have already read the old valid spec, then read the newly replaced info after this tuple is applied and submit verification for an old/new sidecar pair. Keep the canonical spec unreadable until both sidecars from the same attempt are visible, or stage the pair behind a single readiness marker.
Useful? React with 👍 / 👎.
3 participants
Summary
Closes #167 and builds on #168 by fixing the remaining timing, retry, and re-
verification issues around extension-dropped spec sidecars.
Some spec-generation runs write
foo.spec.json/foo.info.jsoninstead of theexpected
foo.py.spec.json/foo.py.info.json, causing false missing-spec retriesand preventing verification from completing.
Changes
extension.
.spec.json/.info.jsonpairs.streaming_reasonerto verify it while other batches are still running.generation attempts.
This keeps normalization and verification correctly ordered without introducing a
layer-wide synchronization barrier.