-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: code-structure cleanup (sub-tasks D + F + G + H + I) #1859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
ac87506
docs(auth): expand AuthService class docstring with async/sync + thre…
Aureliolo c9e69bb
refactor(core): add compare_ci helper, migrate inline .lower()== site…
Aureliolo 08a066a
refactor(escalation): collapse Winner+Hybrid processors into HumanDec…
Aureliolo 6eaaf02
build(web): drop openapi-typescript; migrate enums.ts + reports.ts to…
Aureliolo d5a1ac2
docs: protocols audit (Refs #1850 I)
Aureliolo f723654
chore: bump line-shift baselines after compare_ci import additions (R…
Aureliolo 8b36246
refactor: apply pre-PR review feedback for issue 1850 (Refs #1850)
Aureliolo b83267a
chore: bump line-shift baselines after pre-PR review fixes (Refs #1850)
Aureliolo 7a05a51
fix: address PR #1859 reviewer feedback (round 1)
Aureliolo 65399fc
chore: bump line-shift baseline after registry.py import expansion (R…
Aureliolo 963e08f
fix: address PR #1859 reviewer feedback (round 2)
Aureliolo af7d681
chore: bump line-shift baseline after dispatcher start() expansion (R…
Aureliolo b1cff10
fix: address PR #1859 reviewer feedback (round 3)
Aureliolo 02140f7
chore: bump line-shift baseline after dispatcher start() unsubscribe …
Aureliolo fccb22f
fix: address PR #1859 reviewer feedback (round 4)
Aureliolo 3f73e9d
fix: address PR #1859 reviewer feedback (round 5)
Aureliolo 477dc94
fix: address PR #1859 reviewer feedback (round 6) + proper kill switc…
Aureliolo 3ab673b
fix: cast _FakeConfigResolver to ConfigResolver for mypy strict (Refs…
Aureliolo 979eb1f
fix: round 7 reviewer feedback + tests for round-6 code
Aureliolo 00c503a
fix: round 8 dispatcher recovery edge cases (PR #1859)
Aureliolo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This 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
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| """Audit-time helper: enumerate Protocol classes and collect usage counts. | ||
|
|
||
| Output: a CSV-shaped markdown table (one row per Protocol class): | ||
|
|
||
| | path | line | name | rc | impl | typeuse | testuse | | ||
|
|
||
| Where: | ||
| rc - 1 if ``@runtime_checkable`` decorator on the class, else 0. | ||
| impl - count of explicit ``class X(<Name>...)`` matches in src/. | ||
| typeuse - count of ``: <Name> | -> <Name> | <Name> |`` matches in src/. | ||
| testuse - count of any ``<Name>`` token in tests/. | ||
|
|
||
| Run from the repo root: | ||
|
|
||
| uv run python scripts/protocol_audit.py | ||
|
|
||
| The output is consumed by ``docs/reference/protocols-audit.md``; that | ||
| page's date pin in the title records when the snapshot was last | ||
| regenerated. Re-run when revisiting the audit. | ||
| """ | ||
|
|
||
| import re | ||
| import subprocess | ||
| from dataclasses import dataclass | ||
| from pathlib import Path | ||
|
|
||
| ROOT = Path(__file__).resolve().parent.parent | ||
| SRC = ROOT / "src" / "synthorg" | ||
| TESTS = ROOT / "tests" | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class ProtocolEntry: | ||
| """One Protocol-class definition discovered during the SRC walk.""" | ||
|
|
||
| path: str | ||
| line: int | ||
| name: str | ||
| runtime_checkable: bool | ||
|
|
||
|
|
||
| def _enumerate_protocols() -> list[ProtocolEntry]: | ||
| """Walk SRC and yield every class declared as ``class X(... Protocol ...):``.""" | ||
| pattern = re.compile( | ||
| r"^class (\w+)(?:\[[^\]]+\])?\((?:[\w\s,\.]*\b)Protocol\b", | ||
| ) | ||
| rc_pattern = re.compile(r"^@runtime_checkable\s*$") | ||
| entries: list[ProtocolEntry] = [] | ||
| for py_file in sorted(SRC.rglob("*.py")): | ||
| try: | ||
| text = py_file.read_text(encoding="utf-8") | ||
| except UnicodeDecodeError: | ||
| continue | ||
| lines = text.splitlines() | ||
| rel = py_file.relative_to(ROOT).as_posix() | ||
| for i, line in enumerate(lines): | ||
| m = pattern.match(line) | ||
| if not m: | ||
| continue | ||
| name = m.group(1) | ||
| # Walk backwards through decorators / blank lines to detect | ||
| # @runtime_checkable. | ||
| j = i - 1 | ||
| rc = False | ||
| while j >= 0 and lines[j].strip().startswith("@"): | ||
| if rc_pattern.match(lines[j].strip()): | ||
| rc = True | ||
| break | ||
| j -= 1 | ||
| entries.append( | ||
| ProtocolEntry( | ||
| path=rel, | ||
| line=i + 1, | ||
| name=name, | ||
| runtime_checkable=rc, | ||
| ), | ||
| ) | ||
| return entries | ||
|
|
||
|
|
||
| def _count(pattern: str, root: Path) -> int: | ||
| """Count matches via system ``grep -rEo``. | ||
|
|
||
| The ``-o`` flag emits one line per match (rather than one line | ||
| per matched file-line), so the splitlines-based counting below | ||
| sees actual occurrence counts. Without it the helper would | ||
| silently undercount any source line that contains multiple | ||
| references to the same protocol (common in ``: <Name> | -> | ||
| <Name>`` annotations). | ||
|
|
||
| Raises ``RuntimeError`` on a missing grep binary or a non-zero | ||
| grep failure exit code (>1). Silent-zero fallback would | ||
| misclassify protocols as unused and quietly taint the audit | ||
| table; the script is invoked manually so failing loudly is the | ||
| right default. | ||
| """ | ||
| try: | ||
| result = subprocess.run( | ||
| ["grep", "-rEo", "--include=*.py", pattern, str(root)], | ||
| check=False, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| except FileNotFoundError as exc: | ||
| msg = ( | ||
| "grep binary not found on PATH; install GNU grep or run " | ||
| "the audit on a system that ships it." | ||
| ) | ||
| raise RuntimeError(msg) from exc | ||
| if result.returncode > 1: | ||
| msg = ( | ||
| f"grep exited with code {result.returncode} for pattern " | ||
| f"{pattern!r} under {root}: {result.stderr.strip()}" | ||
| ) | ||
| raise RuntimeError(msg) | ||
| return sum(1 for line in result.stdout.splitlines() if line.strip()) | ||
|
|
||
|
|
||
| def _impl_count(name: str) -> int: | ||
| """Concrete classes inheriting from this Protocol (direct subclass).""" | ||
| return _count(rf"^class \w+\(.*\b{name}\b", SRC) | ||
|
|
||
|
|
||
| def _typeuse_count(name: str) -> int: | ||
| """Type-annotation use of the protocol name across SRC. | ||
|
|
||
| Counts ``: <Name>``, ``-> <Name>``, and union-pipe ``| <Name>`` | ||
| occurrences. The defining ``class <Name>(`` line is already | ||
| excluded by the pattern. | ||
| """ | ||
| return _count(rf"(:|->|\|)\s*\b{name}\b", SRC) | ||
|
|
||
|
|
||
| def _testuse_count(name: str) -> int: | ||
| """Count occurrences of the protocol name anywhere under tests/.""" | ||
| return _count(rf"\b{name}\b", TESTS) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def main() -> None: | ||
| """Emit the Protocol-class audit table to stdout.""" | ||
| entries = _enumerate_protocols() | ||
| print(f"# {len(entries)} Protocol classes") | ||
| print() | ||
| print("| path | line | name | rc | impl | typeuse | testuse |") | ||
| print("|---|---|---|---|---|---|---|") | ||
| for e in entries: | ||
| impl = _impl_count(e.name) | ||
| typeuse = _typeuse_count(e.name) | ||
| testuse = _testuse_count(e.name) | ||
| rc = "1" if e.runtime_checkable else "0" | ||
| print( | ||
| f"| {e.path} | {e.line} | {e.name} | {rc} | " | ||
| f"{impl} | {typeuse} | {testuse} |", | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: Aureliolo/synthorg
Length of output: 103
🏁 Script executed:
Repository: Aureliolo/synthorg
Length of output: 762
🏁 Script executed:
Repository: Aureliolo/synthorg
Length of output: 3187
🏁 Script executed:
grep -n "_count" scripts/protocol_audit.pyRepository: Aureliolo/synthorg
Length of output: 517
🏁 Script executed:
Repository: Aureliolo/synthorg
Length of output: 157
🏁 Script executed:
Repository: Aureliolo/synthorg
Length of output: 1998
🏁 Script executed:
grep -h "raise\|Error" scripts/protocol_audit.pyRepository: Aureliolo/synthorg
Length of output: 127
Fail fast on grep errors instead of returning zero counts; use proper error hierarchy.
Returning
0on grep execution failures silently misclassifies protocols as unused and taints the audit table. Raise on command errors and missing binary so bad audits fail loudly. However, the proposed fix must use an appropriateDomainErrorsubclass (e.g.,ProtocolAuditError) per the mandatory error hierarchy constraint—neverRuntimeError.🔧 Corrected approach
Define a
ProtocolAuditErrorclass inheriting fromDomainErrorin the same file or import an existing audit-related error class. Then update_count()to raise it instead of returning 0.🤖 Prompt for AI Agents