Repository navigation
perf(noema): parallelize bounded changed-file evidence fetches #1011
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
d0372f0
0d1e699
b333b6f
7212d9b
47770c2
a012817
9349275
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| --- a/scripts/ci/noema_review_gate.py | ||
| +++ b/scripts/ci/noema_review_gate.py | ||
| @@ -5,6 +5,7 @@ | ||
|
|
||
| import argparse | ||
| import base64 | ||
| +import concurrent.futures | ||
| import ipaddress | ||
| import json | ||
| import os | ||
| @@ -343,17 +343,31 @@ | ||
| if not paths: | ||
| return "Changed file context unavailable: PR reported no changed files." | ||
| sections: list[str] = [] | ||
| - for path in paths[:MAX_CONTEXT_FILES]: | ||
| + | ||
| + target_paths = paths[:MAX_CONTEXT_FILES] | ||
| + if not target_paths: | ||
| + return "Changed file context unavailable: no paths to check." | ||
| + | ||
| + def _fetch_file_content(path: str) -> tuple[str, str | None, str | None]: | ||
| try: | ||
| content = fetch_head_file_content(repo, path, head_sha) | ||
| + return path, content, None | ||
| except RuntimeError as exc: | ||
| - reason = scrub_sensitive_data(str(exc)) or "unknown error" | ||
| - sections.append(f"### {path}\nUnavailable from head content API: {reason}") | ||
| + return path, None, scrub_sensitive_data(str(exc)) or "unknown error" | ||
| + | ||
| + max_workers = min(10, len(target_paths)) | ||
| + with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: | ||
| + results = list(executor.map(_fetch_file_content, target_paths)) | ||
| + | ||
| + for path, content, error in results: | ||
| + if error: | ||
| + sections.append(f"### {path}\nUnavailable from head content API: {error}") | ||
| continue | ||
| if not content: | ||
| sections.append(f"### {path}\nNo UTF-8 text content available from head content API.") | ||
| continue | ||
| sections.append(f"### {path}\n{truncate_text(content, MAX_FILE_CONTEXT_CHARS)}") | ||
| + | ||
| if len(paths) > MAX_CONTEXT_FILES: | ||
| sections.append(f"[{len(paths) - MAX_CONTEXT_FILES} changed files omitted from context budget]") | ||
| return "\n\n".join(sections) |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |||||||||
|
|
||||||||||
| import argparse | ||||||||||
| import base64 | ||||||||||
| import concurrent.futures | ||||||||||
| import ipaddress | ||||||||||
| import json | ||||||||||
| import os | ||||||||||
|
|
@@ -342,17 +343,31 @@ def changed_file_context(repo: str, number: int, head_sha: str) -> str: | |||||||||
| if not paths: | ||||||||||
| return "Changed file context unavailable: PR reported no changed files." | ||||||||||
| sections: list[str] = [] | ||||||||||
| for path in paths[:MAX_CONTEXT_FILES]: | ||||||||||
|
|
||||||||||
| target_paths = paths[:MAX_CONTEXT_FILES] | ||||||||||
| if not target_paths: | ||||||||||
| return "Changed file context unavailable: no paths to check." | ||||||||||
|
Comment on lines
+347
to
+349
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win 도달할 수 없는 분기입니다. 제거하세요. 343-344행에서 이 분기는 테스트로 도달할 수 없습니다. 코딩 가이드라인은 ♻️ 제안 수정-
target_paths = paths[:MAX_CONTEXT_FILES]
- if not target_paths:
- return "Changed file context unavailable: no paths to check."
-As per coding guidelines: "Maintain 100% test coverage and 100% interrogate docstring coverage for code under 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||
|
|
||||||||||
| def _fetch_file_content(path: str) -> tuple[str, str | None, str | None]: | ||||||||||
| try: | ||||||||||
| content = fetch_head_file_content(repo, path, head_sha) | ||||||||||
| return path, content, None | ||||||||||
| except RuntimeError as exc: | ||||||||||
| reason = scrub_sensitive_data(str(exc)) or "unknown error" | ||||||||||
| sections.append(f"### {path}\nUnavailable from head content API: {reason}") | ||||||||||
| return path, None, scrub_sensitive_data(str(exc)) or "unknown error" | ||||||||||
|
Comment on lines
+351
to
+356
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# interrogate 설정과 커버리지 기준을 확인합니다.
fd -H -t f '^(pyproject\.toml|setup\.cfg|tox\.ini|\.interrogaterc)$' --exec rg -n -A 20 'interrogate' {}Repository: ContextualWisdomLab/.github Length of output: 536 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- target function and surrounding code ---'
sed -n '320,380p' scripts/ci/noema_review_gate.py
printf '%s\n' '--- interrogate configuration and dependency ---'
rg -n -A 12 -B 4 'interrogate|pytest|coverage' pyproject.toml setup.cfg tox.ini .interrogaterc 2>/dev/null || true
printf '%s\n' '--- related tests and references ---'
rg -n '_fetch_file_content|noema_review_gate|fetch_head_file_content' scripts tests 2>/dev/null || true
printf '%s\n' '--- interrogate availability and version ---'
python3 - <<'PY'
import importlib.util
spec = importlib.util.find_spec("interrogate")
print("available:", spec is not None)
if spec is not None:
import interrogate
print("version:", getattr(interrogate, "__version__", "unknown"))
PYRepository: ContextualWisdomLab/.github Length of output: 4896 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("scripts/ci/noema_review_gate.py")
tree = ast.parse(path.read_text())
matches = []
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef) and node.name == "_fetch_file_content":
matches.append({
"lineno": node.lineno,
"end_lineno": node.end_lineno,
"has_docstring": ast.get_docstring(node) is not None,
"docstring": ast.get_docstring(node),
})
print(matches)
PY
printf '%s\n' '--- relevant test coverage references ---'
sed -n '270,335p' tests/test_noema_review_gate.pyRepository: ContextualWisdomLab/.github Length of output: 2737 🌐 Web query:
💡 Result: In Python documentation generation and static analysis, there is no standardized, universal "ignore-nested-functions" setting across all tools. Support for documenting or ignoring nested (inner) functions varies significantly by implementation. Standard Library pydoc The standard library pydoc module does not natively document nested functions [1][2]. It is designed to document modules, classes, and functions at the module level [3][4]. Because nested functions are local to the scope in which they are defined, they are typically not accessible to pydoc's introspection mechanism [2]. Sphinx Autodoc Sphinx's autodoc extension generally ignores nested functions by design, as it focuses on generating reference manuals for public APIs [5]. There is no official configuration flag to force autodoc to include them, and the tool often ignores them during its parsing phase [5]. Users requiring custom behavior often use the autodoc-skip-member event in conf.py to programmatically include or exclude members, though this is primarily intended for skipping, not necessarily for exposing non-exported nested objects [6]. Third-Party Tools and Coverage Some third-party docstring coverage and generation tools provide explicit configuration to ignore or handle nested functions: - gpt4docstrings: This tool includes an explicit command-line option, --ignore-nested-functions (-n), to ignore nested functions and methods during its docstring processing [7][8]. - docsig: This tool (a docstring checker) provides a configuration option to check nested functions and classes, which is disabled by default [9]. When enabled, it verifies the existence and quality of docstrings for nested objects [9]. In summary, standard tools like pydoc and Sphinx implicitly ignore nested functions because they are not part of the top-level public interface [2][5]. Specialized linters and coverage tools often allow you to toggle this behavior explicitly [9][7]. Citations:
🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
import json
import urllib.request
urls = [
"https://raw.githubusercontent.com/econchill/interrogate/main/interrogate/config.py",
"https://raw.githubusercontent.com/econchill/interrogate/main/interrogate/cli.py",
]
for url in urls:
print(f"--- {url} ---")
try:
text = urllib.request.urlopen(url, timeout=10).read().decode()
except Exception as exc:
print(type(exc).__name__, exc)
continue
for i, line in enumerate(text.splitlines(), 1):
if "nested" in line.lower() or "ignore" in line.lower() or "default" in line.lower():
start = max(1, i - 3)
end = min(len(text.splitlines()), i + 5)
print("\n".join(f"{n}: {text.splitlines()[n-1]}" for n in range(start, end + 1)))
print()
PYRepository: ContextualWisdomLab/.github Length of output: 632 🌐 Web query:
💡 Result: In the context of the interrogate tool, which is used for checking Python docstring coverage, --ignore-nested-functions (or the shorthand -n) is a command-line option that instructs the tool to exclude nested functions and methods from the coverage analysis [1][2]. By default, this option is set to False, meaning interrogate will normally include nested functions and methods in its docstring coverage assessment [1][3]. This option can also be configured in a project's pyproject.toml file by setting ignore-nested-functions = true under the [tool.interrogate] section [1][4]. Citations:
🌐 Web query:
💡 Result: Search results found no direct match in Citations: 중첩 함수
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||
|
|
||||||||||
| max_workers = min(10, len(target_paths)) | ||||||||||
| with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: | ||||||||||
| results = list(executor.map(_fetch_file_content, target_paths)) | ||||||||||
|
|
||||||||||
| for path, content, error in results: | ||||||||||
| if error: | ||||||||||
| sections.append(f"### {path}\nUnavailable from head content API: {error}") | ||||||||||
| continue | ||||||||||
| if not content: | ||||||||||
| sections.append(f"### {path}\nNo UTF-8 text content available from head content API.") | ||||||||||
| continue | ||||||||||
| sections.append(f"### {path}\n{truncate_text(content, MAX_FILE_CONTEXT_CHARS)}") | ||||||||||
|
|
||||||||||
| if len(paths) > MAX_CONTEXT_FILES: | ||||||||||
| sections.append(f"[{len(paths) - MAX_CONTEXT_FILES} changed files omitted from context budget]") | ||||||||||
| return "\n\n".join(sections) | ||||||||||
|
|
||||||||||
Uh oh!
There was an error while loading. Please reload this page.