intake(phenokit-config-kit): fold from phenotype-sdk decomp Phase 3 - #47
Conversation
…rectories The rationalization absorb commit f5f4e75 registered 8 nested packages/* paths as mode 160000 (gitlink) entries without matching .gitmodules entries, and pointed them at commits that are no longer reachable in the consolidated object database. This caused every downstream consumer (notably thegent's `uv run pytest`) to fail with: fatal: no url found for submodule path 'packages/auth-kit/go' in .gitmodules Replace each phantom gitlink with a tracked .gitkeep directory placeholder so the path layout is preserved without dangling gitlink entries. Add a regression test in tests/test_gitlinks.py that fails whenever any (160000) gitlink appears in the index without a matching .gitmodules entry whose target commit is reachable. Refs: thegent AUDIT-N+39 dependency unblock.
`uv pip install -e <repo>` on the consolidated workspace failed with `error: Multiple top-level packages discovered in a flat-layout: ['okf', 'packages']`. This blocks external consumers from installing the workspace as a path source. Add explicit build-system configuration: `[tool.setuptools] py-modules = []` plus a no-op `[tool.setuptools.packages.find]` table that disables auto-discovery so the root project declares no installable packages. The root continues to act as a uv workspace aggregator under `[tool.uv] package = false`; per-kit installs still come from `[tool.uv.workspace] members`. Add regression test `tests/test_install_path.py` that runs `uv pip install` against the workspace path in a fresh temp venv and asserts: (a) the install exits 0, (b) the flat-layout error string never appears, (c) editable installs (`-e`) also succeed, (d) the pyproject.toml continues to declare the chosen fix. Pattern verified upstream in astral-sh/uv#12352.
Intake of the genuinely-new phenokit-config-kit package from the
phenotype-sdk Phase 3 decomposition. Per the rescued ORIGIN.md:
'Folded from KooshaPari/PhenoKits/libs/python/phenokit-config-kit
into KooshaPari/phenotype-python-sdk/packages/phenokit-config-kit
on 2026-06-20.'
'This package is intentionally kept distinct from packages/
phenotype-config because the source contains config-kit linting/
pre-commit/pytest scaffolds that are not file-equivalent to the
existing SDK config package.'
What was NOT brought in (corrected Phase 3 disposition):
- lang/python/mcp/agentmcp/ — REJECTED
Per agentmcp-hex/packages/agentmcp-hex/ORIGIN.md: 'agentmcp' was
the pre-rename version. It was already extracted to packages/
agentmcp-hex/ on 2026-06-18 with module rename to 'agentmcp_hex'
and version bump to 0.3.0. The SDK copy is the obsolete pre-rename
version. cli.py differs by 1 line (import sys + trailing newline).
Workspace integration:
- Added packages/phenokit-config-kit to [tool.uv.workspace] members
- Added phenokit-config-kit = { workspace = true } to [tool.uv.sources]
- 45 files, 6390 insertions
Bundle backup of original SDK content:
.preservation-work/2026-08-08-sdk-decomp/phenotype-sdk/
phenotype-sdk-all.bundle (sha256: aca92b82...)
Refs:
- plans/dockets/N00-phenotype-sdk-decomp-operator-summary.md
- plans/dockets/N03-phenotype-sdk-decomp-phase-3-python.md
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughSummaryThis PR adds The workspace packaging fix and regression tests address setuptools flat-layout discovery errors. The PR should not merge yet. Must Fix
Should Fix
Consider
Approve / Request ChangesRequest Changes WalkthroughThe pull request adds the ChangesConfig Kit package
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
codeant-ai
Bot
added
the
size:XXL
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| def random_uuid(self) -> str: | ||
| """Generate a random UUID.""" | ||
| return str(uuid.uuid4()) |
There was a problem hiding this comment.
Suggestion: The seed initializes the instance's private random generator, but random_uuid() uses the global uuid.uuid4() source instead. Two factories created with the same seed therefore produce different UUIDs, making generated records non-reproducible despite the factory's seeded API. [logic error]
Severity Level: Major ⚠️
- ⚠️ Seeded entity fixtures still vary between test runs.
- ⚠️ Snapshot and replay tests cannot reproduce identifiers.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/pytest/data/factory.py
**Line:** 37:39
**Comment:**
*Logic Error: The `seed` initializes the instance's private random generator, but `random_uuid()` uses the global `uuid.uuid4()` source instead. Two factories created with the same seed therefore produce different UUIDs, making generated records non-reproducible despite the factory's seeded API.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| "created_at": self.random_datetime(), | ||
| "updated_at": self.random_datetime(), |
There was a problem hiding this comment.
Suggestion: The factory independently generates created_at and updated_at across the same one-year range, so generated records can have an update timestamp earlier than their creation timestamp. This violates the usual temporal invariant and can cause tests involving ordering or update windows to fail intermittently; generate updated_at at or after created_at. [logic error]
Severity Level: Major ⚠️
- ⚠️ User and organization fixtures can violate timestamp ordering.
- ⚠️ Persistence and ordering tests may fail with invalid records.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/pytest/data/factory.py
**Line:** 85:86
**Comment:**
*Logic Error: The factory independently generates `created_at` and `updated_at` across the same one-year range, so generated records can have an update timestamp earlier than their creation timestamp. This violates the usual temporal invariant and can cause tests involving ordering or update windows to fail intermittently; generate `updated_at` at or after `created_at`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| def monitor(): | ||
| while True: | ||
| current_memory = psutil.Process().memory_info().rss / 1024 / 1024 | ||
| monitor_data["peak_memory"] = max(monitor_data["peak_memory"], current_memory) | ||
| time.sleep(0.1) | ||
|
|
||
| thread = threading.Thread(target=monitor, daemon=True) |
There was a problem hiding this comment.
Suggestion: The monitoring thread runs an unconditional while True loop and is never signaled to stop during fixture teardown. Every test that calls start_thread_monitoring() leaves a daemon thread alive for the rest of the process, retaining monitor_data and continuing to poll psutil; add a stop event and join the thread in the finally block. [resource leak]
Severity Level: Major ⚠️
- ⚠️ Performance fixture leaks one polling thread per invocation.
- ⚠️ Long test suites incur unnecessary CPU and process-state overhead.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/pytest/fixtures/performance.py
**Line:** 52:58
**Comment:**
*Resource Leak: The monitoring thread runs an unconditional `while True` loop and is never signaled to stop during fixture teardown. Every test that calls `start_thread_monitoring()` leaves a daemon thread alive for the rest of the process, retaining `monitor_data` and continuing to poll `psutil`; add a stop event and join the thread in the `finally` block.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| def monitor_memory(): | ||
| while item.nodeid in self.performance_data: | ||
| current_memory = psutil.Process().memory_info().rss / 1024 / 1024 | ||
| self.performance_data[item.nodeid]["peak_memory"] = max(self.performance_data[item.nodeid]["peak_memory"], current_memory) | ||
| time.sleep(0.1) | ||
|
|
||
| monitor_thread = threading.Thread(target=monitor_memory, daemon=True) |
There was a problem hiding this comment.
Suggestion: The plugin starts a daemon monitor thread for every marked test, but teardown never removes the test entry from performance_data. Consequently, the thread's while item.nodeid in self.performance_data condition remains true forever, leaving one polling thread per performance test for the lifetime of the pytest process; remove the entry or use an explicit stop event after teardown. [resource leak]
Severity Level: Major ⚠️
- ⚠️ Marked performance tests leak background monitor threads.
- ⚠️ Repeated test runs accumulate RSS polling work.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/pytest/plugins/performance.py
**Line:** 54:60
**Comment:**
*Resource Leak: The plugin starts a daemon monitor thread for every marked test, but teardown never removes the test entry from `performance_data`. Consequently, the thread's `while item.nodeid in self.performance_data` condition remains true forever, leaving one polling thread per performance test for the lifetime of the pytest process; remove the entry or use an explicit stop event after teardown.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| performance_items.append( | ||
| pytest.Function.from_parent( | ||
| parent=items[0].parent if items else None, | ||
| name="test_benchmark_performance", | ||
| callobj=self._test_benchmark_performance, | ||
| markers=[pytest.mark.performance, pytest.mark.benchmark], | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Suggestion: The plugin unconditionally creates synthetic pytest nodes even when collection produced no tests. In that case items[0].parent evaluates to None, and pytest.Function.from_parent cannot create a valid test node without a parent, causing pytest collection to fail for an otherwise empty or fully filtered test run. Return without adding nodes when items is empty, or attach them to a valid session/package node. [api mismatch]
Severity Level: Major ⚠️
- ❌ Empty or fully filtered pytest collections can error.
- ⚠️ CI commands selecting no tests may fail during collection.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/pytest/plugins/performance.py
**Line:** 68:75
**Comment:**
*Api Mismatch: The plugin unconditionally creates synthetic pytest nodes even when collection produced no tests. In that case `items[0].parent` evaluates to `None`, and `pytest.Function.from_parent` cannot create a valid test node without a parent, causing pytest collection to fail for an otherwise empty or fully filtered test run. Return without adding nodes when `items` is empty, or attach them to a valid session/package node.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| for test_id, data in self.performance_data.items(): | ||
| if "peak_memory" in data and data["peak_memory"] > self.memory_threshold: | ||
| violations.append(f"{test_id}: {data['peak_memory']:.2f}MB (threshold: {self.memory_threshold}MB)") |
There was a problem hiding this comment.
Suggestion: peak_memory is an absolute process RSS value in megabytes, but it is compared directly with a default threshold of 100. On a normal pytest process whose RSS is already above 100 MB, every generated memory validation test reports a violation even when the test caused no memory increase; compare the peak against the baseline or compare a peak delta against a delta threshold. [logic error]
Severity Level: Major ⚠️
- ❌ Valid performance tests can fail on larger pytest processes.
- ⚠️ Memory results depend on runner baseline RSS.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/pytest/plugins/performance.py
**Line:** 104:106
**Comment:**
*Logic Error: `peak_memory` is an absolute process RSS value in megabytes, but it is compared directly with a default threshold of `100`. On a normal pytest process whose RSS is already above 100 MB, every generated memory validation test reports a violation even when the test caused no memory increase; compare the peak against the baseline or compare a peak delta against a delta threshold.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| def load_test(users: int = 10, duration: int = 60) -> Callable: | ||
| """Decorator for marking functions as load tests.""" | ||
| return pytest.mark.performance(pytest.mark.parametrize("load_users", [users])(func)) |
There was a problem hiding this comment.
Suggestion: The decorator factory references func, which is not defined in its scope. Calling load_test(...) raises NameError immediately instead of returning a decorator; accept the decorated function as an inner wrapper or otherwise define and return a callable decorator. [null pointer]
Severity Level: Major ⚠️
- ❌ Load-test decorators cannot be created.
- ⚠️ Tests using `@load_test(...)` fail during module import.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/pytest/plugins/performance.py
**Line:** 143:145
**Comment:**
*Null Pointer: The decorator factory references `func`, which is not defined in its scope. Calling `load_test(...)` raises `NameError` immediately instead of returning a decorator; accept the decorated function as an inner wrapper or otherwise define and return a callable decorator.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| if item.nodeid in self.performance_data: | ||
| data = self.performance_data[item.nodeid] | ||
| data["end_time"] = time.time() | ||
| data["duration"] = data["end_time"] - data["start_time"] | ||
| data["end_memory"] = psutil.Process().memory_info().rss / 1024 / 1024 # MB | ||
| data["memory_delta"] = data["end_memory"] - data["start_memory"] |
There was a problem hiding this comment.
Suggestion: The performance entry remains in performance_data after teardown, so the monitor thread's while item.nodeid in self.performance_data condition never becomes false. Every performance-marked test therefore leaves a daemon polling thread running indefinitely and continues mutating its result dictionary after teardown. Remove the entry during teardown or use an explicit stop event and join the thread. [resource leak]
Severity Level: Major ⚠️
- ❌ Performance suites accumulate one polling thread per test.
- ⚠️ Long-running pytest processes retain completed test data.
- ⚠️ Background polling adds avoidable CPU and process overhead.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/plugins/performance.py
**Line:** 42:47
**Comment:**
*Resource Leak: The performance entry remains in `performance_data` after teardown, so the monitor thread's `while item.nodeid in self.performance_data` condition never becomes false. Every performance-marked test therefore leaves a daemon polling thread running indefinitely and continues mutating its result dictionary after teardown. Remove the entry during teardown or use an explicit stop event and join the thread.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
|
||
| def random_uuid(self) -> str: | ||
| """Generate a random UUID.""" | ||
| return str(uuid.uuid4()) |
There was a problem hiding this comment.
Suggestion: TestDataFactory(seed=...) does not make generated UUIDs deterministic because this method uses the module-level uuid.uuid4() entropy source rather than the factory's seeded random generator. Recreating a factory with the same seed therefore produces different IDs and breaks reproducible test data. [logic error]
Severity Level: Major ⚠️
- ⚠️ Seeded fixture records retain nondeterministic IDs.
- ⚠️ Snapshot and replay tests cannot reproduce complete data.
- ⚠️ Related entity references vary across repeated runs.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/data/factory.py
**Line:** 39:39
**Comment:**
*Logic Error: `TestDataFactory(seed=...)` does not make generated UUIDs deterministic because this method uses the module-level `uuid.uuid4()` entropy source rather than the factory's seeded random generator. Recreating a factory with the same seed therefore produces different IDs and breaks reproducible test data.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| "roles": self.random_choices(["user", "admin", "moderator"], k=self.random_int(1, 3)), | ||
| "created_at": self.random_datetime(), |
There was a problem hiding this comment.
Suggestion: created_at and updated_at are sampled independently, so generated records can have an update timestamp earlier than their creation timestamp. Generate the creation time first and constrain the update time to be at or after it when producing lifecycle data. [logic error]
Severity Level: Major ⚠️
- ⚠️ Lifecycle assertions can reject generated records.
- ⚠️ Sorting and synchronization tests receive invalid chronology.
- ⚠️ All entity factory methods share this timestamp pattern.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/data/factory.py
**Line:** 84:85
**Comment:**
*Logic Error: `created_at` and `updated_at` are sampled independently, so generated records can have an update timestamp earlier than their creation timestamp. Generate the creation time first and constrain the update time to be at or after it when producing lifecycle data.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| def measure(name: str, **kwargs): | ||
| current_time = time.time() | ||
| current_memory = psutil.Process().memory_info().rss / 1024 / 1024 |
There was a problem hiding this comment.
Suggestion: The fixture exposes measure as a normal function that immediately returns a dictionary, but the documented usage passes it to with. That produces a context-manager protocol error; either expose the existing context-manager implementation or make measure a context manager. [api mismatch]
Severity Level: Major ⚠️
- ❌ Documented performance tests fail before measured code runs.
- ❌ Performance fixture context-manager usage is unusable.
- ⚠️ Users must bypass the documented measurement API.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py
**Line:** 32:34
**Comment:**
*Api Mismatch: The fixture exposes `measure` as a normal function that immediately returns a dictionary, but the documented usage passes it to `with`. That produces a context-manager protocol error; either expose the existing context-manager implementation or make `measure` a context manager.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| while True: | ||
| current_memory = psutil.Process().memory_info().rss / 1024 / 1024 | ||
| monitor_data["peak_memory"] = max(monitor_data["peak_memory"], current_memory) | ||
| time.sleep(0.1) | ||
|
|
There was a problem hiding this comment.
Suggestion: The background monitor has no stop condition tied to fixture teardown. Once start_thread_monitoring() is called, the daemon thread loops forever and retains monitor_data through its closure, accumulating one permanent thread per invocation. Add a stop event and join the thread in the fixture's finally block. [resource leak]
Severity Level: Major ⚠️
- ⚠️ Repeated performance tests accumulate monitoring threads.
- ⚠️ Threads retain fixture state after teardown.
- ⚠️ Long test processes incur unnecessary monitoring overhead.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py
**Line:** 53:57
**Comment:**
*Resource Leak: The background monitor has no stop condition tied to fixture teardown. Once `start_thread_monitoring()` is called, the daemon thread loops forever and retains `monitor_data` through its closure, accumulating one permanent thread per invocation. Add a stop event and join the thread in the fixture's `finally` block.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| architecture_items.append( | ||
| pytest.Function.from_parent( | ||
| parent=items[0].parent if items else None, | ||
| name="test_import_boundaries", |
There was a problem hiding this comment.
Suggestion: When pytest collects no ordinary tests, items[0] is unavailable and this creates a synthetic Function with parent=None. Pytest requires a real collector parent, so empty or fully filtered collections fail during collection instead of completing normally. [api mismatch]
Severity Level: Major ⚠️
- ❌ Empty or fully filtered pytest runs fail during collection.
- ⚠️ CI jobs cannot complete cleanly with no selected tests.
- ⚠️ Architecture checks prevent normal pytest exit behavior.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/plugins/architecture.py
**Line:** 42:42
**Comment:**
*Api Mismatch: When pytest collects no ordinary tests, `items[0]` is unavailable and this creates a synthetic `Function` with `parent=None`. Pytest requires a real collector parent, so empty or fully filtered collections fail during collection instead of completing normally.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| for root, dirs, files in Path().rglob("*.py"): | ||
| # Skip certain directories | ||
| if any(skip in str(root) for skip in (".git", "__pycache__", ".pytest_cache", "htmlcov", "dist", "build")): | ||
| continue |
There was a problem hiding this comment.
Suggestion: Path.rglob() yields one Path object per iteration, not (root, dirs, files) tuples. This loop therefore raises an unpacking ValueError whenever a security scan calls _get_python_files; iterate directly over the returned paths instead. [incorrect variable usage]
Severity Level: Critical 🚨
- ❌ Security scans crash before inspecting source files.
- ❌ Vulnerability and secret checks cannot execute.
- ⚠️ Security pytest jobs fail with an internal collection error.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/plugins/security.py
**Line:** 202:205
**Comment:**
*Incorrect Variable Usage: `Path.rglob()` yields one `Path` object per iteration, not `(root, dirs, files)` tuples. This loop therefore raises an unpacking `ValueError` whenever a security scan calls `_get_python_files`; iterate directly over the returned paths instead.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| return python_files | ||
|
|
||
|
|
||
| def security_test(func: Callable) -> Callable: |
There was a problem hiding this comment.
Suggestion: The Callable type is not imported, so evaluating these annotations during pytest plugin discovery raises NameError and prevents the security plugin from loading. Import Callable from collections.abc or defer annotation evaluation. [import error]
Severity Level: Critical 🚨
- ❌ Pytest startup fails when security plugin is discovered.
- ❌ Security scanning and security markers become unavailable.
- ⚠️ Test suites using the package cannot collect normally.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/config_kit/pytest/plugins/security.py
**Line:** 211:211
**Comment:**
*Import Error: The `Callable` type is not imported, so evaluating these annotations during pytest plugin discovery raises `NameError` and prevents the security plugin from loading. Import `Callable` from `collections.abc` or defer annotation evaluation.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| # Quality Gate Script for config-kit | ||
| # This script runs all quality checks and exits with non-zero if any fail | ||
|
|
||
| set -e |
There was a problem hiding this comment.
Suggestion: Because set -e is enabled, the intentional return 1 from a failed run_check aborts the caller immediately. The first failed check exits the script before later checks and before the summary, contradicting the aggregate quality-gate behavior. Run checks in a conditional context or disable errexit around the check invocation while preserving the failure count. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Later quality checks are skipped after one failure.
- ❌ Aggregate pass/fail summary is not printed.
- ⚠️ CI receives incomplete quality-gate diagnostics.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh
**Line:** 6:6
**Comment:**
*Incorrect Condition Logic: Because `set -e` is enabled, the intentional `return 1` from a failed `run_check` aborts the caller immediately. The first failed check exits the script before later checks and before the summary, contradicting the aggregate quality-gate behavior. Run checks in a conditional context or disable errexit around the check invocation while preserving the failure count.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| run_linting_checks() { | ||
| print_header "SECTION" "Linting Checks" | ||
|
|
||
| run_check "Ruff" "ruff check --exit-zero" "Ruff passed" "Ruff failed" |
There was a problem hiding this comment.
Suggestion: Passing --exit-zero forces Ruff to return success even when it finds lint violations, so this quality check can never fail on Ruff errors. Remove the flag or explicitly inspect Ruff's reported violations. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Ruff violations do not fail the quality gate.
- ⚠️ CI reports false-positive lint success.
- ⚠️ Formatting and other checks remain the only lint enforcement.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh
**Line:** 100:100
**Comment:**
*Incorrect Condition Logic: Passing `--exit-zero` forces Ruff to return success even when it finds lint violations, so this quality check can never fail on Ruff errors. Remove the flag or explicitly inspect Ruff's reported violations.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| # Check if we're in the right directory | ||
| if [ ! -f "pyproject.toml" ] || [ ! -d "src" ]; then | ||
| print_error "This script must be run from the config-kit root directory" | ||
| exit 1 |
There was a problem hiding this comment.
Suggestion: The script is intended to run from the package root, which contains config_kit rather than src. This initial guard therefore exits before main can run, and the later checks also target the nonexistent src/ directory. Use the actual package source directory or determine it from the project configuration. [logic error]
Severity Level: Critical 🚨
- ❌ Documented quality-gate invocation exits immediately.
- ❌ Lint, security, coverage, and dependency checks never run.
- ❌ CI cannot use the provided package quality gate.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh
**Line:** 215:218
**Comment:**
*Logic Error: The script is intended to run from the package root, which contains `config_kit` rather than `src`. This initial guard therefore exits before `main` can run, and the later checks also target the nonexistent `src/` directory. Use the actual package source directory or determine it from the project configuration.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![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: 90ad8d37d0
ℹ️ 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".
| def _get_python_files(self) -> list[Path]: | ||
| """Get all Python files in the project.""" | ||
| python_files = [] | ||
| for root, dirs, files in Path().rglob("*.py"): |
There was a problem hiding this comment.
When the security plugin is loaded, every generated security check calls _get_python_files, but Path().rglob("*.py") yields a single Path, not (root, dirs, files), so even a trivial pytest run with this plugin fails with TypeError: cannot unpack non-iterable PosixPath object before any scan can run. Iterate the returned paths directly or switch this helper to os.walk like the architecture plugin.
Useful? React with 👍 / 👎.
| [tool.setuptools.package-data] | ||
| config_kit = [ |
There was a problem hiding this comment.
This package is built with hatchling.build, so the tool.setuptools.package-data table is ignored; combined with the wheel target that only includes config_kit, the sibling pre-commit/, pytest/*.ini, and linting/ assets will be absent from the installed wheel. Consumers following the advertised pip install phenokit-config-kit flow therefore won't have the configuration files this kit is meant to distribute, so these assets need Hatchling include/force-include rules or to live inside the package.
Useful? React with 👍 / 👎.
| [project.entry-points.pytest11] | ||
| config_kit_architecture = "config_kit.pytest.plugins.architecture" | ||
| config_kit_performance = "config_kit.pytest.plugins.performance" | ||
| config_kit_security = "config_kit.pytest.plugins.security" |
There was a problem hiding this comment.
These pytest11 entry points are advertised for every base install, but the project has no required dependencies and puts pytest/psutil only in the optional testing extra. In an environment that already has pytest and installs phenokit-config-kit without [testing], pytest will still auto-load these entry points and config_kit.pytest.plugins.performance imports psutil at module import time, causing pytest startup to fail before collection; either make the plugin runtime deps mandatory/lazy or avoid publishing unconditional pytest entry points for the base install.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 56
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/phenokit-config-kit/pytest/plugins/architecture.py (1)
1-272: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep the root-level
pytest/tree as non-package assets only.
pytest11referencesconfig_kit.pytest.*, andtool.hatchpackages onlyconfig_kit, so the current top-level Python modules underpytest/are unused. The Python files inpytest/data/,pytest/fixtures/, andpytest/plugins/are hash-identical toconfig_kit/pytest/..., and the top-levelpytest/package does not importpytestor register plugins. Remove the duplicate root-level Python modules, or rename/remove the root-level package and ensure pytest config files are packaged fromconfig_kit/pytest.🤖 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 `@packages/phenokit-config-kit/pytest/plugins/architecture.py` around lines 1 - 272, Remove the duplicate root-level Python modules under packages/phenokit-config-kit/pytest, including plugins/architecture.py, plugins/security.py, and data/factory.py; apply the same removal to the remaining duplicate pytest/data, pytest/fixtures, and pytest/plugins modules. Preserve pytest configuration assets by packaging them from config_kit/pytest, and ensure the root-level pytest tree contains only non-package assets.
🤖 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 `@packages/phenokit-config-kit/config_kit/__init__.py`:
- Around line 11-16: Ensure every name listed in __all__ is bound to a valid
symbol across all affected initializers: in
packages/phenokit-config-kit/config_kit/__init__.py, add supported submodules or
remove unsupported linting and pre_commit names; in
packages/phenokit-config-kit/config_kit/pytest/data/__init__.py, import
TestDataFactory, TestDataGenerator, TestDataManager, and TestDataValidator; in
packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py, import
every declared fixture including cache_fixture, database_fixture, and
message_broker_fixture; and in
packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py, import the
exported plugin classes or change __all__ to the actual plugin module names.
In `@packages/phenokit-config-kit/config_kit/pytest/data/factory.py`:
- Around line 37-39: Update random_uuid() to generate UUID values from
self._random instead of the module-level random source, and update the default
datetime-bound handling in the related factory method around lines 61–70 to use
an injected or fixed reference time when callers omit bounds. Preserve explicit
caller-provided bounds while ensuring identical seeds produce identical entity
data across runs.
In `@packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py`:
- Around line 165-175: Update the performance context manager’s finally logic in
both
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py:165-175
and packages/phenokit-config-kit/pytest/fixtures/performance.py:165-175 so
threshold enforcement occurs only when the wrapped block completes successfully;
preserve any exception raised by the measured block instead of replacing it with
pytest.fail(), while retaining the existing failure for slow successful blocks.
- Around line 50-60: Implement a stop lifecycle for every performance monitor
thread: in
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py lines
50-60 and packages/phenokit-config-kit/pytest/fixtures/performance.py lines
50-60, store a per-monitor stop event, make the monitor exit when signaled, and
have fixture teardown signal and join the thread; in
packages/phenokit-config-kit/pytest/plugins/performance.py lines 53-61, store a
per-test stop event and signal and join the monitor from
pytest_runtest_teardown.
In `@packages/phenokit-config-kit/config_kit/pytest/fixtures/security.py`:
- Around line 83-84: Update the MD5 and SHA-1 calls in the fixture data
construction to pass usedforsecurity=False, preserving the existing test-string
inputs and hexadecimal digest output so the fixture works with FIPS-enabled
OpenSSL.
- Around line 16-19: Update the security_context fixture annotation from
Generator to the direct dictionary return type used by the fixture, and remove
the Generator import if no other fixture references it.
In `@packages/phenokit-config-kit/config_kit/pytest/plugins/architecture.py`:
- Around line 153-156: Update the line-count logic in the architecture plugin’s
Python-file loop to open each file within an explicit context manager, then
compute line_count from that managed handle so it is closed deterministically
after counting.
- Around line 181-192: Update _get_python_files to skip .venv, venv, .tox,
.mypy_cache, .ruff_cache, and node_modules in addition to the existing
directories. Initialize _python_files_cache in __init__, return the cached
result on subsequent calls, and populate it after the first directory walk so
the architecture tests scan once.
- Around line 194-197: Move the re import from _is_allowed_import to module
scope alongside the existing ast import, and precompile the constant allowed
patterns when they are defined rather than recompiling them for each import
check. Update _is_allowed_import to reuse those compiled patterns while
preserving its current matching behavior.
- Around line 255-266: Update the pytest option definitions for
--enforce-imports and --enforce-dependencies to support explicit disabling,
rather than using store_true with default=True; ensure the corresponding
config.getoption calls in the plugin read enforce_imports and
enforce_dependencies so the existing guard can observe disabled checks.
- Around line 232-236: Update the complexity traversal around the visible
ast.walk loop to use one isinstance check for all branch-node types, including
ast.IfExp, ast.Assert, ast.Match, and comprehension if clauses, while preserving
the BoolOp increment. Prevent nested FunctionDef and AsyncFunctionDef bodies
from contributing to the enclosing function’s score by skipping their subtrees
during traversal.
- Around line 123-126: Normalize each path returned by _get_python_files before
the src/ prefix check in the dependency-direction loop, using Path normalization
and a POSIX representation as appropriate for the repository layout. Ensure
valid files such as ./src/domain/model.py are inspected while files outside src/
remain skipped.
- Around line 38-78: Update pytest_collection_modifyitems in
packages/phenokit-config-kit/config_kit/pytest/plugins/architecture.py (lines
38-78), performance.py (lines 63-87), and security.py (lines 81-126) to return
immediately when items is empty, create each generated pytest.Function before
applying markers, and use item.add_marker(...) instead of the unsupported
markers= argument. Register every custom marker through pytest_configure using
config.addinivalue_line("markers", ...), preserving the existing marker names
and generated-test behavior.
In `@packages/phenokit-config-kit/config_kit/pytest/plugins/performance.py`:
- Around line 112-113: Rename the context manager function performance_monitor
in the performance monitoring module to a non-conflicting name such as
monitor_performance, and update all references to it while preserving its
existing behavior. Leave the pytest fixture named performance_monitor unchanged.
- Around line 143-145: Update load_test to return a decorator that accepts the
target function, then applies the performance and load_users parametrization
marks to that function before returning it. Preserve the users parameter
behavior and incorporate duration through the existing decorator metadata or
parametrization mechanism so the argument is no longer unused.
- Around line 118-130: Update the performance wrapper around the try/finally
block so the finally clause only records end_time, end_memory, duration, and
memory_delta, then prints the measurement before threshold validation. Move the
duration threshold check after the try/finally, ensuring it runs only when the
wrapped block completes successfully and cannot replace its original exception.
- Around line 63-87: Update pytest_collection_modifyitems to append the
generated benchmark and memory validation items only when the collected items
include at least one performance-marked test; otherwise leave items unchanged.
Also align the generated pytest.Function construction with the established
architecture.py fix by providing a valid parent and applying markers without the
unsupported duplicate markers argument.
- Around line 49-61: Update the performance monitoring lifecycle across
pytest_runtest_call and pytest_runtest_teardown: create an explicit per-test
stop event, have the monitor thread wait on it instead of checking
performance_data, and store the thread/event handles for teardown. Reuse a
single psutil.Process() instance for polling, then signal the event and join the
thread before teardown reads or writes peak_memory, end_memory, and
memory_delta.
In `@packages/phenokit-config-kit/config_kit/pytest/plugins/security.py`:
- Around line 128-142: Refactor the security checks around
_test_vulnerability_scan, _test_forbidden_imports, _test_hardcoded_secrets, and
_test_password_security to enumerate and read each Python file once, then apply
all rule sets to the cached content. Update hardcoded-secret and
password-security violations to report only the file path and matched line
number, never interpolating matches or captured secret values. Preserve the
existing rule detection behavior and failure aggregation.
- Around line 155-162: Update the security scan’s forbidden-import logic in the
AST walk to bind the forbidden-import rules once, match imported module roots so
entries such as “os” also catch “os.path”, and inspect imported names for `from
... import ...` cases such as `os.system`. Remove the non-module entries `eval`,
`exec`, and `compile` from the import rules unless you implement the separate
`forbidden_functions` call check; also wire in or remove the currently unused
`forbidden_functions` and `required_imports` rule sets as appropriate.
- Around line 199-208: Fix _get_python_files by iterating each Path yielded
directly by Path().rglob("*.py") instead of unpacking it as root, dirs, and
files. Filter paths by exact directory components using parts, excluding .git,
__pycache__, .pytest_cache, htmlcov, dist, build, .venv, venv, .tox, and
node_modules, then append the file path.
- Around line 69-79: Update _load_vulnerability_patterns so weak_crypto uses
word boundaries and matches cryptographic call sites rather than substrings in
ordinary words; also require path_traversal matches a standalone traversal
segment instead of any ../ text in imports or documentation. Keep the existing
pattern interface compatible with the scanner, and reuse AST-based detection
where appropriate, consistent with _test_forbidden_imports.
- Around line 211-238: Add the missing Callable import from collections.abc
alongside the imports in the security plugin so the annotations in
security_test, auth_test, injection_test, xss_test, csrf_test, and
sql_injection_test resolve during module import. Follow the existing import
pattern used by the sibling performance plugin.
In `@packages/phenokit-config-kit/linting/ci-cd/gatekeeper.toml`:
- Around line 45-49: Update the forbidden_patterns entries in gatekeeper.toml to
use TOML-compatible escaping: convert the regex strings to literal strings or
double each regex backslash in the existing basic strings. Preserve the current
regular-expression patterns and their matching behavior.
- Around line 83-88: Update the require_all_gates setting in the reporting
configuration to true, aligning gatekeeper.toml with the merge-gate contract and
preventing optional failing gates.
In `@packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh`:
- Around line 215-219: Update the directory precondition and all subsequent
source-directory command arguments in quality-gate.sh to use config_kit instead
of src, while preserving the existing validation and error behavior.
- Line 100: Update the Ruff command passed to run_check in the quality-gate
script by removing the --exit-zero option, so Ruff violations produce a failing
exit status and are recorded as failed checks.
- Around line 143-167: Update the successful branches of the file-size check and
complexity check in the quality-gate script to increment TOTAL before
incrementing PASSED, ensuring both metric checks are counted regardless of
outcome.
- Around line 78-87: Update the failure branch of run_check so it still
increments FAILED, reports the failure, and records the failed check, but
returns 0 instead of 1. Preserve main’s aggregate-result handling so all checks
and the final summary execute before the overall status is returned.
In `@packages/phenokit-config-kit/linting/git-hooks/commit-msg`:
- Around line 82-94: Remove wip from the accepted type alternatives in
check_conventional_commits so WIP messages reach check_commit_message and are
rejected according to the hook policy.
In `@packages/phenokit-config-kit/linting/git-hooks/pre-commit`:
- Around line 30-37: Update get_staged_py_files, get_staged_files, and the
related linter invocation blocks to preserve staged pathnames containing
whitespace or newlines by using NUL-delimited git diff output with arrays or
mapfile -d ''. Pass filenames safely with -- before file arguments so pathnames
beginning with '-' are not treated as options.
- Around line 84-123: Update lint_all_files to partition the staged files by
type before invoking each linter: pass only Python files to bandit, YAML files
to yamllint, Markdown files to markdownlint, and shell files to shellcheck.
Preserve the existing command-availability checks and skip behavior, and avoid
invoking a linter when its filtered file list is empty.
In `@packages/phenokit-config-kit/linting/linters.toml`:
- Around line 100-101: Update the pyupgrade command in the linters.pyupgrade
configuration from the Python 3.7 target to the Python 3.11 target, preserving
the existing --keep-runtime-typing option.
In `@packages/phenokit-config-kit/linting/pre-commit-hooks.yaml`:
- Around line 129-134: Update the commit-message-check hook definition to
declare stages: [commit-msg] and remove its files filter; ensure installation
includes the commit-msg hook type via pre-commit install --hook-type commit-msg
while preserving the existing entry and arguments.
In `@packages/phenokit-config-kit/linting/README.md`:
- Around line 101-104: Update the GitHub Actions workflow example in the
quality-gate section so the command executes from the phenokit-config-kit
package directory: set working-directory to packages/phenokit-config-kit before
running quality-gate.sh, or use the equivalent full package-relative script
path.
- Around line 106-115: Update the “Upload quality reports” README workflow
example to use actions/upload-artifact@v4 instead of the retired v3 action,
preserving the existing artifact name, paths, and always() condition.
In `@packages/phenokit-config-kit/pre-commit/comprehensive.yaml`:
- Around line 10-58: Remove the repeated hook entries from the pre-commit
configuration, retaining exactly one declaration for each hook ID such as
check-merge-conflict, check-yaml, check-json, and the other listed hooks.
Preserve the existing arguments on check-added-large-files and the remaining
unique hook declarations.
- Around line 95-97: Prevent duplicate Bandit inputs in both repository-wide
hooks: update the Bandit configuration at
packages/phenokit-config-kit/pre-commit/comprehensive.yaml:95-97 and
packages/phenokit-config-kit/pre-commit/security.yaml:38-40 to disable
pre-commit filename passing with pass_filenames: false, or remove the recursive
-r . scan mode; apply the same consistent approach to both sites.
In `@packages/phenokit-config-kit/pyproject.toml`:
- Around line 80-97: Update the Hatchling wheel configuration so the documented
pre-commit, pytest, and linting assets are included in the built package. Either
move those directories under config_kit and expose them via the installed
package, or add Hatchling force-include mappings for each into
config_kit/resources; remove reliance on tool.setuptools.package-data, which
Hatchling ignores.
In `@packages/phenokit-config-kit/pytest/ci.ini`:
- Around line 77-88: Remove enclosing TOML-style quotes from the INI scalar
values in packages/phenokit-config-kit/pytest/ci.ini lines 77-88,
comprehensive.ini lines 129-140, performance.ini lines 56-64, and security.ini
lines 67-75: leave timeout_method, junit_family, and junit_logging as unquoted
values, and remove only the outer quotes from log_cli_format and
log_cli_date_format while preserving their percent tokens and square brackets.
In `@packages/phenokit-config-kit/pytest/data/__init__.py`:
- Around line 8-13: Update the package initializer’s __all__ to expose only the
existing TestDataFactory symbol, import TestDataFactory from factory.py, and
remove TestDataGenerator, TestDataManager, and TestDataValidator.
In `@packages/phenokit-config-kit/pytest/data/factory.py`:
- Line 26: Update the affected parameter annotations in the factory methods,
including random_string and the methods containing start, end, organization_id,
project_id, document_id, source_id, and target_id, from non-optional types to
the Python 3.11+ X | None form while preserving their existing None defaults and
behavior.
- Around line 37-39: Update TestDataFactory.random_uuid to derive UUID values
from self._random rather than uuid.uuid4(), ensuring seeded factories produce
deterministic IDs while preserving the string UUID return format.
- Around line 201-225: Refactor create_related_data to replace the repeated
entity_type if/elif chain with a dispatch mapping from each supported entity
name to its corresponding factory method. Validate and retrieve the selected
factory once before the loop, raise ValueError for unknown types, then invoke
that factory for each requested item while preserving overrides and return
behavior.
- Around line 15-17: Mark the TestDataFactory dataclass as non-test code so
pytest does not collect it under the default Test* class pattern; preserve its
existing constructor and factory behavior while applying the repository’s
supported pytest non-collection marker.
In `@packages/phenokit-config-kit/pytest/fixtures/__init__.py`:
- Around line 9-18: Update __all__ in the fixtures package to list only fixture
callables that are actually defined or imported, including the available
mock_cache, mock_database, and mock_message_broker symbols or their existing
_fixture aliases. Ensure every advertised name resolves when the package is
imported.
In `@packages/phenokit-config-kit/pytest/parallel.ini`:
- Line 12: Update the usage comment to replace the invalid --ini option with
pytest’s -c/--config-file option and reference the actual parallel.ini basename
from the pytest configuration directory, preserving the existing marker
expression.
In `@packages/phenokit-config-kit/pytest/plugins/__init__.py`:
- Around line 9-15: Update the package exports around __all__ so they reference
only plugin classes that are actually imported and available, including
ArchitecturePlugin, PerformancePlugin, and SecurityPlugin once security.py
imports successfully. Remove CoveragePlugin and ReportingPlugin until
corresponding modules and imports exist, and add explicit imports for every name
retained in __all__ to make wildcard imports valid.
In `@packages/phenokit-config-kit/pytest/plugins/performance.py`:
- Around line 53-61: Update the memory-monitor lifecycle around monitor_memory
and monitor_thread to use a per-test stop event instead of relying on
performance_data membership. Store the event and thread per test, have the loop
exit when signaled, and signal then join the thread from pytest_runtest_teardown
before retaining performance_data for validation.
- Around line 143-145: Update load_test to return an inner decorator that
accepts func before applying the performance and parametrize markers,
eliminating the undefined reference. Preserve users as the load_users parameter,
and include duration in the generated test parameters if the load_test API
continues to expose it.
- Around line 63-87: Update pytest_collection_modifyitems to return immediately
when items is empty, before constructing performance_items or calling
pytest.Function.from_parent. Preserve adding the benchmark and memory validation
tests with items[0].parent for non-empty collections.
In `@packages/phenokit-config-kit/README.md`:
- Line 45: Fix the Markdown lint violations in the README by adding required
blank lines around headings and fenced code blocks, and specify an appropriate
language identifier for the project-structure fenced block. Apply the same
formatting corrections to the additional referenced sections.
- Around line 51-60: Replace the invalid --ini pytest arguments with -c
configuration-file selection in packages/phenokit-config-kit/README.md at lines
51-60 and 210-214, and packages/phenokit-config-kit/pytest/README.md at lines
18-40, 541-549, and 585-593. Update every specified command, preserving each
existing configuration path and using -o only for INI value overrides.
In `@tests/test_install_path.py`:
- Around line 104-109: Update the CI packaging job that runs
tests/test_install_path.py to install uv before testing and fail explicitly when
the uv CLI is unavailable. Remove or bypass the module-level pytestmark skipif
in tests/test_install_path.py for this CI path so missing uv cannot produce a
passing packaging run.
- Around line 53-70: Update the venv creation flow around the subprocess.run
call to pass timeout=_INSTALL_TIMEOUT_SECS, and wrap the uv venv invocation and
return-code assertion in exception-safe cleanup that removes tempdir when
creation fails or raises. Preserve successful tempdir usage for the returned
virtual environment.
- Around line 165-179: Update test_pyproject_declares_empty_tool_setuptools to
parse PYPROJECT with tomllib and validate the [tool.setuptools] configuration
values directly: py-modules must equal [], packages.find.include must equal [],
and packages.find.namespaces must equal False. Replace the current
source-substring assertions while preserving clear failure messages.
---
Outside diff comments:
In `@packages/phenokit-config-kit/pytest/plugins/architecture.py`:
- Around line 1-272: Remove the duplicate root-level Python modules under
packages/phenokit-config-kit/pytest, including plugins/architecture.py,
plugins/security.py, and data/factory.py; apply the same removal to the
remaining duplicate pytest/data, pytest/fixtures, and pytest/plugins modules.
Preserve pytest configuration assets by packaging them from config_kit/pytest,
and ensure the root-level pytest tree contains only non-package assets.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2318ffe2-cbb1-4b1d-b736-ba1c8c3f39cd
📒 Files selected for processing (49)
WORKLOG.mdpackages/phenokit-config-kit/ORIGIN.mdpackages/phenokit-config-kit/README.mdpackages/phenokit-config-kit/config_kit/__init__.pypackages/phenokit-config-kit/config_kit/pytest/__init__.pypackages/phenokit-config-kit/config_kit/pytest/data/__init__.pypackages/phenokit-config-kit/config_kit/pytest/data/factory.pypackages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.pypackages/phenokit-config-kit/config_kit/pytest/fixtures/common.pypackages/phenokit-config-kit/config_kit/pytest/fixtures/performance.pypackages/phenokit-config-kit/config_kit/pytest/fixtures/security.pypackages/phenokit-config-kit/config_kit/pytest/plugins/__init__.pypackages/phenokit-config-kit/config_kit/pytest/plugins/architecture.pypackages/phenokit-config-kit/config_kit/pytest/plugins/performance.pypackages/phenokit-config-kit/config_kit/pytest/plugins/security.pypackages/phenokit-config-kit/linting/.editorconfigpackages/phenokit-config-kit/linting/.markdownlint.jsonpackages/phenokit-config-kit/linting/README.mdpackages/phenokit-config-kit/linting/ci-cd/gatekeeper.tomlpackages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.shpackages/phenokit-config-kit/linting/git-hooks/commit-msgpackages/phenokit-config-kit/linting/git-hooks/pre-commitpackages/phenokit-config-kit/linting/linters.tomlpackages/phenokit-config-kit/linting/pre-commit-hooks.yamlpackages/phenokit-config-kit/linting/pyproject.tomlpackages/phenokit-config-kit/pre-commit/basic.yamlpackages/phenokit-config-kit/pre-commit/comprehensive.yamlpackages/phenokit-config-kit/pre-commit/security.yamlpackages/phenokit-config-kit/pyproject.tomlpackages/phenokit-config-kit/pytest/README.mdpackages/phenokit-config-kit/pytest/basic.inipackages/phenokit-config-kit/pytest/ci.inipackages/phenokit-config-kit/pytest/comprehensive.inipackages/phenokit-config-kit/pytest/data/__init__.pypackages/phenokit-config-kit/pytest/data/factory.pypackages/phenokit-config-kit/pytest/fixtures/__init__.pypackages/phenokit-config-kit/pytest/fixtures/common.pypackages/phenokit-config-kit/pytest/fixtures/performance.pypackages/phenokit-config-kit/pytest/fixtures/security.pypackages/phenokit-config-kit/pytest/parallel.inipackages/phenokit-config-kit/pytest/performance.inipackages/phenokit-config-kit/pytest/plugins/__init__.pypackages/phenokit-config-kit/pytest/plugins/architecture.pypackages/phenokit-config-kit/pytest/plugins/performance.pypackages/phenokit-config-kit/pytest/plugins/security.pypackages/phenokit-config-kit/pytest/security.inipyproject.tomltests/__init__.pytests/test_install_path.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: semgrep-cloud-platform/scan
⚠️ CI failures not shown inline (2)
GitHub Check: Summary: The current Mergify configuration is invalid
Conclusion: failure
* Invalid condition 'author=dependabot[bot] | renovate[bot]' @ root → pull_request_rules → item 1 → conditions → item 0 → author=dependabot[bot] | renovate[bot]
```
Invalid GitHub login
```
* Invalid condition 'author=trunk-io[bot] | mergify[bot] | github-actions[bot]' @ root → pull_request_rules → item 2 → conditions → item 0 → author=trunk-io[bot] | mergify[bot] | github-actions[bot]
```
Invalid GitHub login
```
* Invalid condition 'age>=30d' @ root → pull_request_rules → item 8 → conditions → item 2 → age>=30d
```
Invalid attribute
```
* Extra inputs are not permitted @ root → pull_request_rules → item 0 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 1 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 3 → actions → request_reviews → github_accounts
GitHub Check: Mergify Merge Queue: The current Mergify configuration is invalid
Conclusion: failure
* Invalid condition 'author=dependabot[bot] | renovate[bot]' @ root → pull_request_rules → item 1 → conditions → item 0 → author=dependabot[bot] | renovate[bot]
```
Invalid GitHub login
```
* Invalid condition 'author=trunk-io[bot] | mergify[bot] | github-actions[bot]' @ root → pull_request_rules → item 2 → conditions → item 0 → author=trunk-io[bot] | mergify[bot] | github-actions[bot]
```
Invalid GitHub login
```
* Invalid condition 'age>=30d' @ root → pull_request_rules → item 8 → conditions → item 2 → age>=30d
```
Invalid attribute
```
* Extra inputs are not permitted @ root → pull_request_rules → item 0 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 1 → actions → post_merge
* Extra inputs are not permitted @ root → pull_request_rules → item 3 → actions → request_reviews → github_accounts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Python 3.11+ as the default target version unless a package pins otherwise
Files:
packages/phenokit-config-kit/linting/ci-cd/gatekeeper.tomlpackages/phenokit-config-kit/pytest/plugins/__init__.pypackages/phenokit-config-kit/config_kit/__init__.pytests/__init__.pypackages/phenokit-config-kit/pytest/fixtures/__init__.pypackages/phenokit-config-kit/pyproject.tomlpackages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.pypackages/phenokit-config-kit/linting/pyproject.tomlpackages/phenokit-config-kit/pytest/data/__init__.pypackages/phenokit-config-kit/linting/linters.tomlpackages/phenokit-config-kit/config_kit/pytest/data/__init__.pypackages/phenokit-config-kit/config_kit/pytest/plugins/__init__.pypyproject.tomlpackages/phenokit-config-kit/config_kit/pytest/plugins/security.pypackages/phenokit-config-kit/config_kit/pytest/plugins/architecture.pypackages/phenokit-config-kit/pytest/plugins/architecture.pypackages/phenokit-config-kit/config_kit/pytest/data/factory.pypackages/phenokit-config-kit/config_kit/pytest/fixtures/security.pypackages/phenokit-config-kit/config_kit/pytest/fixtures/common.pypackages/phenokit-config-kit/pytest/fixtures/common.pypackages/phenokit-config-kit/pytest/fixtures/performance.pypackages/phenokit-config-kit/config_kit/pytest/__init__.pypackages/phenokit-config-kit/pytest/fixtures/security.pypackages/phenokit-config-kit/pytest/plugins/performance.pypackages/phenokit-config-kit/config_kit/pytest/fixtures/performance.pypackages/phenokit-config-kit/config_kit/pytest/plugins/performance.pypackages/phenokit-config-kit/pytest/plugins/security.pypackages/phenokit-config-kit/pytest/data/factory.pytests/test_install_path.py
🪛 ast-grep (0.45.0)
packages/phenokit-config-kit/config_kit/pytest/plugins/security.py
[warning] 136-136: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[warning] 175-175: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[warning] 191-191: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[warning] 136-136: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
[warning] 175-175: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
[warning] 191-191: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
[warning] 132-132: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 148-148: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 171-171: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 187-187: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
packages/phenokit-config-kit/config_kit/pytest/plugins/architecture.py
[warning] 92-92: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 133-133: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 153-153: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 165-165: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 196-196: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.match(pattern, import_name)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
packages/phenokit-config-kit/pytest/plugins/architecture.py
[warning] 92-92: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 133-133: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 153-153: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 165-165: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 196-196: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.match(pattern, import_name)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh
[error] 77-77: eval is invoked on a variable, parameter expansion, or command-substitution result, which re-parses the value as shell code. If any part of that value is attacker-controlled (arguments, environment, file contents, network output), it allows arbitrary command execution. Do not eval dynamic data: invoke the command directly with proper quoting (e.g. "$cmd" "$arg"), use arrays for argument lists (cmd=(prog --flag "$value"); "${cmd[@]}"), or restrict input to a validated allowlist before running it.
Context: eval "$check_command"
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(eval-on-variable-bash)
packages/phenokit-config-kit/config_kit/pytest/fixtures/security.py
[warning] 82-82: Do not use insecure functions
Context: hashlib.md5(test_string.encode())
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm.
(insecure-hash-functions)
packages/phenokit-config-kit/pytest/fixtures/security.py
[warning] 82-82: Do not use insecure functions
Context: hashlib.md5(test_string.encode())
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm.
(insecure-hash-functions)
packages/phenokit-config-kit/pytest/plugins/security.py
[warning] 136-136: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
[warning] 175-175: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
[warning] 191-191: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
[warning] 136-136: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[warning] 175-175: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[warning] 191-191: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.findall(pattern, content, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[warning] 132-132: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 148-148: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 171-171: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 187-187: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(py_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
tests/test_install_path.py
[error] 94-100: Use of unsanitized data to create processes
Context: subprocess.run(
cmd,
capture_output=True,
text=True,
check=False,
timeout=_INSTALL_TIMEOUT_SECS,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
[error] 54-65: Command coming from incoming request
Context: subprocess.run(
(
"uv",
"venv",
"--python",
python_version,
str(venv_dir),
),
capture_output=True,
text=True,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 94-100: Command coming from incoming request
Context: subprocess.run(
cmd,
capture_output=True,
text=True,
check=False,
timeout=_INSTALL_TIMEOUT_SECS,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 GitHub Check: SonarCloud Code Analysis
packages/phenokit-config-kit/pytest/plugins/__init__.py
[failure] 12-12: Change or remove this string; "PerformancePlugin" is not defined.
[failure] 10-10: Change or remove this string; "ArchitecturePlugin" is not defined.
[failure] 13-13: Change or remove this string; "ReportingPlugin" is not defined.
[failure] 14-14: Change or remove this string; "SecurityPlugin" is not defined.
[failure] 11-11: Change or remove this string; "CoveragePlugin" is not defined.
packages/phenokit-config-kit/config_kit/__init__.py
[failure] 15-15: Change or remove this string; "pre_commit" is not defined.
[failure] 14-14: Change or remove this string; "linting" is not defined.
packages/phenokit-config-kit/pytest/fixtures/__init__.py
[failure] 14-14: Change or remove this string; "performance_monitor" is not defined.
[failure] 15-15: Change or remove this string; "security_context" is not defined.
[failure] 11-11: Change or remove this string; "database_fixture" is not defined.
[failure] 17-17: Change or remove this string; "test_data" is not defined.
[failure] 12-12: Change or remove this string; "message_broker_fixture" is not defined.
[failure] 16-16: Change or remove this string; "temp_dir" is not defined.
[failure] 13-13: Change or remove this string; "mock_client" is not defined.
[failure] 10-10: Change or remove this string; "cache_fixture" is not defined.
packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py
[failure] 17-17: Change or remove this string; "test_data" is not defined.
[failure] 10-10: Change or remove this string; "cache_fixture" is not defined.
[failure] 12-12: Change or remove this string; "message_broker_fixture" is not defined.
[failure] 14-14: Change or remove this string; "performance_monitor" is not defined.
[failure] 15-15: Change or remove this string; "security_context" is not defined.
[failure] 13-13: Change or remove this string; "mock_client" is not defined.
[failure] 11-11: Change or remove this string; "database_fixture" is not defined.
[failure] 16-16: Change or remove this string; "temp_dir" is not defined.
packages/phenokit-config-kit/pytest/data/__init__.py
[failure] 10-10: Change or remove this string; "TestDataGenerator" is not defined.
[failure] 11-11: Change or remove this string; "TestDataManager" is not defined.
[failure] 9-9: Change or remove this string; "TestDataFactory" is not defined.
[failure] 12-12: Change or remove this string; "TestDataValidator" is not defined.
packages/phenokit-config-kit/config_kit/pytest/data/__init__.py
[failure] 12-12: Change or remove this string; "TestDataValidator" is not defined.
[failure] 10-10: Change or remove this string; "TestDataGenerator" is not defined.
[failure] 11-11: Change or remove this string; "TestDataManager" is not defined.
[failure] 9-9: Change or remove this string; "TestDataFactory" is not defined.
packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py
[failure] 11-11: Change or remove this string; "CoveragePlugin" is not defined.
[failure] 12-12: Change or remove this string; "PerformancePlugin" is not defined.
[failure] 14-14: Change or remove this string; "SecurityPlugin" is not defined.
[failure] 13-13: Change or remove this string; "ReportingPlugin" is not defined.
[failure] 10-10: Change or remove this string; "ArchitecturePlugin" is not defined.
packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh
[warning] 141-141: Define a constant instead of using the literal 'SECTION' 6 times.
[warning] 35-35: Assign this positional parameter to a local variable.
[warning] 27-27: Assign this positional parameter to a local variable.
[failure] 216-216: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
[failure] 145-145: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
[failure] 204-204: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
[warning] 23-23: Assign this positional parameter to a local variable.
[failure] 216-216: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
[warning] 93-93: Assign this positional parameter to a local variable.
[failure] 158-158: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
[warning] 23-23: Assign this positional parameter to a local variable.
[warning] 31-31: Assign this positional parameter to a local variable.
🪛 markdownlint-cli2 (0.23.2)
packages/phenokit-config-kit/README.md
[warning] 45-45: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 115-115: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 120-120: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 128-128: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 163-163: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 166-166: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 174-174: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 196-196: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/phenokit-config-kit/pytest/README.md
[warning] 605-605: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (15)
packages/phenokit-config-kit/config_kit/pytest/fixtures/common.py (1)
18-227: LGTM!packages/phenokit-config-kit/pytest/fixtures/common.py (1)
18-227: LGTM!packages/phenokit-config-kit/pytest/fixtures/security.py (1)
16-196: LGTM!packages/phenokit-config-kit/linting/.editorconfig (1)
1-40: LGTM!packages/phenokit-config-kit/linting/.markdownlint.json (1)
1-45: LGTM!packages/phenokit-config-kit/pre-commit/basic.yaml (1)
1-32: LGTM!packages/phenokit-config-kit/linting/linters.toml (1)
50-53: 🔒 Security & PrivacyUse one verified Safety CLI contract.
Safety 3 deprecates
safety checkin favor ofsafety scan. The package does not declare a Safety version, so these unpinned commands can fail or produce incompatible reports after an upgrade. (docs.safetycli.com)
packages/phenokit-config-kit/linting/linters.toml#L50-L53: define the supported Safety version and use its valid scan input and report-output options.packages/phenokit-config-kit/linting/pre-commit-hooks.yaml#L100-L106: use the same versioned command and report contract aspackages/phenokit-config-kit/linting/linters.tomlandpackages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh.packages/phenokit-config-kit/linting/pyproject.toml (1)
1-196: LGTM!WORKLOG.md (1)
10-10: LGTM!packages/phenokit-config-kit/ORIGIN.md (1)
1-5: LGTM!pyproject.toml (1)
32-53: LGTM!Also applies to: 77-77, 112-116
tests/__init__.py (1)
1-2: LGTM!tests/test_install_path.py (1)
47-60: 🎯 Functional CorrectnessNo change needed for the fixture assumptions.
The CI installs Python 3.12 in Ubuntu jobs and the fixture targets Python 3.13 on a POSIX
bin/pythonvenv path, so its assumptions remain consistent with the current CI platform.packages/phenokit-config-kit/config_kit/pytest/__init__.py (1)
1-7: LGTM!packages/phenokit-config-kit/pytest/basic.ini (1)
1-27: LGTM!
| def random_uuid(self) -> str: | ||
| """Generate a random UUID.""" | ||
| return str(uuid.uuid4()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make seeded factory output deterministic.
random_uuid() bypasses self._random. Default datetime bounds also depend on the current time. The same seed therefore produces different entity data across runs.
Generate UUID values from self._random. Use an injected or fixed datetime reference when callers omit bounds.
Also applies to: 61-70
🤖 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 `@packages/phenokit-config-kit/config_kit/pytest/data/factory.py` around lines
37 - 39, Update random_uuid() to generate UUID values from self._random instead
of the module-level random source, and update the default datetime-bound
handling in the related factory method around lines 61–70 to use an injected or
fixed reference time when callers omit bounds. Preserve explicit caller-provided
bounds while ensuring identical seeds produce identical entity data across runs.
| def start_thread_monitoring(): | ||
| """Start background thread monitoring.""" | ||
| def monitor(): | ||
| while True: | ||
| current_memory = psutil.Process().memory_info().rss / 1024 / 1024 | ||
| monitor_data["peak_memory"] = max(monitor_data["peak_memory"], current_memory) | ||
| time.sleep(0.1) | ||
|
|
||
| thread = threading.Thread(target=monitor, daemon=True) | ||
| thread.start() | ||
| monitor_data["threads"].append(thread) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a stop lifecycle for all performance monitor threads.
Each implementation starts a monitor without a termination signal. The threads remain active after their measured scope and can retain data or attribute later memory use to the wrong test.
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py#L50-L60: store a stop event with each monitor and stop and join it during fixture teardown.packages/phenokit-config-kit/pytest/fixtures/performance.py#L50-L60: store a stop event with each monitor and stop and join it during fixture teardown.packages/phenokit-config-kit/pytest/plugins/performance.py#L53-L61: store a per-test stop event and stop and join the monitor inpytest_runtest_teardown.
📍 Affects 3 files
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py#L50-L60(this comment)packages/phenokit-config-kit/pytest/fixtures/performance.py#L50-L60packages/phenokit-config-kit/pytest/plugins/performance.py#L53-L61
🤖 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 `@packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py`
around lines 50 - 60, Implement a stop lifecycle for every performance monitor
thread: in
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py lines
50-60 and packages/phenokit-config-kit/pytest/fixtures/performance.py lines
50-60, store a per-monitor stop event, make the monitor exit when signaled, and
have fixture teardown signal and join the thread; in
packages/phenokit-config-kit/pytest/plugins/performance.py lines 53-61, store a
per-test stop event and signal and join the monitor from
pytest_runtest_teardown.
| try: | ||
| yield | ||
| finally: | ||
| end_time = time.time() | ||
| end_memory = psutil.Process().memory_info().rss / 1024 / 1024 | ||
|
|
||
| duration = end_time - start_time | ||
| memory_delta = end_memory - start_memory | ||
|
|
||
| if duration > threshold: | ||
| pytest.fail(f"Performance threshold exceeded for {name}: {duration:.2f}s > {threshold}s") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not replace a measured-block exception with a threshold failure.
Both context managers call pytest.fail() from finally. A slow failing block reports only the performance failure and hides the original test error.
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py#L165-L175: enforcethresholdonly after successful block completion.packages/phenokit-config-kit/pytest/fixtures/performance.py#L165-L175: enforcethresholdonly after successful block completion.
📍 Affects 2 files
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py#L165-L175(this comment)packages/phenokit-config-kit/pytest/fixtures/performance.py#L165-L175
🤖 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 `@packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py`
around lines 165 - 175, Update the performance context manager’s finally logic
in both
packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py:165-175
and packages/phenokit-config-kit/pytest/fixtures/performance.py:165-175 so
threshold enforcement occurs only when the wrapped block completes successfully;
preserve any exception raised by the measured block instead of replacing it with
pytest.fail(), while retaining the existing failure for slow successful blocks.
| @pytest.fixture | ||
| def security_context() -> Generator[dict[str, Any], None, None]: | ||
| """Provide security context for testing.""" | ||
| return { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the return type annotation of security_context.
The fixture returns a dictionary. It does not yield. The Generator[dict[str, Any], None, None] annotation is wrong and breaks static type checks. The other fixtures in this file use the correct direct return type.
🔧 Proposed fix
`@pytest.fixture`
-def security_context() -> Generator[dict[str, Any], None, None]:
+def security_context() -> dict[str, Any]:
"""Provide security context for testing."""
return {Remove the now-unused Generator import at line 9 if no other fixture needs it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.fixture | |
| def security_context() -> Generator[dict[str, Any], None, None]: | |
| """Provide security context for testing.""" | |
| return { | |
| `@pytest.fixture` | |
| def security_context() -> dict[str, Any]: | |
| """Provide security context for testing.""" | |
| return { |
🤖 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 `@packages/phenokit-config-kit/config_kit/pytest/fixtures/security.py` around
lines 16 - 19, Update the security_context fixture annotation from Generator to
the direct dictionary return type used by the fixture, and remove the Generator
import if no other fixture references it.
| "md5": hashlib.md5(test_string.encode()).hexdigest(), | ||
| "sha1": hashlib.sha1(test_string.encode()).hexdigest(), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Mark the MD5 and SHA-1 calls as non-security use.
hashlib.md5 and hashlib.sha1 raise ValueError when Python runs against a FIPS-enabled OpenSSL build. These digests are test data only. Pass usedforsecurity=False to keep the fixture usable and to clear the scanner finding.
🔧 Proposed fix
- "md5": hashlib.md5(test_string.encode()).hexdigest(),
- "sha1": hashlib.sha1(test_string.encode()).hexdigest(),
+ "md5": hashlib.md5(test_string.encode(), usedforsecurity=False).hexdigest(),
+ "sha1": hashlib.sha1(test_string.encode(), usedforsecurity=False).hexdigest(),usedforsecurity requires Python 3.9 or later, which matches the 3.11+ target. As per coding guidelines: "Use Python 3.11+ as the default target version unless a package pins otherwise".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "md5": hashlib.md5(test_string.encode()).hexdigest(), | |
| "sha1": hashlib.sha1(test_string.encode()).hexdigest(), | |
| "md5": hashlib.md5(test_string.encode(), usedforsecurity=False).hexdigest(), | |
| "sha1": hashlib.sha1(test_string.encode(), usedforsecurity=False).hexdigest(), |
🤖 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 `@packages/phenokit-config-kit/config_kit/pytest/fixtures/security.py` around
lines 83 - 84, Update the MD5 and SHA-1 calls in the fixture data construction
to pass usedforsecurity=False, preserving the existing test-string inputs and
hexadecimal digest output so the fixture works with FIPS-enabled OpenSSL.
Sources: Coding guidelines, Linters/SAST tools
| __all__ = [ | ||
| "cache_fixture", | ||
| "database_fixture", | ||
| "message_broker_fixture", | ||
| "mock_client", | ||
| "performance_monitor", | ||
| "security_context", | ||
| "temp_dir", | ||
| "test_data", | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 '__all__|^(def|class) ' \
packages/phenokit-config-kit/pytest/fixtures/__init__.py \
packages/phenokit-config-kit/pytest/fixtures/common.pyRepository: KooshaPari/phenotype-python-sdk
Length of output: 6005
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fixtures __init__ =="
nl -ba packages/phenokit-config-kit/pytest/fixtures/__init__.py | sed -n '1,80p'
echo
echo "== common fixtures outline =="
rg -n '`@pytest.fixture`|def mock_|def .*fixture|from|import' packages/phenokit-config-kit/pytest/fixtures/common.py
echo
echo "== all pytest fixtures definitions across package =="
rg -n '^`@pytest`\.fixture|def [A-Za-z_][A-Za-z0-9_]*\b' packages/phenokit-config-kit/pytest -g '*.py'
echo
echo "== __all__ unresolved probe (local symbols) =="
python3 - <<'PY'
from pathlib import Path
import ast
files = [
Path("packages/phenokit-config-kit/pytest/fixtures/__init__.py"),
Path("packages/phenokit-config-kit/pytest/fixtures/common.py"),
]
names = set()
for path in files:
tree = ast.parse(path.read_text(), filename=str(path))
for node in ast.walk(tree):
if isinstance(node, ast.Name):
names.add(node.id)
init = ast.parse(Path("packages/phenokit-config-kit/pytest/fixtures/__init__.py").read_text())
all_names = next(n for n in init.body if isinstance(n, ast.Assign) and any(a.id == "__all__" for a in n.targets)).value
all_names = [el.value for el in all_names.elts]
missing = [n for n in all_names if n not in names]
print({"__all__": all_names, "unresolved_by_names": missing})
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fixtures __init__ =="
awk '{printf "%6d\t%s\n", NR, $0}' packages/phenokit-config-kit/pytest/fixtures/__init__.py
echo
echo "== common fixture definitions =="
awk '/(^`@pytest`\.fixture|def [A-Za-z_][A-Za-z0-9_]*\b)/ {printf "%6d\t%s\n", NR, $0}' packages/phenokit-config-kit/pytest/fixtures/common.py
echo
echo "== fixture definitions across pytest package =="
awk '/(^`@pytest`\.fixture|def [A-Za-z_][A-Za-z0-9_]*\b)/ {printf "%6d\t%s\n", NR, $0}' packages/phenokit-config-kit/pytest -g '*.py'
echo
echo "== __all__ unresolved probe =="
python3 - <<'PY'
from pathlib import Path
import ast
paths = [
Path("packages/phenokit-config-kit/pytest/fixtures/__init__.py"),
Path("packages/phenokit-config-kit/pytest/fixtures/common.py"),
]
names = set()
for path in paths:
tree = ast.parse(path.read_text(), filename=str(path))
for node in ast.walk(tree):
if isinstance(node, ast.Name):
names.add(node.id)
init_path = Path("packages/phenokit-config-kit/pytest/fixtures/__init__.py")
init = ast.parse(init_path.read_text())
all_node = next(n for n in init.body if isinstance(n, ast.Assign) and any(a.id == "__all__" for a in n.targets))
if isinstance(all_node.value, ast.List):
all_names = [el.value for el in all_node.value.elts]
else:
all_names = []
missing = [n for n in all_names if n not in names]
print({"__all__": all_names, "unresolved_by_local_names": missing})
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 1240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== common.py line 185-230 =="
sed -n '185,230p' packages/phenokit-config-kit/pytest/fixtures/common.py
echo
echo "== Python fixture symbols and __all__ coverage =="
python3 - <<'PY'
from pathlib import Path
import ast
init = ast.parse(Path("packages/phenokit-config-kit/pytest/fixtures/__init__.py").read_text())
common = ast.parse(Path("packages/phenokit-config-kit/pytest/fixtures/common.py").read_text())
def defined_names(tree):
names = set()
for node in ast.walk(tree):
if isinstance(node, ast.Name):
names.add(node.id)
return names
def fixture_names(tree):
names = set()
for node in tree.body:
if (isinstance(node, ast.FunctionDef) and len(node.decorator_list) >= 1
and isinstance(node.decorator_list[0], ast.Call)
and isinstance(node.decorator_list[0].func, ast.Name)
and node.decorator_list[0].func.id == "pytest"):
names.add(node.name)
return names
init_names = defined_names(init)
common_fixture_names = fixture_names(common)
all_names = next(n for n in init.body if isinstance(n, ast.Assign) and any(a.id == "__all__" for a in n.targets)).value.elts
all_names = [x.value for x in all_names]
print("__all__:", all_names)
print("fixture_names:", sorted(common_fixture_names))
print("missing_names_in_init_names:", [n for n in all_names if n not in init_names])
print("unmatched_all_to_fixture_names:", [(n, n in common_fixture_names) for n in all_names])
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 1884
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== common.py imports and decorators =="
sed -n '1,70p' packages/phenokit-config-kit/pytest/fixtures/common.py
sed -n '100,175p' packages/phenokit-config-kit/pytest/fixtures/common.pyRepository: KooshaPari/phenotype-python-sdk
Length of output: 4241
Export the defined fixtures.
pytest/fixtures/__init__.py advertises names that are not defined or imported in this module. Update __all__ to the real fixture callables, such as mock_cache, mock_database, and mock_message_broker, or keep the _fixture aliases, but ensure the package export surface resolves when imported.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 14-14: Change or remove this string; "performance_monitor" is not defined.
[failure] 15-15: Change or remove this string; "security_context" is not defined.
[failure] 11-11: Change or remove this string; "database_fixture" is not defined.
[failure] 17-17: Change or remove this string; "test_data" is not defined.
[failure] 12-12: Change or remove this string; "message_broker_fixture" is not defined.
[failure] 16-16: Change or remove this string; "temp_dir" is not defined.
[failure] 13-13: Change or remove this string; "mock_client" is not defined.
[failure] 10-10: Change or remove this string; "cache_fixture" is not defined.
🤖 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 `@packages/phenokit-config-kit/pytest/fixtures/__init__.py` around lines 9 -
18, Update __all__ in the fixtures package to list only fixture callables that
are actually defined or imported, including the available mock_cache,
mock_database, and mock_message_broker symbols or their existing _fixture
aliases. Ensure every advertised name resolves when the package is imported.
Source: Linters/SAST tools
| __all__ = [ | ||
| "ArchitecturePlugin", | ||
| "CoveragePlugin", | ||
| "PerformancePlugin", | ||
| "ReportingPlugin", | ||
| "SecurityPlugin", | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
__all__ lists five classes that the package does not import.
The module declares ArchitecturePlugin, CoveragePlugin, PerformancePlugin, ReportingPlugin, and SecurityPlugin, but imports none of them. from config_kit.pytest.plugins import * raises AttributeError. SonarCloud reports all five names.
The provided files define ArchitecturePlugin, PerformancePlugin, and SecurityPlugin. They contain no CoveragePlugin and no ReportingPlugin.
🔧 Proposed fix
+from .architecture import ArchitecturePlugin
+from .performance import PerformancePlugin
+from .security import SecurityPlugin
+
__version__ = "0.1.0"
__all__ = [
"ArchitecturePlugin",
- "CoveragePlugin",
"PerformancePlugin",
- "ReportingPlugin",
"SecurityPlugin",
]Add CoveragePlugin and ReportingPlugin back only after the corresponding modules exist. Note that security.py currently fails to import, so this re-export breaks the package until that defect is fixed.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 12-12: Change or remove this string; "PerformancePlugin" is not defined.
[failure] 10-10: Change or remove this string; "ArchitecturePlugin" is not defined.
[failure] 13-13: Change or remove this string; "ReportingPlugin" is not defined.
[failure] 14-14: Change or remove this string; "SecurityPlugin" is not defined.
[failure] 11-11: Change or remove this string; "CoveragePlugin" is not defined.
🤖 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 `@packages/phenokit-config-kit/pytest/plugins/__init__.py` around lines 9 - 15,
Update the package exports around __all__ so they reference only plugin classes
that are actually imported and available, including ArchitecturePlugin,
PerformancePlugin, and SecurityPlugin once security.py imports successfully.
Remove CoveragePlugin and ReportingPlugin until corresponding modules and
imports exist, and add explicit imports for every name retained in __all__ to
make wildcard imports valid.
Source: Linters/SAST tools
| # Monitor memory usage during test execution | ||
| def monitor_memory(): | ||
| while item.nodeid in self.performance_data: | ||
| current_memory = psutil.Process().memory_info().rss / 1024 / 1024 | ||
| self.performance_data[item.nodeid]["peak_memory"] = max(self.performance_data[item.nodeid]["peak_memory"], current_memory) | ||
| time.sleep(0.1) | ||
|
|
||
| monitor_thread = threading.Thread(target=monitor_memory, daemon=True) | ||
| monitor_thread.start() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Stop the test memory monitor after its test.
performance_data retains each node ID for later validation. Therefore, every monitor_memory() loop continues through the remaining test session. Old monitors record memory from unrelated tests and create one thread per performance-marked test.
Use a per-test stop event. Signal and join the monitor thread in pytest_runtest_teardown.
🤖 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 `@packages/phenokit-config-kit/pytest/plugins/performance.py` around lines 53 -
61, Update the memory-monitor lifecycle around monitor_memory and monitor_thread
to use a per-test stop event instead of relying on performance_data membership.
Store the event and thread per test, have the loop exit when signaled, and
signal then join the thread from pytest_runtest_teardown before retaining
performance_data for validation.
| def pytest_collection_modifyitems(self, config, items): | ||
| """Add performance validation tests.""" | ||
| performance_items = [] | ||
|
|
||
| # Add benchmark validation | ||
| performance_items.append( | ||
| pytest.Function.from_parent( | ||
| parent=items[0].parent if items else None, | ||
| name="test_benchmark_performance", | ||
| callobj=self._test_benchmark_performance, | ||
| markers=[pytest.mark.performance, pytest.mark.benchmark], | ||
| ), | ||
| ) | ||
|
|
||
| # Add memory usage validation | ||
| performance_items.append( | ||
| pytest.Function.from_parent( | ||
| parent=items[0].parent if items else None, | ||
| name="test_memory_usage", | ||
| callobj=self._test_memory_usage, | ||
| markers=[pytest.mark.performance, pytest.mark.memory], | ||
| ), | ||
| ) | ||
|
|
||
| items.extend(performance_items) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'pytest_collection_modifyitems|Function\.from_parent|items\[0\]' \
packages/phenokit-config-kit/pytest/plugins/performance.pyRepository: KooshaPari/phenotype-python-sdk
Length of output: 1285
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline packages/phenokit-config-kit/pytest/plugins/performance.py || true
echo "== relevant file section =="
cat -n packages/phenokit-config-kit/pytest/plugins/performance.py | sed -n '1,130p'
echo "== pytest constraints/deps =="
rg -n 'pytest|python_requires|requires-python|requires_pyyaml' pyproject.toml packages/phenokit-config-kit pyproject.toml 2>/dev/null || true
echo "== pytest installed and Function.from_parent signature/behavior probe =="
python3 - <<'PY'
import inspect, pytest
print("pytest_version", getattr(pytest, "__version__", None))
print("Function", pytest.Function)
try:
print(inspect.signature(pytest.Function.from_parent))
except Exception as e:
print("signature_error", repr(e))
try:
item = pytest.Function.from_parent(parent=None, name="probe")
print("created_without_parent_type", type(item).__module__ + "." + type(item).__qualname__)
except Exception as e:
print("created_without_parent_exception", type(e).__module__ + "." + type(e).__qualname__ + ": " + str(e))
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 42509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '155,190p' packages/phenokit-config-kit/pyproject.toml
echo "== check package name in source =="
rg -n 'name\s*=|packages\.phenokit-config-kit|phenokit-config-kit|config_kit\.pytest\.plugins\.performance|config_kit_performance' pyproject.toml packages -g 'pyproject.toml' -g '*.py'
echo "== check matching performance plugins =="
find packages -path '*/pytest/plugins/performance.py' -type f -exec sh -c 'echo "--- $1"; sed -n "63,87p" "$1"' sh {} \;Repository: KooshaPari/phenotype-python-sdk
Length of output: 50388
🌐 Web query:
pytest Function.from_parent parent must be a Node documentation
💡 Result:
In pytest, the from_parent method is the required public constructor for creating nodes (such as Function, Class, or Item) [1][2][3]. The requirement that the parent argument must be a Node (specifically, an instance of _pytest.nodes.Node) is enforced by the from_parent method signature and the internal architecture of pytest [1][4][5]. Key details regarding this pattern include: 1. Mandatory Use: Direct instantiation of pytest Node classes (e.g., MyItem(...)) is deprecated and will raise an error [1][5]. This change was introduced to simplify the creation of the collection tree by moving away from entangled constructor logic [1][2]. 2. Method Signature: The from_parent method is defined as a class method with the following signature: @classmethod def from_parent(cls, parent: Node, kw) -> Self: [1][4] 3. Parent Argument: The parent argument is required and must be an existing Node object [1][5]. The from_parent method uses this parent to establish the node's position within the test collection hierarchy [1][2]. 4. Keyword Arguments: All other parameters (e.g., name, fixtureinfo, etc.) must be passed as keyword arguments [6][3]. If you are developing a plugin or customizing pytest collection, you should always use this method instead of direct class instantiation [3]. For example: # Correct usage item = MyItem.from_parent(parent_collector, name="test_name") This pattern ensures that the node is correctly initialized within the pytest framework's dependency and configuration tree [1][2].
Citations:
- 1: https://docs.pytest.org/en/stable/_modules/_pytest/nodes.html
- 2: https://docs.pytest.org/en/latest/_modules/_pytest/nodes.html
- 3: https://docs.pytest.org/en/stable/deprecations.html
- 4: https://pytest.org/en/8.1.x/%5Fmodules/%5Fpytest/nodes.html
- 5: https://pytest.org/en/stable/%5Fmodules/%5Fpytest/nodes.html
- 6: https://docs.pytest.org/en/7.1.x/deprecations.html
🌐 Web query:
pytest pytest_collection_modifyitems hook signature hook implementation items list
💡 Result:
The pytest_collection_modifyitems hook is used to filter, reorder, or modify the list of collected test items after the collection phase has completed [1][2]. Hook Signature The full signature defined by pytest is: def pytest_collection_modifyitems(session, config, items):... Implementation Details 1. Argument Flexibility: You do not need to include all arguments in your function signature [2]. Pytest uses dynamic argument pruning, meaning it will only pass the arguments you explicitly list (e.g., if you only need items, you can define the function as def pytest_collection_modifyitems(items):) [2][3]. 2. Modifying the List: The items argument is a list of test item objects. You can modify this list in-place (e.g., using list.append, list.pop, or list.clear) or replace its contents (e.g., items[:] = new_list) to filter or reorder tests [1][4]. 3. Deselection: If you remove items from the list, you should notify pytest that they have been deselected so other plugins and reporting tools remain consistent [1]. You can do this by calling the pytest_deselected hook [1]: config.hook.pytest_deselected(items=deselected_items) Example Implementation Below is a standard pattern for filtering test items: def pytest_collection_modifyitems(config, items): # Example: Keep only items that have a specific marker selected = [] deselected = [] for item in items: if item.get_closest_marker("my_marker"): selected.append(item) else: deselected.append(item) if deselected: config.hook.pytest_deselected(items=deselected) # In-place modification of the list items[:] = selected Notes - Execution Order: You can control when your hook runs relative to others by using decorators like @pytest.hookimpl(tryfirst=True) or @pytest.hookimpl(trylast=True) [3][5]. - Session and Config: The session object represents the current test session, and the config object provides access to pytest configuration, command-line options, and hooks [1][4].
Citations:
- 1: https://docs.pytest.org/en/8.3.x/_modules/_pytest/hookspec.html
- 2: https://docs.pytest.org/en/stable/how-to/writing_hook_functions.html
- 3: https://docs.pytest.org/en/8.3.x/how-to/writing_hook_functions.html
- 4: https://raphael.codes/blog/run-tests-using-a-certain-pytest-fixture/
- 5: https://happytest.readthedocs.io/en/latest/writing_plugins.html
🌐 Web query:
Pytest Node.from_parent parent None implementation cls name parent=parent
💡 Result:
In pytest, Node.from_parent is the required, standardized constructor for creating Node instances (such as Items or Collectors), introduced to move away from direct Node instantiation [1][2]. Implementation and Usage: The from_parent method is a class method defined on the Node base class [3]. Its primary purpose is to provide a controlled indirection for object creation, which helps decouple the complex initialization logic previously found in direct constructors [3][4]. The signature is: @classmethod def from_parent(cls, parent: Node, kw) -> Self: Key rules for using from_parent: 1. Mandatory Parent: The parent argument is required and must be an instance of a Node [3][4]. It cannot be None. 2. Keyword-Only Arguments: The remaining arguments (e.g., name, path) must be passed as keyword arguments [1][2]. 3. No Direct Construction: Direct instantiation (e.g., MyItem(name="foo", parent=...)) is deprecated and will trigger errors because the Node class uses a metaclass (NodeMeta) that explicitly prevents direct calls to the class constructor [3][4]. Why it exists: This indirection was implemented to support refactoring of the pytest collection tree [3]. Because Node constructors were historically "fragile" and overly entangled, from_parent serves as a stable API surface that plugins and internal code should use instead of calling init directly [3][1]. Subclasses can call super.from_parent(...) if they need to override or extend the initialization process [3]. If you are a plugin author, you should always use this method to construct nodes, and you can guard your code for compatibility with older pytest versions using hasattr(MyClass, "from_parent") [1][2].
Citations:
- 1: https://docs.pytest.org/en/7.1.x/deprecations.html
- 2: https://docs.pytest.org/en/stable/deprecations.html
- 3: https://docs.pytest.org/en/stable/_modules/_pytest/nodes.html
- 4: https://github.com/pytest-dev/pytest/blob/90465694/src/_pytest/nodes.py
Handle an empty collected-item list.
When pytest collects no tests, items is empty and parent becomes None. pytest.Function.from_parent() requires a pytest node parent. Exit pytest_collection_modifyitems before adding the validation tests when items is empty.
🤖 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 `@packages/phenokit-config-kit/pytest/plugins/performance.py` around lines 63 -
87, Update pytest_collection_modifyitems to return immediately when items is
empty, before constructing performance_items or calling
pytest.Function.from_parent. Preserve adding the benchmark and memory validation
tests with items[0].parent for non-empty collections.
| def load_test(users: int = 10, duration: int = 60) -> Callable: | ||
| """Decorator for marking functions as load tests.""" | ||
| return pytest.mark.performance(pytest.mark.parametrize("load_users", [users])(func)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching performance.py:\n'
fd -a 'performance\.py$' . | sed 's#^\./##'
printf '\nTarget file line count and relevant section:\n'
wc -l packages/phenokit-config-kit/pytest/plugins/performance.py
sed -n '1,230p' packages/phenokit-config-kit/pytest/plugins/performance.py
printf '\nSearch load_test usages:\n'
rg -n "load_test|load_users|load_duration" packages/phenokit-config-kit || trueRepository: KooshaPari/phenotype-python-sdk
Length of output: 8084
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
def load_test(users=10, duration=60):
import pytest
return pytest.mark.performance(pytest.mark.parametrize("load_users", [users])(func))
`@load_test`(5, 120)
def my_test(): pass
PY
printf '\nStatic names in load_test function body:\n'
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("packages/phenokit-config-kit/pytest/plugins/performance.py")
tree = ast.parse(path.read_text())
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef) and node.name == "load_test":
print(f"load_test defined at line {node.lineno}-{node.end_lineno}")
print("arguments:", [arg.arg for arg in node.args.args])
print("loads:", [name.id for subnode in ast.walk(node.body) if isinstance(subnode, ast.Name) and isinstance(subnode.ctx, ast.Load)])
for stmt in node.body:
print(stmt, ast.unparse(stmt))
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("packages/phenokit-config-kit/pytest/plugins/performance.py")
tree = ast.parse(path.read_text())
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef) and node.name == "load_test":
print(f"load_test defined at line {node.lineno}-{node.end_lineno}")
print("arguments:", [arg.arg for arg in node.args.args])
func_names = []
for subnode in ast.walk(node.body):
if isinstance(subnode, ast.Name) and isinstance(subnode.ctx, ast.Load):
func_names.append(subnode.id)
print("name loads inside body:", sorted(set(func_names)))
print("body:", ast.unparse(node.body))
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 667
Return a decorator from load_test.
load_test() currently references func without defining it, so applying @load_test(...) raises NameError. Return an inner function that accepts func, apply the performance and parametrization markers there, and include duration in the generated test parameters if the API keeps it.
🤖 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 `@packages/phenokit-config-kit/pytest/plugins/performance.py` around lines 143
- 145, Update load_test to return an inner decorator that accepts func before
applying the performance and parametrize markers, eliminating the undefined
reference. Preserve users as the load_users parameter, and include duration in
the generated test parameters if the load_test API continues to expose it.
| __version__ = "0.1.0" | ||
| __all__ = [ | ||
| "pytest", | ||
| "linting", | ||
| "pre_commit", | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files 'packages/phenokit-config-kit/**/__init__.py' | sort
echo "== outline root init =="
ast-grep outline packages/phenokit-config-kit/config_kit/__init__.py || true
echo "== root init =="
cat -n packages/phenokit-config-kit/config_kit/__init__.py
echo "== pytest data init =="
cat -n packages/phenokit-config-kit/config_kit/pytest/data/__init__.py
echo "== pytest fixtures init =="
cat -n packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py
echo "== pytest plugins init =="
cat -n packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py
echo "== references to exported names in package =="
rg -n "TestDataFactory|TestDataGenerator|TestDataManager|TestDataValidator|cache_fixture|database_fixture|message_broker_fixture|mock_client|perform|security_context|temp_dir|test_data|ArchitecturePlugin|CoveragePlugin|PerformancePlugin|ReportingPlugin|SecurityPlugin|linting|pre_commit|pytest" packages/phenokit-config-kit/config_kit || trueRepository: KooshaPari/phenotype-python-sdk
Length of output: 24006
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all files under config_kit =="
git ls-files packages/phenokit-config-kit/config_kit | sort
echo "== fixtures __all__ candidates =="
rg -n "^def .*fixture|`@pytest.fixture`|^class .*Fixture|^cache_fixture|^database_fixture|^message_broker_fixture|^mock_client|^performance_monitor|^security_context|^temp_dir|^test_data" packages/phenokit-config-kit/config_kit/pytest/fixtures/config_kit || true
echo "== linting/pre_commit candidate submodules =="
fd -i '^(linting|pre_commit)(\.py|/)?' packages/phenokit-config-kit/config_kit || true
echo "== package init files =="
for f in packages/phenokit-config-kit/config_kit/__init__.py packages/phenokit-config-kit/config_kit/pytest/__init__.py packages/phenokit-config-kit/config_kit/pytest/data/__init__.py packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py; do
echo "-- $f"
cat -n "$f"
done
echo "== deterministic init symbol check =="
python3 - <<'PY'
import ast
from pathlib import Path
roots = [
Path("packages/phenokit-config-kit/config_kit/__init__.py"),
Path("packages/phenokit-config-kit/config_kit/pytest/data/__init__.py"),
Path("packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py"),
Path("packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py"),
]
for init_path in roots:
tree = ast.parse(init_path.read_text())
module_aliases = set()
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == "__all__":
module_aliases.add(target.id)
all_value = ast.literal_eval(node.value)
names = set(all_value)
defined = set(node.name for node in ast.walk(tree) if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef, ast.Assign, ast.AnnAssign)))
imported = set()
for item in ast.walk(tree):
if isinstance(item, ast.Import):
imported.update(alias.name.split(".")[0] for alias in item.names)
elif isinstance(item, ast.ImportFrom):
import_name = item.module.split(".")[0] if item.module else "."
imported.add(import_name)
print(f"{init_path}: __all__={all_value}")
print(" missing:", sorted(names - defined))
print(" imported_names:", sorted(set(imported)))
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 3709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fixtures implementation files =="
cat -n packages/phenokit-config-kit/config_kit/pytest/fixtures/common.py | sed -n '1,120p'
cat -n packages/phenokit-config-kit/config_kit/pytest/fixtures/security.py | sed -n '1,120p'
cat -n packages/phenokit-config-kit/config_kit/pytest/fixtures/performance.py | sed -n '1,120p'
echo "== data implementation files =="
git ls-files packages/phenokit-config-kit/config_kit/pytest/data | sort
for f in packages/phenokit-config-kit/config_kit/pytest/data/factory.py packages/phenokit-config-kit/config_kit/pytest/data/factory.py; do
echo "-- $f"
rg -n "class TestData|TestDataFactory|TestDataGenerator|TestDataManager|TestDataValidator" "$f" || true
done
echo "== plugins implementation files =="
git ls-files packages/phenokit-config-kit/config_kit/pytest/plugins | sort
for f in packages/phenokit-config-kit/config_kit/pytest/plugins/architecture.py packages/phenokit-config-kit/config_kit/pytest/plugins/performance.py packages/phenokit-config-kit/config_kit/pytest/plugins/security.py; do
echo "-- $f"
rg -n "class .*Plugin" "$f" || true
doneRepository: KooshaPari/phenotype-python-sdk
Length of output: 14154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fixtures names in implementation files =="
python3 - <<'PY'
from pathlib import Path
import ast
root = Path("packages/phenokit-config-kit/config_kit/pytest/fixtures")
all_path = root / "__init__.py"
tree = ast.parse(all_path.read_text())
all_value = next((node.value for node in tree.body if isinstance(node, ast.Assign) and any(isinstance(t, ast.Name) and t.id == "__all__" for t in node.targets)), None)
exported = ast.literal_eval(all_value) if all_value is not None else []
names = {name: [] for name in exported}
for py in sorted(root.glob("*.py")):
mod = ast.parse(py.read_text())
for node in mod.body:
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name in names:
names[node.name].append(str(py))
if isinstance(node, ast.ClassDef) and node.name in names:
names[node.name].append(str(py))
missing = [name for name, locs in names.items() if not locs]
print("__all__=", exported)
print("missing_names=", missing)
print("implemented_in=", {name: locs for name, locs in names.items() if locs})
print("\nsubmodules referenced in current init __all__:")
init_root = Path("packages/phenokit-config-kit/config_kit/__init__.py")
for node in ast.parse(init_root.read_text()).body:
if isinstance(node, ast.Assign) and any(isinstance(t, ast.Name) and t.id == "__all__" for t in node.targets):
symbols = ast.literal_eval(node.value)
print({sym: (root/sym).exists() or (root/sym/f"{sym}").exists() or (root/sym/f"{sym}.py").exists() for sym in symbols})
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 1154
Bind every public symbol declared in __all__, or remove it.
__all__ documents the import surface, but these initializers export names that are not bound in the module. Star imports and consumers that rely on these exports will fail.
packages/phenokit-config-kit/config_kit/__init__.py: add the supported submodules, or removelintingandpre_commitif they have no Python implementation.packages/phenokit-config-kit/config_kit/pytest/data/__init__.py: importTestDataFactory,TestDataGenerator,TestDataManager, andTestDataValidatorfrom their implementation modules.packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py: import every fixture declared in__all__, includingcache_fixture,database_fixture, andmessage_broker_fixture, or remove them.packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py: import the exported plugin classes, or export the actual plugin module names.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 15-15: Change or remove this string; "pre_commit" is not defined.
[failure] 14-14: Change or remove this string; "linting" is not defined.
📍 Affects 4 files
packages/phenokit-config-kit/config_kit/__init__.py#L11-L16(this comment)packages/phenokit-config-kit/config_kit/pytest/data/__init__.py#L7-L13packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py#L8-L18packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py#L8-L15
🤖 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 `@packages/phenokit-config-kit/config_kit/__init__.py` around lines 11 - 16,
Ensure every name listed in __all__ is bound to a valid symbol across all
affected initializers: in packages/phenokit-config-kit/config_kit/__init__.py,
add supported submodules or remove unsupported linting and pre_commit names; in
packages/phenokit-config-kit/config_kit/pytest/data/__init__.py, import
TestDataFactory, TestDataGenerator, TestDataManager, and TestDataValidator; in
packages/phenokit-config-kit/config_kit/pytest/fixtures/__init__.py, import
every declared fixture including cache_fixture, database_fixture, and
message_broker_fixture; and in
packages/phenokit-config-kit/config_kit/pytest/plugins/__init__.py, import the
exported plugin classes or change __all__ to the actual plugin module names.
Source: Linters/SAST tools
| if eval "$check_command" >/dev/null 2>&1; then | ||
| PASSED=$((PASSED + 1)) | ||
| print_success "$success_message" | ||
| add_check_result "$check_name" "passed" "$success_message" "" | ||
| return 0 | ||
| else | ||
| FAILED=$((FAILED + 1)) | ||
| print_error "$failure_message" | ||
| add_check_result "$check_name" "failed" "$failure_message" "" | ||
| return 1 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not return failure from an individual check.
set -e exits the script when run_check returns 1. The later checks and the summary do not run after the first failure.
Record the failure in FAILED, then return 0. Let main return the aggregate result.
🤖 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 `@packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh` around
lines 78 - 87, Update the failure branch of run_check so it still increments
FAILED, reports the failure, and records the failed check, but returns 0 instead
of 1. Preserve main’s aggregate-result handling so all checks and the final
summary execute before the overall status is returned.
| # Check file sizes | ||
| local large_files=$(find src/ -name "*.py" -size +50k) | ||
| if [ -n "$large_files" ]; then | ||
| print_warning "Large files found: $large_files" | ||
| add_check_result "File sizes" "warning" "Large files detected" "$large_files" | ||
| TOTAL=$((TOTAL + 1)) | ||
| FAILED=$((FAILED + 1)) | ||
| else | ||
| PASSED=$((PASSED + 1)) | ||
| print_success "All files are under size limit" | ||
| add_check_result "File sizes" "passed" "All files under size limit" "" | ||
| fi | ||
|
|
||
| # Check cyclomatic complexity | ||
| local complex_files=$(radon cc src/ -nb -a | grep -v "F" | awk '$2 > 10 {print $1}') | ||
| if [ -n "$complex_files" ]; then | ||
| print_warning "Complex files found: $complex_files" | ||
| add_check_result "Complexity" "warning" "Complex files detected" "$complex_files" | ||
| TOTAL=$((TOTAL + 1)) | ||
| FAILED=$((FAILED + 1)) | ||
| else | ||
| PASSED=$((PASSED + 1)) | ||
| print_success "All files have acceptable complexity" | ||
| add_check_result "Complexity" "passed" "All files have acceptable complexity" "" | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Increment TOTAL for successful metric checks.
The successful branches increment PASSED but not TOTAL. The summary reports fewer total checks than it ran.
Increment TOTAL before each file-size and complexity condition.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 145-145: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
[failure] 158-158: Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.
🤖 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 `@packages/phenokit-config-kit/linting/ci-cd/scripts/quality-gate.sh` around
lines 143 - 167, Update the successful branches of the file-size check and
complexity check in the quality-gate script to increment TOTAL before
incrementing PASSED, ensuring both metric checks are counted regardless of
outcome.
| # Check if commit message follows conventional commits format | ||
| check_conventional_commits() { | ||
| local msg="$1" | ||
|
|
||
| # Check for conventional commits format | ||
| if echo "$msg" | grep -qE "^(feat|fix|docs|style|refactor|perf|test|chore|build|ci|revert|wip)(\([^)]+\))?: "; then | ||
| return 0 | ||
| fi | ||
|
|
||
| # If not conventional commits, check for minimum quality | ||
| if check_commit_message "$msg"; then | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove wip from the accepted conventional types.
check_commit_message rejects WIP messages. However, check_conventional_commits accepts wip: ... before it calls that validation. This bypasses the hook policy.
🤖 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 `@packages/phenokit-config-kit/linting/git-hooks/commit-msg` around lines 82 -
94, Remove wip from the accepted type alternatives in check_conventional_commits
so WIP messages reach check_commit_message and are rejected according to the
hook policy.
| # Get staged Python files | ||
| get_staged_py_files() { | ||
| git diff --cached --name-only --diff-filter=AM -- "*.py" "*.pyi" | ||
| } | ||
|
|
||
| # Get staged files for other linters | ||
| get_staged_files() { | ||
| git diff --cached --name-only --diff-filter=AM |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve staged pathnames when invoking linters.
git diff --name-only and echo "$files" | xargs split filenames on whitespace. A staged pathname with spaces or newlines is passed as multiple arguments. A pathname beginning with - can also be parsed as a tool option.
Use git diff -z, arrays or mapfile -d '', and -- before file arguments.
Also applies to: 50-78, 92-119
🤖 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 `@packages/phenokit-config-kit/linting/git-hooks/pre-commit` around lines 30 -
37, Update get_staged_py_files, get_staged_files, and the related linter
invocation blocks to preserve staged pathnames containing whitespace or newlines
by using NUL-delimited git diff output with arrays or mapfile -d ''. Pass
filenames safely with -- before file arguments so pathnames beginning with '-'
are not treated as options.
| ``` | ||
|
|
||
| Install pre-commit hooks: | ||
| ```bash |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the reported Markdown lint violations.
Add blank lines around headings and fenced blocks. Add a language identifier to the project-structure fenced block.
Also applies to: 115-120, 163-174, 196-196
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 45-45: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 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 `@packages/phenokit-config-kit/README.md` at line 45, Fix the Markdown lint
violations in the README by adding required blank lines around headings and
fenced code blocks, and specify an appropriate language identifier for the
project-structure fenced block. Apply the same formatting corrections to the
additional referenced sections.
Source: Linters/SAST tools
| ```bash | ||
| # Use basic configuration | ||
| pytest --ini=config-kit/pytest/basic.ini | ||
|
|
||
| # Use comprehensive configuration | ||
| pytest --ini=config-kit/pytest/comprehensive.ini | ||
|
|
||
| # Use CI configuration | ||
| pytest --ini=config-kit/pytest/ci.ini | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files and snippets:"
for f in packages/phenokit-config-kit/README.md packages/phenokit-config-kit/pytest/README.md; do
if [ -f "$f" ]; then
echo "--- $f"
wc -l "$f"
else
echo "missing $f"
fi
done
echo
echo "Occurrences of pytest --ini or pytest -c in markdown:"
rg -n "pytest\s+(--ini|-c)" packages/phenokit-config-kit/README.md packages/phenokit-config-kit/pytest/README.md || true
echo
echo "Current pytest config option help (if pytest is available):"
if command -v pytest >/dev/null 2>&1; then
pytest --help | awk '
/-c, \/\*config-file\//,/^$|^-[A-Za-z]/ {
if (index($0, "pytest") == 0) exit
print
}' || true
else
echo "pytest command not available"
fi
echo
echo "Installed pytest package option metadata (python, if available):"
python3 - <<'PY'
import subprocess, sys
try:
import pytest
print("pytest", getattr(pytest, "__version__", "<unknown>"))
except Exception as e:
print("pytest import failed:", type(e).__name__, e)
try:
result = subprocess.run([sys.executable, "-m", "pytest", "--help"], text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=30)
for line in result.stdout.splitlines():
if "Config-file" in line or "-c - file" in line or "-c file" in line or "-c file" in line:
print(line)
for i in range(1, 6):
print(result.stdout.splitlines()[result.stdout.splitlines().index(line)+i])
break
except Exception as e:
print("pytest help failed:", type(e).__name__, e)
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 1840
🌐 Web query:
pytest command line options -c config file --ini documentation
💡 Result:
To manage command-line options and configuration files in pytest, you can use several configuration files to store default settings, which effectively act as persistent command-line arguments [1][2]. Configuration Files Pytest looks for configuration files in the root directory of your repository [1]. Supported files include: - pytest.ini: Takes high precedence [1]. - pyproject.toml: Uses a [tool.pytest.ini_options] table [1]. - tox.ini: Uses a [pytest] section [1]. - setup.cfg: Uses a [tool:pytest] section (though less recommended than other formats) [1][3]. Adding Default Options The addopts configuration option allows you to set default command-line arguments that will be applied every time you run pytest [2]. This is useful for avoiding repetitive flags [2]. Example (using pytest.ini): [pytest] addopts = -ra -q When you run pytest, these options are automatically included [2]. If you provide conflicting arguments on the actual command line, those will generally override the defaults in your configuration file [2]. Getting Help You can view a full list of available command-line and configuration file options by running the following command in your terminal [1][4]: pytest -h This command prints both standard command-line flags and the options that can be set via configuration files [1][4]. Additionally, you can use the @ prefix to read command-line arguments from a text file, for example: pytest @tests_to_run.txt [5]
Citations:
- 1: https://docs.pytest.org/en/stable/reference/customize.html
- 2: https://docs.pytest.org/en/stable/example/simple.html
- 3: https://docs.pytest.org/en/6.2.x/customize.html
- 4: https://docs.pytest.org/en/stable/how-to/usage.html
- 5: https://docs.pytest.org/en/8.4.x/how-to/usage.html
🌐 Web query:
pytest reference option --ini --iniconfig
💡 Result:
There are no command-line options named --ini or --iniconfig in standard pytest [1][2]. It is likely these terms are being confused with one of the following concepts: 1. INI-style configuration files: Pytest supports configuration through files such as pytest.ini, tox.ini, and setup.cfg [1][3]. These are not invoked via a command-line flag but are automatically detected by pytest based on their filename and location in your project directory [1][4]. 2. The -c option: You can specify a custom configuration file path using the -c command-line option (e.g., pytest -c my_config.ini) [3][5]. 3. The -o/--override-ini option: If you want to override specific configuration settings from the command line without modifying your configuration file, you use the -o or --override-ini flag followed by option=value pairs (e.g., pytest -o cache_dir=my_cache) [2][6]. 4. The iniconfig library: This is a separate, small Python library used internally by pytest for parsing INI files [7][8]. It is not a user-facing command-line interface for pytest [8]. To see all available command-line options and supported configuration file settings, you can run: pytest -h This command displays both built-in command-line options and settings that can be defined in configuration files [1][9].
Citations:
- 1: https://docs.pytest.org/en/stable/reference/customize.html
- 2: https://github.com/pytest-dev/pytest/blob/main/src/_pytest/helpconfig.py
- 3: https://docs.pytest.org/en/8.2.x/reference/customize.html
- 4: https://pytest.org/en/8.2.x/reference/customize.html
- 5: Specifying configuration(
-c config/pytest.ini) from non-project-root directory leads to local fixtures ignore pytest-dev/pytest#13246 - 6: https://pytest.org/en/latest/reference/customize.html
- 7: https://github.com/pytest-dev/pytest/blob/90465694/src/_pytest/config/findpaths.py
- 8: https://github.com/pytest-dev/iniconfig
- 9: https://pytest.org/en/8.1.x/reference/customize.html
Use -c to select a pytest configuration file.
--ini is not a pytest option. Replace the examples with pytest -c <config-file>; use -o only when overriding specific INI values.
packages/phenokit-config-kit/README.md#L51-L60: replace each--ini=config-kit/pytest/...command with-c config-kit/pytest/....packages/phenokit-config-kit/README.md#L210-L214: replace--ini=pytest/comprehensive.iniwith-c pytest/comprehensive.ini.packages/phenokit-config-kit/pytest/README.md#L18-L40: replace all--ini=pytest/...commands with-c pytest/....packages/phenokit-config-kit/pytest/README.md#L541-L549: replace the CI command withpytest -c pytest/ci.ini.packages/phenokit-config-kit/pytest/README.md#L585-L593: replace both debug commands with-c pytest/comprehensive.ini.
📍 Affects 2 files
packages/phenokit-config-kit/README.md#L51-L60(this comment)packages/phenokit-config-kit/README.md#L210-L214packages/phenokit-config-kit/pytest/README.md#L18-L40packages/phenokit-config-kit/pytest/README.md#L541-L549packages/phenokit-config-kit/pytest/README.md#L585-L593
🤖 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 `@packages/phenokit-config-kit/README.md` around lines 51 - 60, Replace the
invalid --ini pytest arguments with -c configuration-file selection in
packages/phenokit-config-kit/README.md at lines 51-60 and 210-214, and
packages/phenokit-config-kit/pytest/README.md at lines 18-40, 541-549, and
585-593. Update every specified command, preserving each existing configuration
path and using -o only for INI value overrides.
| tempdir = Path(tempfile.mkdtemp(prefix="phenotype-sdk-install-")) | ||
| venv_dir = tempdir / ".venv" | ||
| proc = subprocess.run( | ||
| ( | ||
| "uv", | ||
| "venv", | ||
| "--python", | ||
| python_version, | ||
| str(venv_dir), | ||
| ), | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| assert proc.returncode == 0, ( | ||
| f"uv venv creation failed (rc={proc.returncode}):\n" | ||
| f"stdout={proc.stdout}\nstderr={proc.stderr}" | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Changed files/stat:"
git diff --stat || true
echo
echo "Locate test_install_path.py:"
fd -a 'test_install_path.py' . || true
echo
echo "Outline:"
ast-grep outline tests/test_install_path.py --match _create_temp_venv --view expanded || true
echo
echo "Lines 1-160:"
cat -n tests/test_install_path.py | sed -n '1,160p'Repository: KooshaPari/phenotype-python-sdk
Length of output: 6569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search uv subprocess usage and timeout constants:"
rg -n "_INSTALL_TIMEOUT_SECS|subprocess\.run|uv venv|_create_temp_venv|_run_uv_pip_install|tempdir|cleanup" tests/test_install_path.py
echo
echo "Relevant tests around teardown:"
cat -n tests/test_install_path.py | sed -n '110,135p'Repository: KooshaPari/phenotype-python-sdk
Length of output: 2367
Bound the uv venv subprocess and clean up tempdir on failure.
tempdir is created before running uv venv, but the subprocess has no timeout and the temp_venv fixture cleanup runs only after _create_temp_venv returns. If uv venv hangs or raises, it can block longer than _INSTALL_TIMEOUT_SECS or leave the temporary directory uncleaned.
Add timeout=_INSTALL_TIMEOUT_SECS to the uv venv subprocess.run() call, and wrap venv creation in an exception-safe block that removes tempdir after a failed uv venv.
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 54-65: Command coming from incoming request
Context: subprocess.run(
(
"uv",
"venv",
"--python",
python_version,
str(venv_dir),
),
capture_output=True,
text=True,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🤖 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 `@tests/test_install_path.py` around lines 53 - 70, Update the venv creation
flow around the subprocess.run call to pass timeout=_INSTALL_TIMEOUT_SECS, and
wrap the uv venv invocation and return-code assertion in exception-safe cleanup
that removes tempdir when creation fails or raises. Preserve successful tempdir
usage for the returned virtual environment.
| # Skip the entire module if uv is missing — this is an upstream packaging | ||
| # regression, the test is meaningless without uv available. | ||
| pytestmark = pytest.mark.skipif( | ||
| not _uv_available(), | ||
| reason="uv CLI not available on PATH; install uv to run packaging tests", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(tests/test_install_path\.py$|.*tox.*$|.*\.ci.*$|.*\.github.*$|.*setup.*$|pyproject\.toml$|uv.lock$)' || true
echo "== test_install_path outline/size =="
if [ -f tests/test_install_path.py ]; then
wc -l tests/test_install_path.py
sed -n '1,180p' tests/test_install_path.py | cat -n
fi
echo "== uv references in tests =="
rg -n "uv|_uv_available|pytestmark|install_path|uv" tests pyproject.toml tox.ini .github 2>/dev/null || true
echo "== relevant CI config snippets =="
for f in $(git ls-files .github | head -50); do
echo "--- $f"
sed -n '1,220p' "$f"
doneRepository: KooshaPari/phenotype-python-sdk
Length of output: 35205
Require uv in the CI packaging path for tests/test_install_path.py.
The tests use uv for venv creation and install commands, and the module-level skip makes the run pass when uv is missing. Install uv in the relevant CI job and make the packaging job fail when uv is unavailable, or add a dedicated failure step that fails only when tests/test_install_path.py is included.
🤖 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 `@tests/test_install_path.py` around lines 104 - 109, Update the CI packaging
job that runs tests/test_install_path.py to install uv before testing and fail
explicitly when the uv CLI is unavailable. Remove or bypass the module-level
pytestmark skipif in tests/test_install_path.py for this CI path so missing uv
cannot produce a passing packaging run.
| def test_pyproject_declares_empty_tool_setuptools() -> None: | ||
| """Pin the chosen fix: ``[tool.setuptools] py-modules = []`` plus a | ||
| no-op ``packages.find`` table that disables auto-discovery.""" | ||
| text = PYPROJECT.read_text() | ||
| assert "[tool.setuptools]" in text, ( | ||
| "missing [tool.setuptools] section in root pyproject.toml" | ||
| ) | ||
| assert "py-modules = []" in text, ( | ||
| "[tool.setuptools] should declare py-modules = [] to suppress " | ||
| "flat-layout auto-discovery" | ||
| ) | ||
| assert "[tool.setuptools.packages.find]" in text, ( | ||
| "missing [tool.setuptools.packages.find] section; cannot pin " | ||
| "no-package discovery for consumers" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching test_install_path.py:"
fd -a 'test_install_path.py' . || true
PY_FILE="$(fd 'test_install_path.py' . | head -n 1)"
if [ -n "${PY_FILE:-}" ]; then
echo
echo "Test file line count:"
wc -l "$PY_FILE"
echo
echo "Relevant test section:"
sed -n '130,200p' "$PY_FILE" | cat -n
echo
echo "Search for tomllib or install_path tests:"
rg -n "tomllib|test_pyproject_declares_empty_tool_setuptools|py-modules|packages\\.find|namespaces" .
fi
echo
echo "pyproject.toml section:"
if [ -f pyproject.toml ]; then
sed -n '1,220p' pyproject.toml | cat -n
fi
echo
echo "Python/tomllib parser probe:"
python3 - <<'PY'
import ast, tomllib, textwrap
from pathlib import Path
p = next(Path('.').rglob('pyproject.toml'), None)
if not p:
print("NO pyproject.toml")
raise SystemExit
text = p.read_text(encoding='utf-8')
try:
data = tomllib.loads(text)
except Exception as e:
print(f"tomllib.loads failed: {type(e).__name__}: {e}")
data = None
print("has_comment_like_value:", "py-modules = []" in text)
if data is not None:
setuptools = data.get("tool", {}).get("setuptools")
print("setuptools py-modules == []:", setuptools.get("py-modules") == [])
discovery = setuptools.get("packages", {}).get("find") if setuptools else None
print("packages.find:", discovery)
if discovery:
print("packages.find include:", discovery.get("include"))
print("packages.find namespaces:", discovery.get("namespaces"))
PYRepository: KooshaPari/phenotype-python-sdk
Length of output: 11551
🌐 Web query:
Python 3.11 tomllib parses TOML ignores comments documentation
💡 Result:
Yes, the tomllib module introduced in Python 3.11 correctly handles and ignores comments in TOML files [1]. As a TOML 1.0.0-compliant parser [2][3], tomllib adheres to the TOML specification, which defines comments as beginning with a # character and continuing to the end of the line [1][4]. Like other standard TOML parsers, tomllib treats these comments as whitespace and ignores them during the parsing process, extracting only the data structure into a Python dictionary [2][5]. Because tomllib is designed strictly for reading and parsing TOML, it does not preserve comments or formatting if you were to modify the data and attempt to write it back out (a task for which other third-party libraries like tomlkit are recommended) [3][4].
Citations:
- 1: https://inventwithpython.com/blog/toml-python-guide.html
- 2: https://docs.python.org/3/library/tomllib.html
- 3: https://realpython.com/python311-tomllib/
- 4: https://www.playfulpython.com/python-3-11-the-tomllib/
- 5: https://towardsdev.com/python-3-11-toml-parser-20fb06e79bd1
Assert parsed TOML values, not source substrings.
Use tomllib to load pyproject.toml and assert the installed setuptools values: py-modules == [], packages.find.include == [], and packages.find.namespaces == False. The current string assertions can pass even if these values appear only in comments or other sections.
🤖 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 `@tests/test_install_path.py` around lines 165 - 179, Update
test_pyproject_declares_empty_tool_setuptools to parse PYPROJECT with tomllib
and validate the [tool.setuptools] configuration values directly: py-modules
must equal [], packages.find.include must equal [], and packages.find.namespaces
must equal False. Replace the current source-substring assertions while
preserving clear failure messages.
Source: Coding guidelines
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ), | ||
| ) | ||
|
|
||
| items.extend(security_items) |
There was a problem hiding this comment.
CRITICAL: pytest.Function.from_parent does not accept markers=, and parent=None crashes when no tests are collected.
from_parent requires a real pytest node as parent. The loop also passes markers=[...], which is not a supported keyword. Every generated item raises TypeError. Return when items is empty and apply markers with item.add_marker(...) after construction.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| def _get_python_files(self) -> list[Path]: | ||
| """Get all Python files in the project.""" | ||
| python_files = [] | ||
| for root, dirs, files in Path().rglob("*.py"): |
There was a problem hiding this comment.
CRITICAL: Path().rglob("*.py") yields Path objects, not three-tuples.
The loop unpacks each yielded value into root, dirs, and files. A Path is not iterable of length three, so _get_python_files raises TypeError on the first iteration. Iterate the paths directly and filter with path.parts.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return python_files | ||
|
|
||
|
|
||
| def security_test(func: Callable) -> Callable: |
There was a problem hiding this comment.
CRITICAL: Callable is used in type annotations but never imported.
security_test, auth_test, injection_test, xss_test, csrf_test, and sql_injection_test all annotate parameters and return values with Callable. Python evaluates annotations at definition time, so importing this module raises NameError: name 'Callable' is not defined. Add from collections.abc import Callable.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "path_traversal": r"\.\./", | ||
| "command_injection": r"(os\.system|subprocess\.call|subprocess\.run)", | ||
| "unsafe_deserialization": r"(pickle\.loads|marshal\.loads)", | ||
| "weak_crypto": r"(md5|sha1|des|rc4)", |
There was a problem hiding this comment.
WARNING: weak_crypto pattern matches substrings in ordinary English words.
r"(md5|sha1|des|rc4)" with re.IGNORECASE has no word boundaries. des matches inside describe, modes, indexes, and nodes. Almost every Python file triggers a violation, so the vulnerability scan always fails.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| for pattern in self.security_rules["hardcoded_secrets"]: | ||
| matches = re.findall(pattern, content, re.IGNORECASE) | ||
| if matches: | ||
| violations.append(f"{py_file}: Hardcoded secret - {matches}") |
There was a problem hiding this comment.
WARNING: Failure messages interpolate matched secret values into CI logs.
_test_hardcoded_secrets and _test_password_security append matches to the violation string. The regexes capture the full assignment including the literal value. When these tests fail, the real secret appears in the build log and any published test report. Report only the file path and line number.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| @pytest.hookimpl(tryfirst=True) | ||
| def pytest_collection_modifyitems(self, config, items): | ||
| """Add architecture tests to the test collection.""" | ||
| if not self.enforce_imports and not self.enforce_dependencies: |
There was a problem hiding this comment.
WARNING: store_true with default=True makes both flags permanently enabled.
--enforce-imports and --enforce-dependencies default to True and can never be set to False. The guard at line 32 is therefore unreachable. Use --no-enforce-imports with action="store_false" and dest="enforce_imports".
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ), | ||
| ) | ||
|
|
||
| items.extend(architecture_items) |
There was a problem hiding this comment.
CRITICAL: pytest.Function.from_parent does not accept markers=, and parent=None crashes when no tests are collected.
from_parent requires a real pytest node as parent. The loop also passes markers=[...], which is not a supported keyword. Every generated item raises TypeError. Return when items is empty and apply markers with item.add_marker(...) after construction.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| for py_file in self._get_python_files(): | ||
| file_path = str(py_file) | ||
| if not file_path.startswith("src/"): |
There was a problem hiding this comment.
WARNING: startswith("src/") never matches paths from os.walk(".").
os.walk(".") yields paths like ./src/domain/model.py. The string representation always starts with ./, so file_path.startswith("src/") is always false. The dependency-direction check silently skips every file.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| violations = [] | ||
|
|
||
| for py_file in self._get_python_files(): | ||
| line_count = sum(1 for _ in open(py_file, encoding="utf-8")) |
There was a problem hiding this comment.
WARNING: File handle is never closed in the line-count loop.
open(py_file, encoding="utf-8") inside the generator expression is never closed explicitly. Use a with block to ensure the handle is closed after counting.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| duration = end_time - start_time | ||
| memory_delta = end_memory - start_memory | ||
|
|
||
| if duration > threshold: |
There was a problem hiding this comment.
WARNING: pytest.fail inside finally hides the original exception.
If the wrapped block raises, the finally clause still evaluates the threshold. When the duration exceeds the threshold, pytest.fail replaces the real error with a timing message. The print on line 130 also never executes on failure. Move the threshold check outside the try block so it only runs when the body succeeds.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 10 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 235.9K · Output: 39.3K · Cached: 6.4M |
|
User description
Summary
Intake of the genuinely-new
phenokit-config-kitpackage from thephenotype-sdkPhase 3 decomposition.What's included
phenotype-sdk/lang/python/packages/phenokit-config-kit/)pyproject.tomlworkspace integration:packages/phenokit-config-kitto[tool.uv.workspace]membersphenokit-config-kit = { workspace = true }to[tool.uv.sources]Origin
Per the rescued
ORIGIN.md:What was NOT brought in (corrected Phase 3 disposition)
lang/python/mcp/agentmcp/packages/agentmcp-hex/ORIGIN.md:agentmcpwas the pre-rename version.packages/agentmcp-hex/on 2026-06-18 with module rename toagentmcp_hexand version bump to 0.3.0.cli.pydiffers by 1 trivial line (import sys + trailing newline)..preservation-work/2026-08-08-sdk-decomp/phenotype-python-rescue/REJECTION_LOG.txtBundle backup
.preservation-work/2026-08-08-sdk-decomp/phenotype-sdk/phenotype-sdk-all.bundleaca92b82...git bundle verify(returnsis okay)Dockets
plans/dockets/N00-phenotype-sdk-decomp-operator-summary.mdplans/dockets/N03-phenotype-sdk-decomp-phase-3-python.mdStats
Test plan
cd phenotype-python-sdk && uv synccd packages/phenokit-config-kit && uv run pytestphenokit-config-kitis importable in workspaceuv lock)CodeAnt-AI Description
Add a reusable Python configuration and testing toolkit and make the workspace installable from a path
What Changed
phenokit-config-kitpackage with pytest configurations, architecture/performance/security plugins, reusable fixtures, test-data generation, linting settings, and pre-commit hooksokfandpackagesdirectoriesImpact
✅ Reusable pytest fixtures and test-data generation✅ Configurable architecture, performance, and security checks✅ Successful workspace path and editable installs💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.