Codex-generated pull request#15
Open
NguyenCuong1989 wants to merge 2 commits into
Open
Conversation
Hướng dẫn cho người reviewThêm một helper cấu hình dùng chung để kiểm soát việc cho phép sử dụng các stub cho runtime/protocol thông qua biến môi trường Sơ đồ trình tự khi import HAIOSRuntime với stub fallback có thể cấu hìnhsequenceDiagram
actor PythonImporter
participant haios_runtime as haios_runtime_module
participant hyperai_config as hyperai_config_module
participant HAIOSRuntimeImpl as HAIOSRuntimeImpl_module
participant StubHAIOSRuntime as HAIOSRuntime_stub
PythonImporter->>haios_runtime: import haios_runtime
activate haios_runtime
haios_runtime->>HAIOSRuntimeImpl: import HAIOSRuntimeImpl
alt HAIOSRuntimeImpl import succeeds
HAIOSRuntimeImpl-->>haios_runtime: HAIOSRuntimeImpl
haios_runtime-->>PythonImporter: bind HAIOSRuntime = HAIOSRuntimeImpl
else HAIOSRuntimeImpl raises ImportError
HAIOSRuntimeImpl-->>haios_runtime: ImportError
haios_runtime->>hyperai_config: allow_stubs()
activate hyperai_config
hyperai_config-->>haios_runtime: bool allow
deactivate hyperai_config
alt allow is false
haios_runtime-->>PythonImporter: re_raise ImportError
else allow is true
haios_runtime-->>PythonImporter: define HAIOSRuntime_stub as HAIOSRuntime
end
end
deactivate haios_runtime
Sơ đồ lớp cho cấu hình stub dùng chung và các module runtime/protocolclassDiagram
class hyperai_config {
+bool allow_stubs()
}
class haios_runtime {
+HAIOSRuntime
}
class dr_protocol {
+DRProtocol
}
haios_runtime ..> hyperai_config : uses_allow_stubs
dr_protocol ..> hyperai_config : uses_allow_stubs
Sơ đồ luồng cho việc parse biến môi trường allow_stubsflowchart TD
A[Read HYPERAI_ALLOW_STUBS from environment
default empty string] --> B[Strip whitespace
and lower case value]
B --> C{Value in
1, true, yes, on?}
C -- Yes --> D[Return True]
C -- No --> E[Return False]
Thay đổi theo từng tệp
Mẹo và các lệnhTương tác với Sourcery
Tùy chỉnh trải nghiệm của bạnTruy cập dashboard để:
Nhận hỗ trợ
Original review guide in EnglishReviewer's GuideAdds a shared configuration helper to control whether stub runtime/protocol implementations are allowed via the HYPERAI_ALLOW_STUBS environment variable, uses it to gate the HAIOSRuntime and DRProtocol stub fallbacks, and introduces unit tests for the new configuration behavior. Sequence diagram for importing HAIOSRuntime with configurable stub fallbacksequenceDiagram
actor PythonImporter
participant haios_runtime as haios_runtime_module
participant hyperai_config as hyperai_config_module
participant HAIOSRuntimeImpl as HAIOSRuntimeImpl_module
participant StubHAIOSRuntime as HAIOSRuntime_stub
PythonImporter->>haios_runtime: import haios_runtime
activate haios_runtime
haios_runtime->>HAIOSRuntimeImpl: import HAIOSRuntimeImpl
alt HAIOSRuntimeImpl import succeeds
HAIOSRuntimeImpl-->>haios_runtime: HAIOSRuntimeImpl
haios_runtime-->>PythonImporter: bind HAIOSRuntime = HAIOSRuntimeImpl
else HAIOSRuntimeImpl raises ImportError
HAIOSRuntimeImpl-->>haios_runtime: ImportError
haios_runtime->>hyperai_config: allow_stubs()
activate hyperai_config
hyperai_config-->>haios_runtime: bool allow
deactivate hyperai_config
alt allow is false
haios_runtime-->>PythonImporter: re_raise ImportError
else allow is true
haios_runtime-->>PythonImporter: define HAIOSRuntime_stub as HAIOSRuntime
end
end
deactivate haios_runtime
Class diagram for shared stub configuration and runtime/protocol modulesclassDiagram
class hyperai_config {
+bool allow_stubs()
}
class haios_runtime {
+HAIOSRuntime
}
class dr_protocol {
+DRProtocol
}
haios_runtime ..> hyperai_config : uses_allow_stubs
dr_protocol ..> hyperai_config : uses_allow_stubs
Flow diagram for allow_stubs environment variable parsingflowchart TD
A[Read HYPERAI_ALLOW_STUBS from environment
default empty string] --> B[Strip whitespace
and lower case value]
B --> C{Value in
1, true, yes, on?}
C -- Yes --> D[Return True]
C -- No --> E[Return False]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||
PR Code Suggestions ✨Latest suggestions up to d0bf5c7
Previous suggestionsSuggestions up to commit d0ad92c
|
||||||||||||||||||||||||
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - mình đã tìm thấy 1 vấn đề và để lại một số phản hồi tổng quan:
- Helper
allow_stubsmới đọc biến môi trường ở mỗi lần gọi; nếu hàm này được dùng trên các đường chạy nóng (hot path) thì bạn có thể muốn cache giá trị đã được parse ngay khi import module (kèm theo một cách rõ ràng để ghi đè trong test) để tránh phải parse lặp lại. - Trong các bài test, thay vì tự tải
config.pybằngimportlib.util.spec_from_file_location, hãy cân nhắc import trực tiếphyperai.configđể các bài test sử dụng cùng đường import và hành vi đóng gói như code chạy trong môi trường production.
Prompt cho AI Agents
Please address the comments from this code review:
## Overall Comments
- Helper `allow_stubs` mới đọc biến môi trường ở mỗi lần gọi; nếu hàm này được dùng trên các đường chạy nóng (hot path) thì bạn có thể muốn cache giá trị đã được parse ngay khi import module (kèm theo một cách rõ ràng để ghi đè trong test) để tránh phải parse lặp lại.
- Trong các bài test, thay vì tự tải `config.py` bằng `importlib.util.spec_from_file_location`, hãy cân nhắc import trực tiếp `hyperai.config` để các bài test sử dụng cùng đường import và hành vi đóng gói như code chạy trong môi trường production.
## Individual Comments
### Comment 1
<location> `tests/test_config_allow_stubs.py:37-41` </location>
<code_context>
+ os.environ["HYPERAI_ALLOW_STUBS"] = value
+ self.assertTrue(allow_stubs())
+
+ def test_falsy_values(self):
+ for value in ("0", "false", "off", "no", "random"):
+ with self.subTest(value=value):
+ os.environ["HYPERAI_ALLOW_STUBS"] = value
+ self.assertFalse(allow_stubs())
+
+ def test_missing_env_var_defaults_false(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Thêm các giá trị falsy với khoảng trắng và kiểu chữ khác nhau để phản chiếu các test truthy
Các test truthy hiện đã bao phủ trường hợp thay đổi khoảng trắng và kiểu chữ (ví dụ: `" yes "`, `"On"`), nhưng các test falsy chỉ dùng các giá trị chữ thường đơn giản. Để phản chiếu hành vi và kiểm thử đầy đủ logic `.strip().lower()`, hãy thêm các case falsy tương tự như `" false "`, `" NO "`, và `" Off "` vào `test_falsy_values`.
```suggestion
def test_falsy_values(self):
for value in ("0", "false", "off", "no", "random", " false ", " NO ", " Off "):
with self.subTest(value=value):
os.environ["HYPERAI_ALLOW_STUBS"] = value
self.assertFalse(allow_stubs())
```
</issue_to_address>Sourcery miễn phí cho mã nguồn mở - nếu bạn thấy review này hữu ích, hãy cân nhắc chia sẻ ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The new
allow_stubshelper reads the environment on every call; if this is used on hot paths you may want to cache the parsed value at module import time (with an explicit way to override in tests) to avoid repeated parsing. - In the tests, instead of manually loading
config.pyviaimportlib.util.spec_from_file_location, consider importinghyperai.configdirectly so the tests exercise the same import path and packaging behavior as production code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `allow_stubs` helper reads the environment on every call; if this is used on hot paths you may want to cache the parsed value at module import time (with an explicit way to override in tests) to avoid repeated parsing.
- In the tests, instead of manually loading `config.py` via `importlib.util.spec_from_file_location`, consider importing `hyperai.config` directly so the tests exercise the same import path and packaging behavior as production code.
## Individual Comments
### Comment 1
<location> `tests/test_config_allow_stubs.py:37-41` </location>
<code_context>
+ os.environ["HYPERAI_ALLOW_STUBS"] = value
+ self.assertTrue(allow_stubs())
+
+ def test_falsy_values(self):
+ for value in ("0", "false", "off", "no", "random"):
+ with self.subTest(value=value):
+ os.environ["HYPERAI_ALLOW_STUBS"] = value
+ self.assertFalse(allow_stubs())
+
+ def test_missing_env_var_defaults_false(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Add falsy value cases with varying whitespace and casing to mirror the truthy tests
The truthy tests already cover whitespace and case variations (e.g., `" yes "`, `"On"`), but the falsy tests only use plain lowercase values. To mirror the behavior and fully exercise the `.strip().lower()` logic, please add analogous falsy cases such as `" false "`, `" NO "`, and `" Off "` to `test_falsy_values`.
```suggestion
def test_falsy_values(self):
for value in ("0", "false", "off", "no", "random", " false ", " NO ", " Off "):
with self.subTest(value=value):
os.environ["HYPERAI_ALLOW_STUBS"] = value
self.assertFalse(allow_stubs())
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Collaborator
Author
|
ok |
Contributor
|
👋 This pull request has been automatically marked as stale because it has not had recent activity. What happens next:
Tips to get your PR merged:
Thank you for your contribution! 🙏 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared configuration flag to explicitly control whether stub implementations may be used when “real” runtime/protocol implementations are unavailable.
Changes:
- Added
allow_stubs()helper insrc/hyperai/config.pyto parseHYPERAI_ALLOW_STUBS. - Guarded stub fallbacks in
HAIOSRuntimeandDRProtocolbehind the shared flag. - Added unit tests for
HYPERAI_ALLOW_STUBSparsing/default behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/hyperai/config.py | Adds allow_stubs() env-var parsing helper. |
| src/hyperai/core/haios_runtime.py | Blocks stub runtime fallback unless explicitly allowed. |
| src/hyperai/protocols/dr_protocol.py | Blocks stub protocol fallback unless explicitly allowed. |
| tests/test_config_allow_stubs.py | Adds tests for allow_stubs() parsing and defaults. |
Comment on lines
+14
to
18
| from hyperai.config import allow_stubs as _allow_stubs | ||
|
|
||
| # Add root directory to path to import from root-level modules | ||
| root_dir = Path(__file__).parent.parent.parent.parent | ||
| sys.path.insert(0, str(root_dir)) |
There was a problem hiding this comment.
from hyperai.config ... is likely an invalid import in this codebase: modules are imported as src.hyperai... (e.g., tests import src.hyperai), so there may be no top-level hyperai package on sys.path. This will break import src.hyperai because src.hyperai.__init__ imports this module. Use a package-relative import (e.g., from ..config import allow_stubs) or import via src.hyperai.config so it resolves consistently in the existing module layout.
Comment on lines
+14
to
18
| from hyperai.config import allow_stubs as _allow_stubs | ||
|
|
||
| # Add root directory to path to import from root-level modules | ||
| root_dir = Path(__file__).parent.parent.parent.parent | ||
| sys.path.insert(0, str(root_dir)) |
There was a problem hiding this comment.
from hyperai.config ... is likely an invalid import in this repository’s module layout (tests import src.hyperai, not hyperai). Since src.hyperai.__init__ imports src.hyperai.protocols.dr_protocol, this will raise ModuleNotFoundError in normal test/runtime execution. Switch to a package-relative import (e.g., from ..config import allow_stubs) or import via src.hyperai.config to match the established src.hyperai namespace.
Comment on lines
+4
to
+16
| import importlib.util | ||
| import os | ||
| import unittest | ||
| from pathlib import Path | ||
|
|
||
| PROJECT_ROOT = Path(__file__).resolve().parents[1] | ||
| CONFIG_MODULE_PATH = PROJECT_ROOT / "src" / "hyperai" / "config.py" | ||
|
|
||
| spec = importlib.util.spec_from_file_location("hyperai_config", CONFIG_MODULE_PATH) | ||
| config_module = importlib.util.module_from_spec(spec) | ||
| assert spec is not None and spec.loader is not None | ||
| spec.loader.exec_module(config_module) | ||
| allow_stubs = config_module.allow_stubs |
There was a problem hiding this comment.
The test loads src/hyperai/config.py via importlib at import time and uses a bare assert to validate the loader. This bypasses the normal package import path (so it won’t catch issues with src.hyperai.config being unimportable) and assert can be stripped with python -O. Prefer importing allow_stubs through the same module namespace used by the codebase (e.g., from src.hyperai.config import allow_stubs after adjusting sys.path like other tests) and replace the assert with an explicit exception if the spec/loader is missing.
Suggested change
| import importlib.util | |
| import os | |
| import unittest | |
| from pathlib import Path | |
| PROJECT_ROOT = Path(__file__).resolve().parents[1] | |
| CONFIG_MODULE_PATH = PROJECT_ROOT / "src" / "hyperai" / "config.py" | |
| spec = importlib.util.spec_from_file_location("hyperai_config", CONFIG_MODULE_PATH) | |
| config_module = importlib.util.module_from_spec(spec) | |
| assert spec is not None and spec.loader is not None | |
| spec.loader.exec_module(config_module) | |
| allow_stubs = config_module.allow_stubs | |
| import os | |
| import sys | |
| import unittest | |
| from pathlib import Path | |
| PROJECT_ROOT = Path(__file__).resolve().parents[1] | |
| if str(PROJECT_ROOT) not in sys.path: | |
| sys.path.insert(0, str(PROJECT_ROOT)) | |
| from src.hyperai.config import allow_stubs |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Codex generated this pull request, but encountered an unexpected error after generation. This is a placeholder PR message.
Codex Task
Summary by Sourcery
Bảo vệ việc sử dụng các bản triển khai stub cho runtime lõi và các thành phần giao thức bằng một cờ cấu hình dùng chung.
Tính năng mới:
HYPERAI_ALLOW_STUBS.Sửa lỗi:
HAIOSRuntimevàDRProtocolkhi bản triển khai thật không khả dụng và stub không được cho phép một cách tường minh.Kiểm thử:
HYPERAI_ALLOW_STUBSvà hành vi mặc định khi biến này không được thiết lập.Original summary in English
Summary by Sourcery
Guard use of stub implementations for core runtime and protocol components behind a shared configuration flag.
New Features:
Bug Fixes:
Tests:
Sửa lỗi:
HAIOSRuntimevàDRProtocolkhi stub không được cho phép một cách rõ ràng.Cải tiến:
Kiểm thử:
HYPERAI_ALLOW_STUBSvà hành vi mặc định.Original summary in English
Summary by Sourcery
Bảo vệ việc sử dụng các bản triển khai stub cho runtime lõi và các thành phần giao thức bằng một cờ cấu hình dùng chung.
Tính năng mới:
HYPERAI_ALLOW_STUBS.Sửa lỗi:
HAIOSRuntimevàDRProtocolkhi bản triển khai thật không khả dụng và stub không được cho phép một cách tường minh.Kiểm thử:
HYPERAI_ALLOW_STUBSvà hành vi mặc định khi biến này không được thiết lập.Original summary in English
Summary by Sourcery
Guard use of stub implementations for core runtime and protocol components behind a shared configuration flag.
New Features:
Bug Fixes:
Tests:
PR Type
Bug fix, Enhancement, Tests
Description
Add shared configuration helper to control stub implementation usage via environment variable
Prevent unintended use of stub HAIOSRuntime and DRProtocol when stubs not explicitly allowed
Add comprehensive unit tests for HYPERAI_ALLOW_STUBS parsing and default behavior
File Walkthrough
config.py
New shared config helper for stub controlsrc/hyperai/config.py
allow_stubs()function"yes", "on" values
haios_runtime.py
Add stub allowance guard to HAIOSRuntimesrc/hyperai/core/haios_runtime.py
allow_stubsconfiguration helper from hyperai.configimplementation missing
enabled
dr_protocol.py
Add stub allowance guard to DRProtocolsrc/hyperai/protocols/dr_protocol.py
allow_stubsconfiguration helper from hyperai.configimplementation missing
enabled
test_config_allow_stubs.py
Unit tests for allow_stubs configuration parsingtests/test_config_allow_stubs.py
allow_stubs()function behaviorwhitespace