-
Notifications
You must be signed in to change notification settings - Fork 1
Codex/add GitHub workflow for pytest on pr #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
AlexanderOnischenko
merged 27 commits into
main
from
codex/add-github-workflow-for-pytest-on-pr
Jan 23, 2026
Merged
Codex/add GitHub workflow for pytest on pr #110
AlexanderOnischenko
merged 27 commits into
main
from
codex/add-github-workflow-for-pytest-on-pr
Jan 23, 2026
Conversation
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
This reverts commit c737c77.
…etdefault and normalize text lists (#105) * Do not synthesize required_context in PlanNormalizer; always merge required_context via `setdefault` ### Motivation - Prevent silently overwriting baseline `ContextFetchSpec` entries by stopping the normalizer from fabricating specs for `required_context` and moving that responsibility to the merge layer. - Ensure baseline-aware materialization of missing providers so baseline selectors and modes are preserved. - Reduce confusing public API surface by removing now-dead `PlanNormalizerOptions` flags related to required-context synthesis. ### Description - Stop synthesizing `ContextFetchSpec` from `required_context` in `PlanNormalizer.normalize()` and add an explanatory comment that merge logic must be baseline-safe. - Remove the `fill_missing_context_plan` and `add_required_context_specs` fields from `PlanNormalizerOptions`. - Change `BaseGraphAgent._merge_baseline_with_plan` to always `setdefault` entries from `plan.required_context`, materializing missing providers as `ContextFetchSpec(provider=p, mode="full")` while preserving any baseline specs. - Add `tests/test_context_merge.py` with three regression tests exercising baseline preservation and required-context materialization, and make the materialization test less brittle by asserting individual fields rather than a full `model_dump`, and explicitly use an empty baseline list for clarity. ### Testing - Added unit tests in `tests/test_context_merge.py` covering baseline preservation when `context_plan` is missing or empty and materialization of `required_context` when no baseline exists. - An attempted test run (`pytest -k context_merge`) failed during collection due to an unrelated `ImportError: cannot import name 'NotRequired' from 'typing'` in `examples/demo_qa` on Python 3.10, so the new tests were not executed in this environment. - No further automated tests were run in this update. * Stop PlanNormalizer synthesizing required_context; always merge via setdefault and normalize text lists ### Motivation - Prevent overwriting baseline `ContextFetchSpec` entries by moving required-provider synthesis out of the normalizer and into the baseline/plan merge logic. - Ensure downstream code and templates always receive list values for `adr_queries` and `constraints` so they never get `None`. - Add regression coverage to guarantee that `required_context` is materialized even when `context_plan` is non-empty. ### Description - Removed synthesis of `ContextFetchSpec` from `PlanNormalizer.normalize()` and added a comment that baseline/plan merge must handle required providers in a baseline-safe way. - Removed unused flags from `PlanNormalizerOptions` related to filling/mutating `context_plan` and `required_context`. - Changed `BaseGraphAgent._merge_baseline_with_plan` to always `setdefault` entries from `plan.required_context`, materializing missing providers as `ContextFetchSpec(provider=p, mode="full")` while preserving any baseline specs. - Updated `PlanNormalizer._normalize_text_list` to always return a `List[str]` (returning an empty list when input is `None`), and used it for `adr_queries` and `constraints` so normalized plans never contain `None` for those fields. - Added `tests/test_context_merge.py` with four regression tests covering baseline preservation and required_context materialization, including the case where `context_plan` is non-empty. ### Testing - Added unit tests in `tests/test_context_merge.py` covering baseline preservation when `context_plan` is missing or empty, materialization of `required_context` when no baseline exists, and materialization when `context_plan` is non-empty. - No automated tests were executed as part of this update, so there are no test run results to report.
Feature/normalization
…sensitive-checks-in-runner.py Make expected checks case-insensitive and normalize list/set elements
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.