feat: package server and add mkdocs documentation site#41
feat: package server and add mkdocs documentation site#41
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR reorganizes the project into a src/ package, adds a console entry point and CLI, replaces the root script with a compatibility wrapper, implements a full MCP server in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as mcp-github-pr-review-spec-maker
participant Env as dotenv/ENV
participant Generator as ReviewSpecGenerator
participant MCP as MCP stdio runtime
participant GitHub as GitHub API
User->>CLI: run (opts/.env)
CLI->>Env: load env & apply CLI overrides
CLI->>Generator: instantiate with config
Generator->>MCP: register tools (fetch_pr_review_comments, resolve_open_pr_url)
MCP->>User: stdio ready / accept tool calls
User->>MCP: call fetch_pr_review_comments(pr_url, opts)
MCP->>Generator: dispatch call
Generator->>GitHub: fetch via GraphQL or REST (host-aware)
GitHub-->>Generator: comment data / rate-limit/errors
Generator->>Generator: generate_markdown / format output
Generator-->>MCP: return result (markdown/json)
MCP-->>User: tool response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary of ChangesHello @petems, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's maintainability and user experience by packaging the server into a distributable Python package and introducing a comprehensive, structured documentation site. These changes simplify installation, improve clarity for developers and users, and lay the groundwork for future development and publishing. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a substantial and well-executed pull request that successfully packages the server, introduces a standard src layout, and adds a console script entry point. The addition of a comprehensive MkDocs documentation site is a major improvement for usability and maintainability. The backward compatibility wrapper for mcp_server.py is a thoughtful touch, and the test suite has been diligently updated to reflect the new project structure. My only suggestion is to clean up a minor redundancy in the project's dependency configuration. Overall, this is an excellent contribution that greatly matures the project.
| "mkdocs>=1.6.0", | ||
| "mkdocs-material>=9.5.0", | ||
| "mkdocstrings[python]>=0.24.1", | ||
| "hatch>=1.9.0", |
There was a problem hiding this comment.
The dev dependencies are defined redundantly in both [project.optional-dependencies] (lines 23-33) and [dependency-groups] (lines 164-178). This can cause confusion and maintenance issues.
To resolve this, I recommend consolidating all development dependencies into the standard [project.optional-dependencies].dev section and removing the [dependency-groups] section. The [dependency-groups] table is a non-standard, legacy Hatch feature.
Here is a suggested consolidated list for [project.optional-dependencies].dev:
dev = [
"pytest>=8.4.1",
"pytest-asyncio>=1.1.0",
"pytest-timeout>=2.3.1",
"pytest-cov>=6.0.0",
"pytest-httpx>=0.32.0",
"hypothesis>=6.100.0",
"mypy>=1.8.0",
"respx>=0.21.0",
"ruff>=0.12.10",
"pre-commit>=3.5.0",
"mkdocs>=1.6.0",
"mkdocs-material>=9.5.0",
"mkdocstrings[python]>=0.24.1",
"hatch>=1.9.0",
]![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
91-93: Update isort first‑party to new package name.Keep import sorting accurate after the rename.
-[tool.ruff.lint.isort] -known-first-party = ["mcp_server"] +[tool.ruff.lint.isort] +known-first-party = ["mcp_github_pr_review_spec_maker"]
🧹 Nitpick comments (15)
docs/changelog.md (1)
5-8: Adopt Keep a Changelog section headings for Unreleased.Add “Added/Changed/Deprecated/Fixed” subsections and date/version links for upcoming releases to keep structure consistent.
AGENTS.md (1)
124-125: Document key CLI flags and tweak quick‑ref wording.
- After the run command, add a brief “Common flags: --per-page, --max-pages, --max-comments, --max-retries, --env-file” with
-hhint.- In Quick Reference, prefer “Test a specific file”.
Also applies to: 237-239
docs/contributing/index.md (1)
35-37: Consistency: prefer US English across docs.Change “behaviour” → “behavior” for consistency, or document dialect choice and apply repo‑wide.
mcp_server.py (1)
3-3: Stabilize the re‑export surface.Star import is acceptable for a shim, but consider defining
__all__explicitly (mirroring the new module) to avoid accidental surface changes when upstream adds internals.-from mcp_github_pr_review_spec_maker.server import * # noqa: F401,F403 +from mcp_github_pr_review_spec_maker import server as _server # type: ignore +__all__ = getattr(_server, "__all__", [n for n in dir(_server) if not n.startswith("_")]) +globals().update({name: getattr(_server, name) for name in __all__})tests/test_rest_error_handling.py (1)
89-89: Apply consistent line formatting for readability.These patch statements follow the same pattern as line 56. While they may not all exceed 88 characters, applying consistent multi-line formatting improves readability and prevents future violations.
Consider applying similar formatting to all patch statements for consistency:
- with patch("mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", return_value=mock_client): + with patch( + "mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", + return_value=mock_client, + ):Also applies to: 123-123, 151-151, 176-176, 192-192, 214-214, 230-230
docs/guides/remote-uv-endpoint.md (1)
7-9: Consider adding language identifier to silence linter.The markdown linter suggests adding a language identifier to the fenced code block. While not strictly necessary for ASCII diagrams, you can silence the warning by adding
text:-``` +```text client host (Claude, Codex) ──ssh──► uv-managed worker ──► GitHub API</blockquote></details> <details> <summary>docs/reference/environment.md (1)</summary><blockquote> `10-10`: **Clarify "future roadmap features" phrasing.** The phrase "Currently used by future roadmap features" is contradictory. Consider revising to either: - "Reserved for future roadmap features" (if not yet used) - "Used by roadmap features" (if currently used) Apply this diff to clarify: ```diff -| `GITHUB_GRAPHQL_URL` | string | `https://{GH_HOST}/api/graphql` | GraphQL endpoint. Currently used by future roadmap features. | +| `GITHUB_GRAPHQL_URL` | string | `https://{GH_HOST}/api/graphql` | GraphQL endpoint. Reserved for future roadmap features. |tests/test_git_pr_resolver.py (1)
688-691: Future-proof long line.This monkeypatch will also exceed 88 chars as code evolves. Apply same wrapping pattern here.
pyproject.toml (3)
11-17: Consider bumping httpx minimum to 0.28.1.0.28.x includes SSL API updates and removes deprecated args; your usage is compatible. Improves security/perf.
- "httpx>=0.27.0", + "httpx>=0.28.1",Based on learnings.
94-101: Limit pytest discovery to tests/ only.Avoids accidental discovery of example/docs files.
-testpaths = ["tests", "."] +testpaths = ["tests"]
22-33: Dep duplication: extras vs dependency‑groups.You define dev deps in both [project.optional-dependencies].dev and [dependency-groups].dev. This can drift.
- Pick one source of truth (recommend dependency-groups for uv) and reference it from the other, or remove one.
Also applies to: 163-178
prompts/GITHUB_API_ROBUSTNESS.md (2)
28-35: Avoid hard‑coded line numbers in docs.They drift quickly; reference function names/sections instead.
Also applies to: 89-92
25-34: Spelling: “GitHub”.Use capital H consistently.
Also applies to: 171-177
src/mcp_github_pr_review_spec_maker/server.py (2)
140-151: Raise backoff ceiling to 15s (matches spec, improves resilience).Current cap is 5s.
- Returns: - Delay in seconds, capped at 5.0 seconds + Returns: + Delay in seconds, capped at 15.0 seconds @@ - return min(5.0, delay) + return min(15.0, delay)
613-632: Gate noisy DEBUG prints behind an env flag.Helpful locally but noisy in CI/clients.
- print( - "DEBUG: page_count=" - f"{page_count}, MAX_PAGES={max_pages_v}, " - f"comments_len={len(all_comments)}", - file=sys.stderr, - ) + if os.getenv("DEBUG_REST_PAGINATION"): + print( + "DEBUG: page_count=" + f"{page_count}, MAX_PAGES={max_pages_v}, " + f"comments_len={len(all_comments)}", + file=sys.stderr, + ) @@ - print(f"DEBUG: next_url={next_url}", file=sys.stderr) + if os.getenv("DEBUG_REST_PAGINATION"): + print(f"DEBUG: next_url={next_url}", file=sys.stderr)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/assets/demo.gifis excluded by!**/*.gifuv.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
AGENTS.md(2 hunks)CLAUDE.md(2 hunks)README.md(6 hunks)UV_COMMANDS.md(1 hunks)docs/architecture/overview.md(1 hunks)docs/changelog.md(1 hunks)docs/contributing/index.md(1 hunks)docs/getting-started/configuration.md(1 hunks)docs/getting-started/installation.md(1 hunks)docs/getting-started/quickstart.md(1 hunks)docs/guides/editor-integrations.md(1 hunks)docs/guides/local-stdio.md(1 hunks)docs/guides/remote-uv-endpoint.md(1 hunks)docs/index.md(1 hunks)docs/reference/cli.md(1 hunks)docs/reference/environment.md(1 hunks)docs/reference/mcp-manifest.md(1 hunks)docs/reference/publishing.md(1 hunks)docs/reference/tools.md(1 hunks)docs/security/index.md(1 hunks)mcp_server.py(1 hunks)mkdocs.yml(1 hunks)prompts/GITHUB_API_ROBUSTNESS.md(7 hunks)pyproject.toml(2 hunks)requirements.txt(1 hunks)run-server.sh(2 hunks)specs/PHASE2_ENTERPRISE_GITHUB_URLS.md(7 hunks)src/mcp_github_pr_review_spec_maker/__init__.py(1 hunks)src/mcp_github_pr_review_spec_maker/__main__.py(1 hunks)src/mcp_github_pr_review_spec_maker/cli.py(1 hunks)src/mcp_github_pr_review_spec_maker/git_pr_resolver.py(1 hunks)src/mcp_github_pr_review_spec_maker/mcp.json(1 hunks)src/mcp_github_pr_review_spec_maker/server.py(1 hunks)tests/conftest.py(1 hunks)tests/test_api_headers.py(1 hunks)tests/test_config_edge_cases.py(1 hunks)tests/test_config_helpers.py(1 hunks)tests/test_enterprise_url_support.py(3 hunks)tests/test_get_pr_info.py(1 hunks)tests/test_git_pr_resolver.py(27 hunks)tests/test_graphql_error_handling.py(1 hunks)tests/test_graphql_timeout.py(1 hunks)tests/test_integration.py(2 hunks)tests/test_mcp_server_tools.py(12 hunks)tests/test_null_author.py(1 hunks)tests/test_pagination_limits.py(1 hunks)tests/test_rest_error_handling.py(12 hunks)tests/test_rest_timeout.py(1 hunks)tests/test_security.py(1 hunks)tests/test_status_fields.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Ruff and adhere to a maximum line length of 88 characters
Ensure imports are sorted per Ruff’s configuration
Code must be compatible with and target Python 3.10+ features/configuration
Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval)
Pass static type checks with mypy; add/maintain type annotations as needed
Python files must compile without SyntaxError (compile-check must pass)
**/*.py: Code must pass ruff linting with zero violations (ruff check .)
Code must be formatted with ruff format .
Static type checking must pass with mypy
Use type hints for all function parameters and return values
Follow async/await patterns for I/O operations (GitHub API calls, file operations)
Handle errors gracefully with proper exception handling and logging
Log errors to stderr using print(..., file=sys.stderr)
Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces
Implement retry logic for transient failures honoring HTTP_MAX_RETRIES
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (e.g., exclusive create, path validation)
Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage
Files:
tests/test_null_author.pytests/test_mcp_server_tools.pysrc/mcp_github_pr_review_spec_maker/git_pr_resolver.pytests/test_enterprise_url_support.pysrc/mcp_github_pr_review_spec_maker/__main__.pytests/test_security.pytests/test_rest_timeout.pytests/test_graphql_error_handling.pytests/test_config_helpers.pytests/test_integration.pytests/test_get_pr_info.pymcp_server.pysrc/mcp_github_pr_review_spec_maker/__init__.pytests/test_rest_error_handling.pytests/test_config_edge_cases.pytests/test_pagination_limits.pytests/test_graphql_timeout.pytests/test_status_fields.pytests/test_api_headers.pytests/test_git_pr_resolver.pytests/conftest.pysrc/mcp_github_pr_review_spec_maker/server.pysrc/mcp_github_pr_review_spec_maker/cli.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest for all tests and keep tests under the tests/ directory
Avoid live GitHub calls in tests by default; use mocks/fixtures, and keep real-call integration tests token-gated/conditionally skipped
Files:
tests/test_null_author.pytests/test_mcp_server_tools.pytests/test_enterprise_url_support.pytests/test_security.pytests/test_rest_timeout.pytests/test_graphql_error_handling.pytests/test_config_helpers.pytests/test_integration.pytests/test_get_pr_info.pytests/test_rest_error_handling.pytests/test_config_edge_cases.pytests/test_pagination_limits.pytests/test_graphql_timeout.pytests/test_status_fields.pytests/test_api_headers.pytests/test_git_pr_resolver.pytests/conftest.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Place test files under tests/ with filenames matching tests/test_.py
Name test functions with the test_ prefix
Use pytest-asyncio for async test functions
Files:
tests/test_null_author.pytests/test_mcp_server_tools.pytests/test_enterprise_url_support.pytests/test_security.pytests/test_rest_timeout.pytests/test_graphql_error_handling.pytests/test_config_helpers.pytests/test_integration.pytests/test_get_pr_info.pytests/test_rest_error_handling.pytests/test_config_edge_cases.pytests/test_pagination_limits.pytests/test_graphql_timeout.pytests/test_status_fields.pytests/test_api_headers.pytests/test_git_pr_resolver.py
tests/test_integration.py
📄 CodeRabbit inference engine (AGENTS.md)
Keep integration tests in tests/test_integration.py (skips when GITHUB_TOKEN is not set)
Files:
tests/test_integration.py
mcp_server.py
📄 CodeRabbit inference engine (CLAUDE.md)
mcp_server.py: All GitHub API interactions should be async/await based
Implement robust error handling for GitHub calls (timeouts, API errors, rate limiting) with retries as appropriate
Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported)
Enforce pagination safety limits for GitHub API calls (max pages/comments, per-page size, retry caps)
Generate markdown with dynamic fencing to safely handle nested backticks in output
Files:
mcp_server.py
tests/test_pagination_limits.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain pagination safety tests in tests/test_pagination_limits.py
Files:
tests/test_pagination_limits.py
pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Project must target and enforce Python 3.10+ via pyproject.toml
Files:
pyproject.toml
tests/conftest.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared pytest fixtures (HTTP client mock, git context, temp dirs, timeouts) in tests/conftest.py
Share common pytest fixtures in tests/conftest.py
Files:
tests/conftest.py
🧠 Learnings (3)
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported)
Applied to files:
docs/reference/environment.mdprompts/GITHUB_API_ROBUSTNESS.mdspecs/PHASE2_ENTERPRISE_GITHUB_URLS.mdtests/test_git_pr_resolver.py
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : All GitHub API interactions should be async/await based
Applied to files:
tests/test_rest_timeout.pyprompts/GITHUB_API_ROBUSTNESS.mdtests/test_graphql_timeout.pyspecs/PHASE2_ENTERPRISE_GITHUB_URLS.md
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : Implement robust error handling for GitHub calls (timeouts, API errors, rate limiting) with retries as appropriate
Applied to files:
prompts/GITHUB_API_ROBUSTNESS.md
🧬 Code graph analysis (20)
tests/test_null_author.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
fetch_pr_comments_graphql(254-445)
tests/test_enterprise_url_support.py (2)
src/mcp_github_pr_review_spec_maker/git_pr_resolver.py (1)
graphql_url_for_host(300-346)src/mcp_github_pr_review_spec_maker/server.py (3)
fetch_pr_comments(448-651)fetch_pr_comments_graphql(254-445)get_pr_info(219-251)
src/mcp_github_pr_review_spec_maker/__main__.py (1)
src/mcp_github_pr_review_spec_maker/cli.py (1)
main(59-85)
tests/test_security.py (1)
src/mcp_github_pr_review_spec_maker/server.py (2)
escape_html_safe(39-50)generate_markdown(654-731)
tests/test_rest_timeout.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
fetch_pr_comments(448-651)
tests/test_graphql_error_handling.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
fetch_pr_comments_graphql(254-445)
tests/test_get_pr_info.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
get_pr_info(219-251)
mcp_server.py (1)
src/mcp_github_pr_review_spec_maker/cli.py (1)
main(59-85)
src/mcp_github_pr_review_spec_maker/__init__.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
create_server(1173-1176)
tests/test_rest_error_handling.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
fetch_pr_comments(448-651)
tests/test_config_edge_cases.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
_int_conf(62-92)
run-server.sh (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
run(1145-1170)
tests/test_pagination_limits.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
fetch_pr_comments(448-651)
tests/test_graphql_timeout.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
fetch_pr_comments_graphql(254-445)
tests/test_status_fields.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
generate_markdown(654-731)
tests/test_api_headers.py (2)
src/mcp_github_pr_review_spec_maker/git_pr_resolver.py (1)
resolve_pr_url(161-297)src/mcp_github_pr_review_spec_maker/server.py (2)
fetch_pr_comments(448-651)fetch_pr_comments_graphql(254-445)
tests/test_git_pr_resolver.py (2)
tests/conftest.py (1)
FakeClient(418-450)src/mcp_github_pr_review_spec_maker/git_pr_resolver.py (5)
_get_repo(73-79)git_detect_repo_branch(82-127)_graphql_find_pr_number(353-422)graphql_url_for_host(300-346)_html_pr_url(349-350)
tests/conftest.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
ReviewSpecGenerator(737-1170)
src/mcp_github_pr_review_spec_maker/server.py (2)
tests/conftest.py (6)
json(408-409)raise_for_status(411-415)post(138-148)post(440-450)get(128-136)get(430-438)src/mcp_github_pr_review_spec_maker/git_pr_resolver.py (4)
api_base_for_host(130-158)git_detect_repo_branch(82-127)graphql_url_for_host(300-346)resolve_pr_url(161-297)
src/mcp_github_pr_review_spec_maker/cli.py (1)
src/mcp_github_pr_review_spec_maker/server.py (2)
ReviewSpecGenerator(737-1170)run(1145-1170)
🪛 GitHub Actions: Lint
tests/test_mcp_server_tools.py
[error] 119-119: E501 Line too long (103 > 88) in test_mcp_server_tools.py.
tests/test_enterprise_url_support.py
[error] 63-63: E501 Line too long (96 > 88) in test_enterprise_url_support.py.
[error] 72-72: E501 Line too long (96 > 88) in test_enterprise_url_support.py.
tests/test_integration.py
[error] 144-144: E501 Line too long (101 > 88) in test_integration.py.
tests/test_rest_error_handling.py
[error] 56-56: E501 Line too long (101 > 88) in test_rest_error_handling.py.
tests/test_git_pr_resolver.py
[error] 80-80: E501 Line too long (112 > 88) in test_git_pr_resolver.py.
[error] 98-98: E501 Line too long (112 > 88) in test_git_pr_resolver.py.
[error] 165-165: E501 Line too long (112 > 88) in test_git_pr_resolver.py.
[error] 279-279: E501 Line too long (118 > 88) in test_git_pr_resolver.py.
[error] 407-407: E501 Line too long (107 > 88) in test_git_pr_resolver.py.
[error] 431-431: E501 Line too long (107 > 88) in test_git_pr_resolver.py.
🪛 LanguageTool
docs/reference/environment.md
[uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”.
Context: ...t:readscope. | |GH_HOST| string |github.com| Automatically updatesGITHUB_AP...
(GITHUB)
docs/getting-started/quickstart.md
[grammar] ~8-~8: There might be a mistake here.
Context: ...al.sh/uv/) package manager (recommended) - GitHub personal access token with **read...
(QB_NEW_EN)
docs/reference/cli.md
[grammar] ~14-~14: There might be a mistake here.
Context: ..._FETCH_MAX_COMMENTSfor this process. | |--json-logs` | Emit JSON formatted lo...
(QB_NEW_EN)
[grammar] ~15-~15: Use a hyphen to join words.
Context: ...s process. | | --json-logs | Emit JSON formatted log lines. | | --help | Show...
(QB_NEW_EN_HYPHEN)
docs/architecture/overview.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...git_pr_resolver module when necessary. 3. HTTP requests retrieve review comments, ...
(QB_NEW_EN)
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ...CP tools by registering their schema in src/mcp_github_pr_review_spec_maker/server.py and imp...
(GITHUB)
docs/reference/tools.md
[grammar] ~22-~22: There might be a mistake here.
Context: ...of branch, latest, first, error. - owner, repo, branch (string, optional): O...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...on. | Review server logs, file an issue with stack trace. |
(QB_NEW_EN)
docs/getting-started/configuration.md
[uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”.
Context: ...l-request comments. | | GH_HOST | ❌ | github.com | GitHub Enterprise hostname. Auto...
(GITHUB)
docs/reference/mcp-manifest.md
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: ...soninside the package directory (e.g.src/mcp_github_pr_review_spec_maker/mcp.json`). Update...
(GITHUB)
prompts/GITHUB_API_ROBUSTNESS.md
[grammar] ~25-~25: There might be a mistake here.
Context: ...: 1. REST API Headers (both files): - Change: `Accept: application/vnd.github....
(QB_NEW_EN)
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...PI Headers** (both files): - Change: Accept: application/vnd.github.v3+json → `Accept: application/vnd.git...
(GITHUB)
[grammar] ~26-~26: There might be a mistake here.
Context: ...cept: application/vnd.github.v3+json→Accept: application/vnd.github+json - Add:X-GitHub-Api-Version: 2022-11-28` ...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ... application/vnd.github+json - Add:X-GitHub-Api-Version: 2022-11-28 - Location insrc/mcp_github_pr_review_sp...
(QB_NEW_EN)
[uncategorized] ~28-~28: The official name of this software platform is spelled with a capital “H”.
Context: ...i-Version: 2022-11-28 - Location insrc/mcp_github_pr_review_spec_maker/server.py`: Lines ...
(GITHUB)
[grammar] ~28-~28: There might be a mistake here.
Context: ...iew_spec_maker/server.py: Lines 380-386 - Location in git_pr_resolver.py`: Lines ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...2. GraphQL API Headers (both files): - Add: `Accept: application/vnd.github+jso...
(QB_NEW_EN)
[uncategorized] ~32-~32: The official name of this software platform is spelled with a capital “H”.
Context: ...L API Headers** (both files): - Add: Accept: application/vnd.github+json (currently missing) - Add: `X-...
(GITHUB)
[grammar] ~32-~32: There might be a mistake here.
Context: ...ion/vnd.github+json(currently missing) - Add:X-GitHub-Api-Version: 2022-11-28` ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...hub+json(currently missing) - Add:X-GitHub-Api-Version: 2022-11-28 - Location insrc/mcp_github_pr_review_sp...
(QB_NEW_EN)
[uncategorized] ~34-~34: The official name of this software platform is spelled with a capital “H”.
Context: ...i-Version: 2022-11-28 - Location insrc/mcp_github_pr_review_spec_maker/server.py`: Lines ...
(GITHUB)
[grammar] ~34-~34: There might be a mistake here.
Context: ...iew_spec_maker/server.py: Lines 172-176 - Location in git_pr_resolver.py`: GraphQ...
(QB_NEW_EN)
[uncategorized] ~89-~89: The official name of this software platform is spelled with a capital “H”.
Context: ...se GitHub URL construction 2. **Update src/mcp_github_pr_review_spec_maker/server.py URL con...
(GITHUB)
[grammar] ~89-~89: There might be a mistake here.
Context: ...pec_maker/server.pyURL construction**: - Line 241: Change hardcodedhttps://api....
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...gic in git_pr_resolver.py not used by src/mcp_github_pr_review_spec_maker/server.py - Hardcoded URLs prevent enterprise GitHub...
(QB_NEW_EN)
[uncategorized] ~169-~169: The official name of this software platform is spelled with a capital “H”.
Context: ...r GitHub support tickets - Location: src/mcp_github_pr_review_spec_maker/server.py lines 4...
(GITHUB)
[grammar] ~171-~171: There might be a mistake here.
Context: ...ng) 2. Primary Rate Limit Handling: - Detection: X-RateLimit-Remaining: 0 or...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ...eLimit-Remaining: 0or standard 403/429 - Action: UseRetry-AfterorX-RateLimi...
(QB_NEW_EN)
[grammar] ~176-~176: There might be a mistake here.
Context: ...ct) 3. GraphQL Rate Limit Handling: - Location: `src/mcp_github_pr_review_spec...
(QB_NEW_EN)
[uncategorized] ~177-~177: The official name of this software platform is spelled with a capital “H”.
Context: ...L Rate Limit Handling**: - Location: src/mcp_github_pr_review_spec_maker/server.py lines 2...
(GITHUB)
[grammar] ~177-~177: There might be a mistake here.
Context: ...view_spec_maker/server.py` lines 260-279 - Currently missing: Add rate limit detect...
(QB_NEW_EN)
[grammar] ~232-~232: There might be a mistake here.
Context: ...ges**: 1. Increase backoff ceiling: - Current: min(5.0, backoff) - New: `...
(QB_NEW_EN)
[grammar] ~233-~233: There might be a mistake here.
Context: ...crease backoff ceiling**: - Current: min(5.0, backoff) - New: min(15.0, backoff) - Location ...
(QB_NEW_EN)
[grammar] ~234-~234: There might be a mistake here.
Context: ... Current: min(5.0, backoff) - New: min(15.0, backoff) - Location in `src/mcp_github_pr_review_sp...
(QB_NEW_EN)
[uncategorized] ~235-~235: The official name of this software platform is spelled with a capital “H”.
Context: ...: min(15.0, backoff) - Location in src/mcp_github_pr_review_spec_maker/server.py: Line 4...
(GITHUB)
[grammar] ~235-~235: There might be a mistake here.
Context: ...r_review_spec_maker/server.py: Line 488 - Location in git_pr_resolver.py`: Simila...
(QB_NEW_EN)
docs/getting-started/installation.md
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ...aker --help ``` This entry point wraps uv run python -m mcp_github_pr_review_spec_maker.server when insta...
(GITHUB)
[uncategorized] ~44-~44: The official name of this software platform is spelled with a capital “H”.
Context: ...``` The server can then be imported as mcp_github_pr_review_spec_maker for programmatic ...
(GITHUB)
specs/PHASE2_ENTERPRISE_GITHUB_URLS.md
[uncategorized] ~24-~24: The official name of this software platform is spelled with a capital “H”.
Context: ...et ### Current Hardcoded URLs (src/mcp_github_pr_review_spec_maker/server.py) These ...
(GITHUB)
[uncategorized] ~76-~76: The official name of this software platform is spelled with a capital “H”.
Context: ...mport Enterprise URL Helpers in src/mcp_github_pr_review_spec_maker/server.py *File...
(GITHUB)
[grammar] ~424-~424: There might be a mistake here.
Context: ...l_for_hostpublic 4. Update imports insrc/mcp_github_pr_review_spec_maker/server.py5. Modifyget_pr_info()` to extract host 6...
(QB_NEW_EN)
docs/contributing/index.md
[grammar] ~36-~36: There might be a mistake here.
Context: ...d CHANGELOG.md when behaviour changes. 3. Link related issues and request review f...
(QB_NEW_EN)
README.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...//docs.astral.sh/uv/) - Quickstart: uv add mcp-github-pr-review-spec-maker && mcp-github-pr-review-spec-maker - Full documentation lives in [docs/](do...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...ps://www.mkdocs.org/). Key entry points: - [Quickstart](docs/getting-started/quickst...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...org/). Key entry points: - Quickstart - [Installation](docs/getting-started/insta...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...started/quickstart.md) - Installation - [Remote hosting with uv](docs/guides/re...
(QB_NEW_EN)
AGENTS.md
[grammar] ~237-~237: There might be a mistake here.
Context: ...v run mcp-github-pr-review-spec-maker| | Test Specific File |uv run pytest te...
(QB_NEW_EN)
CLAUDE.md
[grammar] ~90-~90: There might be a mistake here.
Context: ...r/server.py: Main server implementation - tests/`: Consolidated pytest suite and fixtures...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
docs/guides/remote-uv-endpoint.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (50)
UV_COMMANDS.md (1)
24-24: Console entry point wiring is correctly configured.The
console_scriptsentry point is properly defined at line 20 ofpyproject.toml, themain()function exists incli.py(line 59), and the package is included in the wheel configuration (line 45). The commanduv run mcp-github-pr-review-spec-makerwill work as expected.requirements.txt (1)
5-5: The dulwich constraint recommendation and smoke test suggestion are not applicable to this codebase.The code only uses two stable dulwich APIs:
Repo.discover()andporcelain.active_branch(). It does not perform clone or fetch operations, so adding smoke tests for clone/status operations is unnecessary. These low-level APIs have been stable across versions 0.22 through 0.24.3. The currentdulwich>=0.22.0constraint is appropriate and allows access to bug fixes and security updates. The overly conservative>=0.22,<0.23suggestion would unnecessarily restrict the dependency without providing real risk mitigation.Likely an incorrect or invalid review comment.
specs/PHASE2_ENTERPRISE_GITHUB_URLS.md (1)
1-436: LGTM! Comprehensive enterprise GitHub support specification.This specification document is well-structured and provides clear, detailed implementation guidance for adding enterprise GitHub URL support. The references to the new server module path (
src/mcp_github_pr_review_spec_maker/server.py) are consistent throughout, and the backward-compatibility considerations are well thought out.README.md (1)
1-330: LGTM! Documentation updates align perfectly with package restructuring.All references to the CLI entry point, server module paths, and documentation structure have been updated consistently throughout the README. The changes preserve all essential information while reflecting the new
mcp-github-pr-review-spec-makerentry point andsrc/package layout.tests/test_graphql_timeout.py (1)
7-7: LGTM! Clean import path update.The import path has been correctly updated to reference the new package structure. Test logic remains unchanged.
tests/test_security.py (1)
3-3: LGTM! Import path correctly updated.The import path for
escape_html_safeandgenerate_markdownhas been properly updated to the new package structure, with no changes to test behavior.src/mcp_github_pr_review_spec_maker/git_pr_resolver.py (1)
13-17: LGTM! Correct use of relative imports.The switch from absolute to relative imports (
.github_api_constants) is the proper approach for a package structure. This follows Python packaging best practices when importing sibling modules within the same package.tests/test_null_author.py (1)
7-7: LGTM! Import path updated correctly.The import path has been properly updated to align with the new package structure.
tests/test_pagination_limits.py (1)
16-16: LGTM! Import correctly updated.The import path for
fetch_pr_commentshas been updated to the new package location with no changes to test functionality.tests/test_rest_timeout.py (1)
7-7: LGTM! Import path correctly updated.The import has been properly updated to reference the new package structure, completing the consistent refactoring across all test files.
tests/test_config_edge_cases.py (1)
5-5: LGTM - Clean package migration.The import path update aligns with the new package structure. The test logic remains unchanged.
tests/conftest.py (1)
267-267: LGTM - Test fixture updated correctly.The import path for
ReviewSpecGeneratoris updated to match the new package structure. The fixture logic remains intact.tests/test_status_fields.py (1)
3-3: LGTM - Import path updated correctly.The
generate_markdownimport now references the new package structure. Test behavior is preserved.tests/test_config_helpers.py (1)
7-22: LGTM - Configuration imports migrated successfully.All configuration constants and helper functions are now imported from the new package path. The test suite continues to verify the same behavior.
tests/test_get_pr_info.py (1)
3-3: LGTM - Import path updated for new package structure.The
get_pr_infoimport now references the correct module path. Tests remain functionally identical.docs/guides/local-stdio.md (1)
1-27: LGTM - Clear documentation for local execution.The documentation provides a comprehensive guide for running the server locally via stdio. The setup checklist, optional flags, and troubleshooting sections are well-organized and helpful.
tests/test_rest_error_handling.py (1)
9-9: LGTM - Import path updated correctly.The
fetch_pr_commentsimport now references the new package structure.src/mcp_github_pr_review_spec_maker/__init__.py (1)
1-5: LGTM - Clean package initialization.The package initializer properly exposes the
create_serverfactory function as the public API. The use ofnoqa: F401is appropriate for re-exports in__init__.py, and the__all__declaration explicitly documents the public interface.tests/test_enterprise_url_support.py (1)
10-11: LGTM: Import paths correctly updated.The imports correctly reference the new package structure, aligning with the module reorganization.
CLAUDE.md (2)
40-40: LGTM: CLI command updated correctly.The command now references the new console script entry point, consistent with the package restructuring.
90-90: LGTM: Server path reference updated.The file structure section correctly reflects the new module location.
docs/index.md (1)
1-14: LGTM: Well-structured documentation landing page.The landing page provides clear navigation and appropriately introduces the documentation sections for both human and agent users.
docs/getting-started/installation.md (1)
1-30: LGTM: Clear installation instructions.The installation guide provides comprehensive coverage of different installation methods (uv, pip, and as a dependency).
Also applies to: 32-44
tests/test_integration.py (1)
21-27: LGTM: Import paths correctly migrated.The imports correctly reference the new package structure, maintaining all necessary imports from the relocated modules.
docs/guides/remote-uv-endpoint.md (1)
1-66: LGTM: Comprehensive remote hosting guide.The guide provides clear architecture overview, provisioning steps, systemd configuration, and security recommendations for remote deployment scenarios.
tests/test_graphql_error_handling.py (1)
8-8: LGTM: Import path correctly updated.The import correctly references the new package structure for
fetch_pr_comments_graphql.docs/reference/tools.md (1)
1-37: LGTM: Comprehensive tool reference documentation.The tool reference clearly documents the MCP tools' parameters, response formats, and error codes. This will be valuable for both human developers and AI agents integrating with the server.
tests/test_mcp_server_tools.py (2)
11-15: LGTM! Import migration is clean.The imports have been correctly updated to reference the new package structure
mcp_github_pr_review_spec_maker.server. All imported symbols remain functionally identical.
232-232: All monkeypatch paths updated consistently to new module path.Verification confirms all
monkeypatch.setattrandpatchcalls in the specified lines (232, 251, 260–263, 369, 384–385, 407–411, 497, 537, 542, 571, 576) use the correct new module path"mcp_github_pr_review_spec_maker.server". No oldmcp_servermodule path references remain in these locations or elsewhere in test files.docs/reference/publishing.md (1)
1-20: LGTM! Clear PyPI publishing guide.The documentation provides a concise, step-by-step guide for publishing to PyPI with proper references to the src/ layout, version metadata updates, and distribution verification. The GitHub Actions automation note is a helpful addition.
src/mcp_github_pr_review_spec_maker/mcp.json (2)
13-13: Verify the homepage URL.The homepage URL points to
https://github.com/petersouter/mcp-github-pr-review-spec-maker. Ensure this is the intended canonical repository URL for this package, especially if this is a fork or if the official repository is under a different organization.
1-16: LGTM! MCP manifest is well-structured.The manifest correctly defines the package metadata, command entry point, required environment variables, and categorization. The
GITHUB_TOKENis appropriately marked as required with a clear description.docs/getting-started/quickstart.md (2)
15-15: Verify the repository clone URL.The clone URL points to
https://github.com/petersouter/mcp-github-pr-review-spec-maker.git. Ensure this matches the canonical repository location referenced in the MCP manifest and is the intended URL for users to clone from.
1-44: LGTM! Comprehensive quickstart guide.The quickstart provides clear prerequisites, installation steps using
uv, and verification commands. The structure flows well from installation through verification to next steps.docs/security/index.md (1)
1-24: LGTM! Well-structured security guidance.The security documentation appropriately highlights key threats (malicious PR comments, token leakage, agent drift), provides concrete controls, and references the main SECURITY.md for detailed mitigations.
run-server.sh (1)
675-675: LGTM! Server path migration is complete.All references to the server script have been consistently updated from
mcp_server.pytosrc/mcp_github_pr_review_spec_maker/server.pyacross all client integration functions (Claude, Codex, Gemini) and runtime execution paths. The behavioral flow remains unchanged.Also applies to: 710-710, 712-712, 715-715, 717-717, 720-720, 722-722, 725-725, 727-727, 735-735, 737-737, 742-742, 744-744, 749-749
src/mcp_github_pr_review_spec_maker/__main__.py (1)
1-4: LGTM! Clean CLI bootstrap module.The
__main__.pymodule correctly imports and invokes the CLI entry point fromcli.py, following Python best practices for executable packages. The use ofraise SystemExit(main())properly propagates the exit code.mkdocs.yml (2)
3-5: Verify the site and repository URLs.The site URL and repository URL both point to
petersouter/mcp-github-pr-review-spec-maker. Ensure these URLs are correct and represent the canonical location for this project's documentation and repository, matching the URLs inmcp.jsonand the quickstart guide.
1-66: LGTM! Comprehensive MkDocs configuration.The MkDocs configuration is well-structured with Material theme features, proper navigation hierarchy, essential Markdown extensions, and correctly configured plugins. The mkdocstrings Python handler properly points to the
srcdirectory, aligning with the new package structure.tests/test_api_headers.py (1)
6-12: LGTM! Import paths correctly updated.The imports have been cleanly migrated to the new package structure under
mcp_github_pr_review_spec_maker. Test logic remains unchanged, ensuring consistency.docs/guides/editor-integrations.md (1)
1-56: LGTM! Comprehensive integration guide.The documentation provides clear, actionable instructions for multiple popular editors and CLIs. Examples are well-formatted and use the correct entry point.
docs/reference/cli.md (1)
1-18: LGTM! Clear CLI reference.The documentation accurately describes the CLI options and their behavior. The note about environment variable precedence is helpful.
src/mcp_github_pr_review_spec_maker/cli.py (4)
1-24: LGTM! Clean imports and validation helper.The imports are well-organized, and the
_positive_inthelper provides proper validation for CLI arguments with appropriate error handling.
26-56: LGTM! Well-structured argument parser.The argument parsing is properly implemented with:
- Clear help text for each option
- Appropriate type validation using
_positive_int- Testable design accepting optional
argvparameter
59-85: LGTM! Robust CLI entry point.The
mainfunction properly handles:
- Environment variable loading with correct override semantics
- CLI argument precedence over environment variables
- Graceful shutdown on KeyboardInterrupt (exit code 130)
- Error logging to stderr per coding guidelines
88-89: LGTM! Standard main guard.Uses the idiomatic pattern with proper exit code handling.
docs/architecture/overview.md (1)
9-9: Image reference path is correct; verify whether content illustrates architecture.The file
docs/assets/demo.gifexists and the relative path reference is technically valid. However, whether this demo adequately illustrates the architecture is a design decision that requires your judgment:
- Confirm the demo.gif content actually conveys architecture concepts
- If it does, consider renaming the file or the caption for clarity
- If it doesn't, either replace it with an architecture-specific diagram or remove the reference
docs/reference/mcp-manifest.md (1)
20-20: No action required; repository URL is correct.The homepage URL in the reviewed file correctly references
petersouter/mcp-github-pr-review-spec-maker. This matches authoritative project metadata across the codebase:
pyproject.toml(lines 36–37): Homepage and Repository fields both specify this URLsrc/mcp_github_pr_review_spec_maker/mcp.json(line 13): Homepage field confirms the same URLmkdocs.yml(lines 4–5): Repo URL and name reference this organization and repositoryA search of the entire codebase found zero occurrences of "cool-kids-inc" or "github-pr-review-mcp-server", indicating these identifiers do not appear in project metadata or documentation.
Likely an incorrect or invalid review comment.
tests/test_git_pr_resolver.py (1)
10-15: Import path migration looks good.Module references updated to the packaged location; no functional changes.
src/mcp_github_pr_review_spec_maker/server.py (1)
738-751: MCP handler registration: good pattern.Decorators ensure methods are discoverable; avoids “Method not found”.
|
|
||
| Set `LOG_LEVEL` (`DEBUG`, `INFO`, `WARNING`, `ERROR`) to control verbosity. Logs are written to stderr by default and can be directed to a file using the `--log-file` CLI flag. | ||
|
|
||
| ### MPC Manifest overrides |
There was a problem hiding this comment.
Fix typo: "MPC" should be "MCP".
The heading contains a typo.
Apply this diff:
-### MPC Manifest overrides
+### MCP Manifest overrides📝 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.
| ### MPC Manifest overrides | |
| ### MCP Manifest overrides |
🤖 Prompt for AI Agents
In docs/getting-started/configuration.md around line 27, the heading reads "MPC
Manifest overrides" but should be "MCP Manifest overrides"; edit that line to
correct the typo by replacing "MPC" with "MCP" (update the heading text and any
related anchor or cross-reference if present).
| mcp-github-pr-review-spec-maker --help | ||
| ``` | ||
|
|
||
| This entry point wraps `uv run python -m mcp_github_pr_review_spec_maker.server` when installed from PyPI. |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the entry point description accuracy.
The documentation states the entry point "wraps uv run python -m mcp_github_pr_review_spec_maker.server" but based on the PR context, the console script is defined in pyproject.toml pointing to a CLI module. Please verify this description accurately reflects the actual implementation.
Run this script to check the actual entry point definition:
🏁 Script executed:
#!/bin/bash
# Check the console script entry point definition in pyproject.toml
rg -A 5 '\[project.scripts\]' pyproject.tomlLength of output: 156
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine the CLI module to see what main() actually does
fd -name "cli.py" -o -name "cli" -type d | head -5
Length of output: 230
---
🏁 Script executed:
```shell
#!/bin/bash
# Find CLI module file
fd "cli\.py" | head -5
Length of output: 42
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the CLI module main function
cat src/mcp_github_pr_review_spec_maker/cli.py
Length of output: 2477
Fix documentation: entry point does not wrap uv run command.
The documentation incorrectly claims the entry point "wraps uv run python -m mcp_github_pr_review_spec_maker.server". In reality, when installed from PyPI, the console script directly instantiates ReviewSpecGenerator() and runs the server via asyncio.run(server.run()). There is no uv run wrapper involved—that is a development tool. Update the documentation to accurately reflect that the entry point is a console script that directly starts the server with optional CLI argument overrides for environment configuration.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ...aker --help ``` This entry point wraps uv run python -m mcp_github_pr_review_spec_maker.server when insta...
(GITHUB)
🤖 Prompt for AI Agents
In docs/getting-started/installation.md around line 31, the docs incorrectly
state the entry point "wraps `uv run python -m
mcp_github_pr_review_spec_maker.server`"; update the sentence to state that the
PyPI-installed console script directly instantiates ReviewSpecGenerator() and
starts the server with asyncio.run(server.run()), and mention that optional CLI
arguments can override environment configuration; remove any reference to `uv
run` and add a brief note that `uv run` is a development tool only.
| if __name__ == "__main__": # pragma: no cover | ||
| from mcp_github_pr_review_spec_maker.cli import main | ||
|
|
||
| # Load environment variables | ||
| load_dotenv() | ||
|
|
||
|
|
||
| def escape_html_safe(text: Any) -> str: | ||
| """Escape HTML entities to prevent XSS while preserving readability. | ||
|
|
||
| Args: | ||
| text: Input text of any type that will be converted to string | ||
|
|
||
| Returns: | ||
| HTML-escaped string safe for inclusion in markdown/HTML | ||
| """ | ||
| if text is None: | ||
| return "N/A" | ||
| return html.escape(str(text), quote=True) | ||
|
|
||
|
|
||
| # Parameter ranges (keep in sync with env clamping) | ||
| PER_PAGE_MIN, PER_PAGE_MAX = 1, 100 | ||
| MAX_PAGES_MIN, MAX_PAGES_MAX = 1, 200 | ||
| MAX_COMMENTS_MIN, MAX_COMMENTS_MAX = 100, 100000 | ||
| MAX_RETRIES_MIN, MAX_RETRIES_MAX = 0, 10 | ||
| TIMEOUT_MIN, TIMEOUT_MAX = 1.0, 300.0 | ||
| CONNECT_TIMEOUT_MIN, CONNECT_TIMEOUT_MAX = 1.0, 60.0 | ||
|
|
||
|
|
||
| def _int_conf( | ||
| name: str, default: int, min_v: int, max_v: int, override: int | None | ||
| ) -> int: | ||
| """Load integer configuration from environment with bounds and optional override. | ||
|
|
||
| Args: | ||
| name: Environment variable name | ||
| default: Default value if env var not set or invalid | ||
| min_v: Minimum allowed value | ||
| max_v: Maximum allowed value | ||
| override: Optional override value (takes precedence over env var) | ||
|
|
||
| Returns: | ||
| Clamped integer value within [min_v, max_v] | ||
| """ | ||
| if override is not None: | ||
| try: | ||
| override_int = int(override) | ||
| except (TypeError, ValueError): | ||
| return default | ||
| return max(min_v, min(max_v, override_int)) | ||
|
|
||
| env_value = os.getenv(name) | ||
| if env_value is None: | ||
| env_value = str(default) | ||
|
|
||
| try: | ||
| env_int = int(env_value) | ||
| except (TypeError, ValueError): | ||
| return default | ||
| return max(min_v, min(max_v, env_int)) | ||
|
|
||
|
|
||
| def _float_conf(name: str, default: float, min_v: float, max_v: float) -> float: | ||
| """Load float configuration from environment with bounds. | ||
|
|
||
| Args: | ||
| name: Environment variable name | ||
| default: Default value if env var not set or invalid | ||
| min_v: Minimum allowed value | ||
| max_v: Maximum allowed value | ||
|
|
||
| Returns: | ||
| Clamped float value within [min_v, max_v] | ||
| """ | ||
| env_value = os.getenv(name) | ||
| if env_value is None: | ||
| env_value = str(default) | ||
|
|
||
| try: | ||
| env_float = float(env_value) | ||
| except (TypeError, ValueError): | ||
| return default | ||
| return max(min_v, min(max_v, env_float)) | ||
|
|
||
|
|
||
| class UserData(TypedDict, total=False): | ||
| login: str | ||
|
|
||
|
|
||
| class ReviewComment(TypedDict, total=False): | ||
| user: UserData | ||
| path: str | ||
| line: int | ||
| body: str | ||
| diff_hunk: str | ||
| is_resolved: bool | ||
| is_outdated: bool | ||
| resolved_by: str | None | ||
|
|
||
|
|
||
| class ErrorMessage(TypedDict): | ||
| error: str | ||
|
|
||
|
|
||
| CommentResult = ReviewComment | ErrorMessage | ||
|
|
||
|
|
||
| def _calculate_backoff_delay(attempt: int) -> float: | ||
| """Calculate exponential backoff delay with jitter. | ||
|
|
||
| Args: | ||
| attempt: Current retry attempt number (0-indexed) | ||
|
|
||
| Returns: | ||
| Delay in seconds, capped at 5.0 seconds | ||
| """ | ||
| jitter: float = random.uniform(0, 0.25) # noqa: S311 | ||
| delay: float = (0.5 * (2**attempt)) + jitter | ||
| return min(5.0, delay) | ||
|
|
||
|
|
||
| async def _retry_http_request( | ||
| request_fn: Callable[[], Awaitable[httpx.Response]], | ||
| max_retries: int, | ||
| *, | ||
| status_handler: Callable[[httpx.Response, int], Awaitable[str | None]] | ||
| | None = None, | ||
| ) -> httpx.Response: | ||
| """Execute HTTP request with retry logic for transient errors. | ||
|
|
||
| Handles httpx.RequestError and 5xx server errors with exponential backoff. | ||
| Allows custom status code handling via optional callback. | ||
|
|
||
| Args: | ||
| request_fn: Async callable that performs the HTTP request | ||
| max_retries: Maximum number of retry attempts | ||
| status_handler: Optional async callback for custom status code handling. | ||
| Should return "retry" to retry immediately without incrementing | ||
| attempt counter, or None to continue with default handling. | ||
|
|
||
| Returns: | ||
| httpx.Response on success | ||
|
|
||
| Raises: | ||
| httpx.RequestError: If request errors exceed max_retries | ||
| httpx.HTTPStatusError: If non-retryable HTTP errors occur | ||
| """ | ||
| attempt = 0 | ||
| while True: | ||
| try: | ||
| response = await request_fn() | ||
| except httpx.RequestError as e: | ||
| if attempt < max_retries: | ||
| delay = _calculate_backoff_delay(attempt) | ||
| print( | ||
| f"Request error: {e}. Retrying in {delay:.2f}s...", | ||
| file=sys.stderr, | ||
| ) | ||
| await asyncio.sleep(delay) | ||
| attempt += 1 | ||
| continue | ||
| raise | ||
|
|
||
| # Allow custom status handling (rate limiting, auth fallback, etc.) | ||
| if status_handler: | ||
| action = await status_handler(response, attempt) | ||
| if action == "retry": | ||
| continue # Retry without incrementing attempt counter | ||
|
|
||
| # Handle 5xx server errors with retry | ||
| if 500 <= response.status_code < 600 and attempt < max_retries: | ||
| delay = _calculate_backoff_delay(attempt) | ||
| print( | ||
| f"Server error {response.status_code}. Retrying in {delay:.2f}s...", | ||
| file=sys.stderr, | ||
| ) | ||
| await asyncio.sleep(delay) | ||
| attempt += 1 | ||
| continue | ||
|
|
||
| # For other errors or success, let caller handle | ||
| response.raise_for_status() | ||
| return response | ||
|
|
||
|
|
||
| # Helper functions can remain at the module level as they are pure functions. | ||
| def get_pr_info(pr_url: str) -> tuple[str, str, str, str]: | ||
| """ | ||
| Parses a GitHub pull request URL and returns its host, owner, | ||
| repository, and pull number. | ||
|
|
||
| Accepts URLs of the form https://<host>/<owner>/<repo>/pull/<number> | ||
| with optional trailing path segments, query strings, or fragments | ||
| (for example, ?diff=split or /files). | ||
|
|
||
| Parameters: | ||
| pr_url: The full pull request URL to parse. | ||
|
|
||
| Returns: | ||
| A tuple (host, owner, repo, pull_number) where each element | ||
| is a string. | ||
|
|
||
| Raises: | ||
| ValueError: If the URL does not match the expected pull | ||
| request format. | ||
| """ | ||
|
|
||
| # Allow optional trailing ``/...``, query string, or fragment after the PR | ||
| # number. Everything up to ``pull/<num>`` must match exactly. | ||
| pattern = r"^https://([^/]+)/([^/]+)/([^/]+)/pull/(\d+)(?:[/?#].*)?$" | ||
| match = re.match(pattern, pr_url) | ||
| if not match: | ||
| raise ValueError( | ||
| "Invalid PR URL format. Expected format: https://{host}/owner/repo/pull/123" | ||
| ) | ||
| groups = match.groups() | ||
| assert len(groups) == 4 | ||
| host, owner, repo, num = groups[0], groups[1], groups[2], groups[3] | ||
| return host, owner, repo, num | ||
|
|
||
|
|
||
| async def fetch_pr_comments_graphql( | ||
| owner: str, | ||
| repo: str, | ||
| pull_number: int, | ||
| *, | ||
| host: str = "github.com", | ||
| max_comments: int | None = None, | ||
| max_retries: int | None = None, | ||
| ) -> list[CommentResult] | None: | ||
| """ | ||
| Fetch review comments for a pull request via the GitHub GraphQL API, | ||
| including resolution and outdated status. | ||
|
|
||
| Requires the environment variable GITHUB_TOKEN to be set. The returned | ||
| items are dictionaries matching the ReviewComment TypedDict with | ||
| additional fields: `is_resolved`, `is_outdated`, and `resolved_by`. | ||
|
|
||
| Parameters: | ||
| host (str): GitHub host to target (e.g., "github.com"). | ||
| Defaults to "github.com". | ||
| max_comments (int | None): Maximum number of comments to fetch; | ||
| if None, the configured/default limit is used. | ||
| max_retries (int | None): Maximum retry attempts for transient | ||
| HTTP errors; if None, the configured/default is used. | ||
|
|
||
| Returns: | ||
| list[CommentResult] | None: A list of review comment objects on | ||
| success, or `None` if the operation failed or timed out. | ||
|
|
||
| Raises: | ||
| httpx.RequestError: If a network/request error occurs after | ||
| exhausting retries. | ||
| """ | ||
| print( | ||
| f"Fetching comments via GraphQL for {owner}/{repo}#{pull_number}", | ||
| file=sys.stderr, | ||
| ) | ||
| token = os.getenv("GITHUB_TOKEN") | ||
| if not token: | ||
| print("ERROR: GITHUB_TOKEN required for GraphQL API", file=sys.stderr) | ||
| return None | ||
|
|
||
| headers: dict[str, str] = { | ||
| "Authorization": f"Bearer {token}", | ||
| "Accept": GITHUB_ACCEPT_HEADER, | ||
| "X-GitHub-Api-Version": GITHUB_API_VERSION, | ||
| "Content-Type": "application/json", | ||
| "User-Agent": GITHUB_USER_AGENT, | ||
| } | ||
|
|
||
| # Load configurable limits | ||
| max_comments_v = _int_conf("PR_FETCH_MAX_COMMENTS", 2000, 100, 100000, max_comments) | ||
| max_retries_v = _int_conf("HTTP_MAX_RETRIES", 3, 0, 10, max_retries) | ||
|
|
||
| # GraphQL query to fetch review threads with resolution and outdated status | ||
| query = """ | ||
| query($owner: String!, $repo: String!, $prNumber: Int!, $cursor: String) { | ||
| repository(owner: $owner, name: $repo) { | ||
| pullRequest(number: $prNumber) { | ||
| reviewThreads(first: 100, after: $cursor) { | ||
| pageInfo { | ||
| hasNextPage | ||
| endCursor | ||
| } | ||
| nodes { | ||
| isResolved | ||
| isOutdated | ||
| resolvedBy { | ||
| login | ||
| } | ||
| comments(first: 100) { | ||
| nodes { | ||
| author { | ||
| login | ||
| } | ||
| body | ||
| path | ||
| line | ||
| diffHunk | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| """ | ||
|
|
||
| all_comments: list[CommentResult] = [] | ||
| cursor = None | ||
| has_next_page = True | ||
|
|
||
| # Load timeout configuration | ||
| total_timeout = _float_conf("HTTP_TIMEOUT", 30.0, TIMEOUT_MIN, TIMEOUT_MAX) | ||
| connect_timeout = _float_conf( | ||
| "HTTP_CONNECT_TIMEOUT", 10.0, CONNECT_TIMEOUT_MIN, CONNECT_TIMEOUT_MAX | ||
| ) | ||
|
|
||
| try: | ||
| timeout = httpx.Timeout(timeout=total_timeout, connect=connect_timeout) | ||
| async with httpx.AsyncClient(timeout=timeout, follow_redirects=True) as client: | ||
| while has_next_page and len(all_comments) < max_comments_v: | ||
| variables = { | ||
| "owner": owner, | ||
| "repo": repo, | ||
| "prNumber": pull_number, | ||
| "cursor": cursor, | ||
| } | ||
|
|
||
| graphql_url = graphql_url_for_host(host) | ||
|
|
||
| # Use retry helper for GraphQL request (capture loop variables) | ||
| async def make_graphql_request( | ||
| url: str = graphql_url, gql_vars: dict[str, Any] = variables | ||
| ) -> httpx.Response: | ||
| return await client.post( | ||
| url, | ||
| headers=headers, | ||
| json={"query": query, "variables": gql_vars}, | ||
| ) | ||
|
|
||
| response = await _retry_http_request( | ||
| make_graphql_request, max_retries_v | ||
| ) | ||
|
|
||
| data = response.json() | ||
| if "errors" in data: | ||
| print(f"GraphQL errors: {data['errors']}", file=sys.stderr) | ||
| return None | ||
|
|
||
| pr_data = data.get("data", {}).get("repository", {}).get("pullRequest") | ||
| if not pr_data: | ||
| print("No pull request data returned", file=sys.stderr) | ||
| return None | ||
|
|
||
| review_threads = pr_data.get("reviewThreads", {}) | ||
| threads = review_threads.get("nodes", []) | ||
|
|
||
| # Process each thread and its comments | ||
| for thread in threads: | ||
| is_resolved = thread.get("isResolved", False) | ||
| is_outdated = thread.get("isOutdated", False) | ||
| resolved_by_data = thread.get("resolvedBy") | ||
| resolved_by = ( | ||
| resolved_by_data.get("login") if resolved_by_data else None | ||
| ) | ||
|
|
||
| comments = thread.get("comments", {}).get("nodes", []) | ||
| for comment in comments: | ||
| # Convert GraphQL format to REST-like format with added fields | ||
| # Guard against null author (e.g., deleted user accounts) | ||
| author = comment.get("author") or {} | ||
| review_comment: ReviewComment = { | ||
| "user": {"login": author.get("login") or "unknown"}, | ||
| "path": comment.get("path", ""), | ||
| "line": comment.get("line") or 0, | ||
| "body": comment.get("body", ""), | ||
| "diff_hunk": comment.get("diffHunk", ""), | ||
| "is_resolved": is_resolved, | ||
| "is_outdated": is_outdated, | ||
| "resolved_by": resolved_by, | ||
| } | ||
| all_comments.append(review_comment) | ||
|
|
||
| if len(all_comments) >= max_comments_v: | ||
| break | ||
|
|
||
| # Check pagination | ||
| page_info = review_threads.get("pageInfo", {}) | ||
| has_next_page = page_info.get("hasNextPage", False) | ||
| cursor = page_info.get("endCursor") | ||
|
|
||
| print( | ||
| f"Fetched {len(threads)} threads, " | ||
| f"total comments: {len(all_comments)}", | ||
| file=sys.stderr, | ||
| ) | ||
|
|
||
| print( | ||
| f"Successfully fetched {len(all_comments)} comments via GraphQL", | ||
| file=sys.stderr, | ||
| ) | ||
| return all_comments | ||
|
|
||
| except httpx.TimeoutException as e: | ||
| print(f"Timeout error fetching PR comments: {str(e)}", file=sys.stderr) | ||
| traceback.print_exc(file=sys.stderr) | ||
| return None | ||
| except httpx.RequestError as e: | ||
| print(f"Error fetching PR comments: {str(e)}", file=sys.stderr) | ||
| traceback.print_exc(file=sys.stderr) | ||
| raise | ||
|
|
||
|
|
||
| async def fetch_pr_comments( | ||
| owner: str, | ||
| repo: str, | ||
| pull_number: int, | ||
| *, | ||
| host: str = "github.com", | ||
| per_page: int | None = None, | ||
| max_pages: int | None = None, | ||
| max_comments: int | None = None, | ||
| max_retries: int | None = None, | ||
| ) -> list[CommentResult] | None: | ||
| """ | ||
| Fetch and combine review comments for a pull request by iterating | ||
| the repository REST API pagination. | ||
|
|
||
| Parameters: | ||
| per_page (int | None): Override for number of comments to | ||
| request per page. | ||
| max_pages (int | None): Override for maximum number of pages | ||
| to fetch. | ||
| max_comments (int | None): Override for a hard limit on total | ||
| comments to collect. | ||
| max_retries (int | None): Override for maximum retry attempts | ||
| on transient errors. | ||
|
|
||
| Returns: | ||
| list[CommentResult] with comments combined from all fetched | ||
| pages, or `None` when fetching fails due to timeouts or | ||
| unrecoverable server errors. | ||
| """ | ||
| print(f"Fetching comments for {owner}/{repo}#{pull_number}", file=sys.stderr) | ||
| token = os.getenv("GITHUB_TOKEN") | ||
| headers: dict[str, str] = { | ||
| "Accept": GITHUB_ACCEPT_HEADER, | ||
| "X-GitHub-Api-Version": GITHUB_API_VERSION, | ||
| "User-Agent": GITHUB_USER_AGENT, | ||
| } | ||
| if token: | ||
| # Use Bearer prefix for fine-grained tokens | ||
| headers["Authorization"] = f"Bearer {token}" | ||
|
|
||
| # URL-encode owner/repo to be safe, even though regex validation restricts format | ||
| safe_owner = quote(owner, safe="") | ||
| safe_repo = quote(repo, safe="") | ||
|
|
||
| # Load configurable limits from environment with safe defaults; allow per-call | ||
| # overrides | ||
| per_page_v = _int_conf("HTTP_PER_PAGE", 100, 1, 100, per_page) | ||
| max_pages_v = _int_conf("PR_FETCH_MAX_PAGES", 50, 1, 200, max_pages) | ||
| max_comments_v = _int_conf("PR_FETCH_MAX_COMMENTS", 2000, 100, 100000, max_comments) | ||
| max_retries_v = _int_conf("HTTP_MAX_RETRIES", 3, 0, 10, max_retries) | ||
|
|
||
| api_base = api_base_for_host(host) | ||
| base_url = ( | ||
| f"{api_base}/repos/" | ||
| f"{safe_owner}/{safe_repo}/pulls/{pull_number}/comments?per_page={per_page_v}" | ||
| ) | ||
| all_comments: list[CommentResult] = [] | ||
| url: str | None = base_url | ||
| page_count = 0 | ||
|
|
||
| # Load timeout configuration | ||
| total_timeout = _float_conf("HTTP_TIMEOUT", 30.0, TIMEOUT_MIN, TIMEOUT_MAX) | ||
| connect_timeout = _float_conf( | ||
| "HTTP_CONNECT_TIMEOUT", 10.0, CONNECT_TIMEOUT_MIN, CONNECT_TIMEOUT_MAX | ||
| ) | ||
|
|
||
| try: | ||
| timeout = httpx.Timeout(timeout=total_timeout, connect=connect_timeout) | ||
| async with httpx.AsyncClient(timeout=timeout, follow_redirects=True) as client: | ||
| used_token_fallback = False | ||
| had_server_error = False | ||
| while url: | ||
| print(f"Fetching page {page_count + 1}...", file=sys.stderr) | ||
|
|
||
| # Status handler for REST-specific logic (rate limiting, auth fallback) | ||
| async def handle_rest_status( | ||
| resp: httpx.Response, attempt: int | ||
| ) -> str | None: | ||
| nonlocal used_token_fallback, had_server_error | ||
|
|
||
| # Track 5xx errors for conservative failure behavior | ||
| if 500 <= resp.status_code < 600: | ||
| had_server_error = True | ||
|
|
||
| # 401 Bearer token fallback | ||
| if ( | ||
| resp.status_code == 401 | ||
| and token | ||
| and not used_token_fallback | ||
| and headers.get("Authorization", "").startswith("Bearer ") | ||
| ): | ||
| print( | ||
| "401 Unauthorized with Bearer; retrying with 'token' " | ||
| "scheme...", | ||
| file=sys.stderr, | ||
| ) | ||
| headers["Authorization"] = f"token {token}" | ||
| used_token_fallback = True | ||
| return "retry" | ||
|
|
||
| # Rate limiting | ||
| if resp.status_code in (429, 403): | ||
| retry_after_header = resp.headers.get("Retry-After") | ||
| remaining = resp.headers.get("X-RateLimit-Remaining") | ||
| reset = resp.headers.get("X-RateLimit-Reset") | ||
|
|
||
| if retry_after_header or remaining == "0": | ||
| retry_after = 60 | ||
| try: | ||
| if retry_after_header: | ||
| retry_after = int(retry_after_header) | ||
| elif reset: | ||
| import time | ||
|
|
||
| now = int(time.time()) | ||
| retry_after = max(int(reset) - now, 1) | ||
| except (ValueError, TypeError): | ||
| retry_after = 60 | ||
|
|
||
| print( | ||
| f"Rate limited. Backing off for {retry_after}s...", | ||
| file=sys.stderr, | ||
| ) | ||
| await asyncio.sleep(retry_after) | ||
| return "retry" | ||
|
|
||
| return None | ||
|
|
||
| # Use retry helper with custom status handler (capture loop variable) | ||
| current_page_url = url # Captured by while loop type narrowing | ||
|
|
||
| async def make_rest_request( | ||
| page_url: str = current_page_url, | ||
| ) -> httpx.Response: | ||
| return await client.get(page_url, headers=headers) | ||
|
|
||
| try: | ||
| response = await _retry_http_request( | ||
| make_rest_request, | ||
| max_retries_v, | ||
| status_handler=handle_rest_status, | ||
| ) | ||
| except httpx.HTTPStatusError as e: | ||
| # On exhausted 5xx retries, return None per test expectations | ||
| if 500 <= e.response.status_code < 600: | ||
| return None | ||
| raise | ||
|
|
||
| # Conservative behavior: return None if any server error occurred, | ||
| # even if retry succeeded | ||
| if had_server_error: | ||
| return None | ||
|
|
||
| # Process page | ||
| page_comments = response.json() | ||
| if not isinstance(page_comments, list) or not all( | ||
| isinstance(c, dict) for c in page_comments | ||
| ): | ||
| return None | ||
| all_comments.extend(cast(list[CommentResult], page_comments)) | ||
| page_count += 1 | ||
|
|
||
| # Enforce safety bounds to prevent unbounded memory/time use | ||
| print( | ||
| "DEBUG: page_count=" | ||
| f"{page_count}, MAX_PAGES={max_pages_v}, " | ||
| f"comments_len={len(all_comments)}", | ||
| file=sys.stderr, | ||
| ) | ||
| if page_count >= max_pages_v or len(all_comments) >= max_comments_v: | ||
| print( | ||
| "Reached safety limits for pagination; stopping early", | ||
| file=sys.stderr, | ||
| ) | ||
| break | ||
|
|
||
| # Check for next page using Link header | ||
| link_header = response.headers.get("Link") | ||
| next_url: str | None = None | ||
| if link_header: | ||
| match = re.search(r"<([^>]+)>;\s*rel=\"next\"", link_header) | ||
| next_url = match.group(1) if match else None | ||
| print(f"DEBUG: next_url={next_url}", file=sys.stderr) | ||
| if next_url: | ||
| url = next_url | ||
| else: | ||
| break | ||
|
|
||
| total_comments = len(all_comments) | ||
| print( | ||
| f"Successfully fetched {total_comments} comments across {page_count} pages", | ||
| file=sys.stderr, | ||
| ) | ||
| return all_comments | ||
|
|
||
| except httpx.TimeoutException as e: | ||
| print(f"Timeout error fetching PR comments: {str(e)}", file=sys.stderr) | ||
| traceback.print_exc(file=sys.stderr) | ||
| return None | ||
| except httpx.RequestError as e: | ||
| print(f"Error fetching PR comments: {str(e)}", file=sys.stderr) | ||
| traceback.print_exc(file=sys.stderr) | ||
| raise | ||
|
|
||
|
|
||
| def generate_markdown(comments: Sequence[CommentResult]) -> str: | ||
| """Generates a markdown string from a list of review comments.""" | ||
|
|
||
| def fence_for(text: str, minimum: int = 3) -> str: | ||
| # Choose a backtick fence longer than any run of backticks in the text | ||
| longest_run = 0 | ||
| current = 0 | ||
| for ch in text or "": | ||
| if ch == "`": | ||
| current += 1 | ||
| if current > longest_run: | ||
| longest_run = current | ||
| else: | ||
| current = 0 | ||
| return "`" * max(minimum, longest_run + 1) | ||
|
|
||
| markdown = "# Pull Request Review Spec\n\n" | ||
| if not comments: | ||
| return markdown + "No comments found.\n" | ||
|
|
||
| for comment in comments: | ||
| # Skip error messages - they are not review comments | ||
| if "error" in comment: | ||
| continue | ||
|
|
||
| # At this point, we know comment is a ReviewComment | ||
| # Escape username to prevent HTML injection in headers | ||
| # Handle malformed user objects gracefully | ||
| user_data = comment.get("user") | ||
| login = user_data.get("login", "N/A") if isinstance(user_data, dict) else "N/A" | ||
| username = escape_html_safe(login) | ||
| markdown += f"## Review Comment by {username}\n\n" | ||
|
|
||
| # Escape file path - inside backticks but could break out | ||
| file_path = escape_html_safe(comment.get("path", "N/A")) | ||
| markdown += f"**File:** `{file_path}`\n" | ||
|
|
||
| # Line number is typically safe but escape for consistency | ||
| line_num = escape_html_safe(comment.get("line", "N/A")) | ||
| markdown += f"**Line:** {line_num}\n" | ||
|
|
||
| # Add status indicators if available | ||
| status_parts = [] | ||
| is_resolved = comment.get("is_resolved") | ||
| is_outdated = comment.get("is_outdated") | ||
| resolved_by = comment.get("resolved_by") | ||
|
|
||
| if is_resolved is True: | ||
| status_str = "✓ Resolved" | ||
| if resolved_by: | ||
| status_str += f" by {escape_html_safe(resolved_by)}" | ||
| status_parts.append(status_str) | ||
| elif is_resolved is False: | ||
| status_parts.append("○ Unresolved") | ||
|
|
||
| if is_outdated: | ||
| status_parts.append("⚠ Outdated") | ||
|
|
||
| if status_parts: | ||
| markdown += f"**Status:** {' | '.join(status_parts)}\n" | ||
|
|
||
| markdown += "\n" | ||
|
|
||
| # Escape comment body to prevent XSS - this is the main attack vector | ||
| body = escape_html_safe(comment.get("body", "")) | ||
| body_fence = fence_for(body) | ||
| markdown += f"**Comment:**\n{body_fence}\n{body}\n{body_fence}\n\n" | ||
|
|
||
| if "diff_hunk" in comment: | ||
| # Escape diff content to prevent injection through malicious diffs | ||
| diff_text = escape_html_safe(comment["diff_hunk"]) | ||
| diff_fence = fence_for(diff_text) | ||
| # Language hint remains after the opening fence | ||
| markdown += ( | ||
| f"**Code Snippet:**\n{diff_fence}diff\n{diff_text}\n{diff_fence}\n\n" | ||
| ) | ||
| markdown += "---\n\n" | ||
| return markdown | ||
|
|
||
|
|
||
| T = TypeVar("T") | ||
|
|
||
|
|
||
| class ReviewSpecGenerator: | ||
| def __init__(self) -> None: | ||
| self.server = server.Server("github_review_spec_generator") | ||
| print("MCP Server initialized", file=sys.stderr) | ||
| self._register_handlers() | ||
|
|
||
| def _register_handlers(self) -> None: | ||
| """Register MCP handlers.""" | ||
| # Properly register handlers with the MCP server. The low-level Server | ||
| # uses decorator-style registration to populate request_handlers. | ||
| # Direct attribute assignment does not wire up RPC methods and results | ||
| # in "Method not found" errors from clients. | ||
| self.server.list_tools()(self.handle_list_tools) # type: ignore[no-untyped-call] | ||
| self.server.call_tool()(self.handle_call_tool) | ||
|
|
||
| async def handle_list_tools(self) -> list[Tool]: | ||
| """ | ||
| List available tools. | ||
| Each tool is defined as a Tool object containing name, description, | ||
| and parameters. | ||
| """ | ||
| return [ | ||
| Tool( | ||
| name="fetch_pr_review_comments", | ||
| description=( | ||
| "Fetches all review comments from a GitHub PR. Provide a PR URL, " | ||
| "or omit it to auto-detect from the current git repo/branch." | ||
| ), | ||
| inputSchema={ | ||
| "type": "object", | ||
| "properties": { | ||
| "pr_url": { | ||
| "type": "string", | ||
| "description": ( | ||
| "The full URL of the GitHub pull request. If omitted, " | ||
| "the server will try to resolve the PR for the current " | ||
| "git repo and branch." | ||
| ), | ||
| }, | ||
| "output": { | ||
| "type": "string", | ||
| "enum": ["markdown", "json", "both"], | ||
| "description": ( | ||
| "Output format. Default 'markdown'. Use 'json' for " | ||
| "raw data; 'both' returns json then markdown." | ||
| ), | ||
| }, | ||
| "select_strategy": { | ||
| "type": "string", | ||
| "enum": ["branch", "latest", "first", "error"], | ||
| "description": ( | ||
| "Strategy when auto-resolving a PR (default 'branch')." | ||
| ), | ||
| }, | ||
| "owner": { | ||
| "type": "string", | ||
| "description": "Override repo owner for PR resolution", | ||
| }, | ||
| "repo": { | ||
| "type": "string", | ||
| "description": "Override repo name for PR resolution", | ||
| }, | ||
| "branch": { | ||
| "type": "string", | ||
| "description": "Override branch name for PR resolution", | ||
| }, | ||
| "per_page": { | ||
| "type": "integer", | ||
| "description": "GitHub API page size (1-100)", | ||
| "minimum": 1, | ||
| "maximum": 100, | ||
| }, | ||
| "max_pages": { | ||
| "type": "integer", | ||
| "description": ( | ||
| "Max number of pages to fetch (server-capped)" | ||
| ), | ||
| "minimum": 1, | ||
| "maximum": 200, | ||
| }, | ||
| "max_comments": { | ||
| "type": "integer", | ||
| "description": ( | ||
| "Max total comments to collect (server-capped)" | ||
| ), | ||
| "minimum": 100, | ||
| "maximum": 100000, | ||
| }, | ||
| "max_retries": { | ||
| "type": "integer", | ||
| "description": ( | ||
| "Max retries for transient errors (server-capped)" | ||
| ), | ||
| "minimum": 0, | ||
| "maximum": 10, | ||
| }, | ||
| }, | ||
| }, | ||
| ), | ||
| Tool( | ||
| name="resolve_open_pr_url", | ||
| description=( | ||
| "Resolves the open PR URL for the current branch using git " | ||
| "detection. Optionally pass owner/repo/branch/host overrides " | ||
| "and a select strategy." | ||
| ), | ||
| inputSchema={ | ||
| "type": "object", | ||
| "properties": { | ||
| "select_strategy": { | ||
| "type": "string", | ||
| "enum": ["branch", "latest", "first", "error"], | ||
| "description": ( | ||
| "Strategy when auto-resolving a PR (default 'branch')." | ||
| ), | ||
| }, | ||
| "owner": { | ||
| "type": "string", | ||
| "description": "Override repo owner for PR resolution", | ||
| }, | ||
| "repo": { | ||
| "type": "string", | ||
| "description": "Override repo name for PR resolution", | ||
| }, | ||
| "branch": { | ||
| "type": "string", | ||
| "description": "Override branch name for PR resolution", | ||
| }, | ||
| "host": { | ||
| "type": "string", | ||
| "description": ( | ||
| "GitHub host (e.g., 'github.com' or " | ||
| "'github.enterprise.com'). If not provided, " | ||
| "detected from git context or defaults to github.com" | ||
| ), | ||
| }, | ||
| }, | ||
| }, | ||
| ), | ||
| ] | ||
|
|
||
| async def handle_call_tool( | ||
| self, name: str, arguments: dict[str, Any] | ||
| ) -> Sequence[TextContent]: | ||
| """ | ||
| Dispatch a tool invocation by name and return its textual outputs. | ||
|
|
||
| Parameters: | ||
| name (str): The tool identifier to invoke (e.g., | ||
| "fetch_pr_review_comments", "resolve_open_pr_url"). | ||
| arguments (dict[str, Any]): Tool-specific arguments; expected keys | ||
| depend on `name` (for example, "pr_url", "per_page", | ||
| "max_pages", "max_comments", "max_retries", | ||
| "select_strategy", "owner", "repo", "branch", and "output" for | ||
| "fetch_pr_review_comments"). | ||
|
|
||
| Returns: | ||
| Sequence[TextContent]: One or more text outputs produced by the | ||
| invoked tool (e.g., markdown and/or JSON). | ||
|
|
||
| Raises: | ||
| ValueError: If `name` is unknown or if provided arguments fail | ||
| validation. | ||
| RuntimeError: If an underlying HTTP or OS error occurs while | ||
| executing the requested tool. | ||
| """ | ||
|
|
||
| async def _run_with_handling(operation: Callable[[], Awaitable[T]]) -> T: | ||
| try: | ||
| return await operation() | ||
| except ValueError: | ||
| raise | ||
| except (httpx.HTTPError, OSError, RuntimeError, TypeError) as exc: | ||
| error_msg = f"Error executing tool {name}: {exc}" | ||
| print(error_msg, file=sys.stderr) | ||
| traceback.print_exc(file=sys.stderr) | ||
| raise RuntimeError(error_msg) from exc | ||
|
|
||
| if name == "fetch_pr_review_comments": | ||
| # Validate optional numeric parameters | ||
| def _validate_int( | ||
| arg_name: str, value: Any, min_v: int, max_v: int | ||
| ) -> int | None: | ||
| """ | ||
| Validate and coerce a value to an integer within specified | ||
| bounds. | ||
|
|
||
| Parameters: | ||
| arg_name (str): Name of the argument (used in error | ||
| messages). | ||
| value (Any): The value to validate; accepts integers or | ||
| numeric strings. A value of None is allowed and | ||
| returns None. | ||
| min_v (int): Minimum allowed value (inclusive). | ||
| max_v (int): Maximum allowed value (inclusive). | ||
|
|
||
| Returns: | ||
| int | None: The coerced integer when a valid value is | ||
| provided, or None if `value` is None. | ||
|
|
||
| Raises: | ||
| ValueError: If `value` is a boolean, not an integer or | ||
| numeric string, or if the coerced integer is outside the | ||
| [min_v, max_v] range. | ||
| """ | ||
| if value is None: | ||
| return None | ||
|
|
||
| type_error = f"Invalid type for {arg_name}: expected integer" | ||
|
|
||
| # Reject bools explicitly (they're a subclass of int in Python) | ||
| if isinstance(value, bool): | ||
| raise ValueError(type_error) | ||
|
|
||
| # Coerce to int: accept int directly or parse numeric string | ||
| if isinstance(value, int): | ||
| result = value | ||
| elif isinstance(value, str): | ||
| try: | ||
| result = int(value, 10) | ||
| except ValueError: | ||
| raise ValueError(type_error) from None | ||
| else: | ||
| raise ValueError(type_error) | ||
|
|
||
| # Validate range | ||
| if not (min_v <= result <= max_v): | ||
| raise ValueError( | ||
| f"Invalid value for {arg_name}: must be between " | ||
| f"{min_v} and {max_v}" | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| per_page = _validate_int( | ||
| "per_page", arguments.get("per_page"), PER_PAGE_MIN, PER_PAGE_MAX | ||
| ) | ||
| max_pages = _validate_int( | ||
| "max_pages", | ||
| arguments.get("max_pages"), | ||
| MAX_PAGES_MIN, | ||
| MAX_PAGES_MAX, | ||
| ) | ||
| max_comments = _validate_int( | ||
| "max_comments", | ||
| arguments.get("max_comments"), | ||
| MAX_COMMENTS_MIN, | ||
| MAX_COMMENTS_MAX, | ||
| ) | ||
| max_retries = _validate_int( | ||
| "max_retries", | ||
| arguments.get("max_retries"), | ||
| MAX_RETRIES_MIN, | ||
| MAX_RETRIES_MAX, | ||
| ) | ||
|
|
||
| comments = await _run_with_handling( | ||
| lambda: self.fetch_pr_review_comments( | ||
| arguments.get("pr_url", ""), | ||
| per_page=per_page, | ||
| max_pages=max_pages, | ||
| max_comments=max_comments, | ||
| max_retries=max_retries, | ||
| select_strategy=arguments.get("select_strategy"), | ||
| owner=arguments.get("owner"), | ||
| repo=arguments.get("repo"), | ||
| branch=arguments.get("branch"), | ||
| ) | ||
| ) | ||
|
|
||
| output = arguments.get("output") or "markdown" | ||
| if output not in ("markdown", "json", "both"): | ||
| raise ValueError( | ||
| "Invalid output: must be 'markdown', 'json', or 'both'" | ||
| ) | ||
|
|
||
| # Build responses according to requested format (default markdown) | ||
| results: list[TextContent] = [] | ||
| if output in ("json", "both"): | ||
| results.append(TextContent(type="text", text=json.dumps(comments))) | ||
| if output in ("markdown", "both"): | ||
| try: | ||
| md = generate_markdown(comments) | ||
| except (AttributeError, KeyError, TypeError, IndexError) as exc: | ||
| traceback.print_exc(file=sys.stderr) | ||
| md = f"# Error\n\nFailed to generate markdown from comments: {exc}" | ||
| results.append(TextContent(type="text", text=md)) | ||
| return results | ||
|
|
||
| if name == "resolve_open_pr_url": | ||
| select_strategy = arguments.get("select_strategy") or "branch" | ||
| owner = arguments.get("owner") | ||
| repo = arguments.get("repo") | ||
| branch = arguments.get("branch") | ||
| host = arguments.get("host") | ||
|
|
||
| if not (owner and repo and branch): | ||
| ctx = git_detect_repo_branch() | ||
| owner = owner or ctx.owner | ||
| repo = repo or ctx.repo | ||
| branch = branch or ctx.branch | ||
| host = host or ctx.host | ||
|
|
||
| resolved_url = await _run_with_handling( | ||
| lambda: resolve_pr_url( | ||
| owner=owner or "", | ||
| repo=repo or "", | ||
| branch=branch, | ||
| select_strategy=select_strategy, | ||
| host=host, | ||
| ) | ||
| ) | ||
| return [TextContent(type="text", text=resolved_url)] | ||
|
|
||
| raise ValueError(f"Unknown tool: {name}") | ||
|
|
||
| async def fetch_pr_review_comments( | ||
| self, | ||
| pr_url: str | None, | ||
| *, | ||
| per_page: int | None = None, | ||
| max_pages: int | None = None, | ||
| max_comments: int | None = None, | ||
| max_retries: int | None = None, | ||
| select_strategy: str | None = None, | ||
| owner: str | None = None, | ||
| repo: str | None = None, | ||
| branch: str | None = None, | ||
| ) -> list[CommentResult]: | ||
| """ | ||
| Fetch all review comments for a pull request, resolving the PR | ||
| URL if not provided. | ||
|
|
||
| If pr_url is None, the function attempts to resolve the open PR | ||
| URL using the provided select_strategy, owner, repo, and branch. | ||
| The function parses the PR URL to determine host/owner/repo/pull | ||
| number, queries the repository's GraphQL API to retrieve review | ||
| comments (including resolution/outdated metadata), and returns | ||
| the collected comments. On parse errors or other ValueError | ||
| conditions, returns a single error object. | ||
|
|
||
| Parameters: | ||
| pr_url (str | None): Full GitHub pull request URL to fetch | ||
| comments from. If None, the function will attempt to | ||
| resolve the current PR URL. | ||
| per_page (int | None): (Accepted but not used by this | ||
| implementation) Page size hint for REST pagination. | ||
| max_pages (int | None): (Accepted but not used by this | ||
| implementation) Maximum pages to request when using | ||
| REST. | ||
| max_comments (int | None): Maximum number of comments to | ||
| fetch; honored by the underlying fetcher. | ||
| max_retries (int | None): Maximum retry attempts for | ||
| network/HTTP errors; forwarded to the fetcher. | ||
| select_strategy (str | None): Strategy used when resolving | ||
| an open PR URL (e.g., "branch"); used only if pr_url | ||
| is None. | ||
| owner (str | None): Repository owner to use when resolving | ||
| the PR URL if pr_url is None. | ||
| repo (str | None): Repository name to use when resolving | ||
| the PR URL if pr_url is None. | ||
| branch (str | None): Branch name to use when resolving the | ||
| PR URL if pr_url is None. | ||
|
|
||
| Returns: | ||
| list[CommentResult]: A list of review comment objects or | ||
| error objects. If the fetch succeeds, each item is a | ||
| ReviewComment; if a ValueError occurs while resolving | ||
| or parsing the PR URL, returns a single ErrorMessage | ||
| dict with an "error" key describing the problem. | ||
| """ | ||
| print( | ||
| f"Tool 'fetch_pr_review_comments' called with pr_url: {pr_url}", | ||
| file=sys.stderr, | ||
| ) | ||
| try: | ||
| # If URL not provided, attempt auto-resolution via git + GitHub | ||
| if not pr_url: | ||
| # Reuse the tool to resolve PR URL; keeps behavior consistent | ||
| tool_resp = await self.handle_call_tool( | ||
| "resolve_open_pr_url", | ||
| { | ||
| "select_strategy": select_strategy or "branch", | ||
| "owner": owner, | ||
| "repo": repo, | ||
| "branch": branch, | ||
| }, | ||
| ) | ||
| pr_url = tool_resp[0].text | ||
|
|
||
| host, owner, repo, pull_number_str = get_pr_info(pr_url) | ||
| pull_number = int(pull_number_str) | ||
| # Use GraphQL API to get resolution and outdated status | ||
| comments = await fetch_pr_comments_graphql( | ||
| owner, | ||
| repo, | ||
| pull_number, | ||
| host=host, | ||
| max_comments=max_comments, | ||
| max_retries=max_retries, | ||
| ) | ||
| return comments if comments is not None else [] | ||
| except ValueError as e: | ||
| error_msg = f"Error in fetch_pr_review_comments: {str(e)}" | ||
| print(error_msg, file=sys.stderr) | ||
| traceback.print_exc(file=sys.stderr) | ||
| return [{"error": error_msg}] | ||
|
|
||
| async def run(self) -> None: | ||
| """Start the MCP server.""" | ||
| print("Running MCP Server...", file=sys.stderr) | ||
| # Import stdio here to avoid potential issues with event loop | ||
| from mcp.server.stdio import stdio_server | ||
|
|
||
| async with stdio_server() as (read_stream, write_stream): | ||
| notif = NotificationOptions( | ||
| prompts_changed=False, | ||
| resources_changed=False, | ||
| tools_changed=False, | ||
| ) | ||
| capabilities = self.server.get_capabilities( | ||
| notif, | ||
| experimental_capabilities={}, | ||
| ) | ||
|
|
||
| await self.server.run( | ||
| read_stream, | ||
| write_stream, | ||
| InitializationOptions( | ||
| server_name="github_review_spec_generator", | ||
| server_version="1.0.0", | ||
| capabilities=capabilities, | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| server_instance = ReviewSpecGenerator() | ||
| asyncio.run(server_instance.run()) | ||
| raise SystemExit(main()) |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM: legacy entrypoint delegates to CLI.
Simple, minimal, and avoids import‑time side effects. Please confirm this file is included in the wheel/sdist.
🏁 Script executed:
#!/bin/bash
# Ensure shim is packaged
rg -nP 'package.*src' -C3 pyproject.toml
fd -a src/mcp_server.pyLength of output: 181
I need to locate where mcp_server.py actually exists to determine if it's included in the wheel distribution.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find mcp_server.py location
fd -a mcp_server.py
# Also check the full pyproject.toml configuration for sdist/wheel includes
cat pyproject.toml | grep -A 10 -B 5 "\[tool.hatch"
Length of output: 512
File mcp_server.py is NOT included in wheel/sdist distribution—requires explicit configuration.
The file is located at the repository root (/home/jailuser/git/mcp_server.py), but pyproject.toml only packages src/mcp_github_pr_review_spec_maker for wheels and includes only src/mcp_github_pr_review_spec_maker/mcp.json for sdists. The root-level shim needs either:
- Explicit entry in
[tool.hatch.build.targets.wheel] includeand[tool.hatch.build.targets.sdist] include - Or move into
src/mcp_github_pr_review_spec_maker/
Without this, the entrypoint won't be available after installation.
🤖 Prompt for AI Agents
In mcp_server.py around lines 5 to 8 (repo root), the module is a root-level CLI
shim that is not included in the wheel/sdist as currently configured; either (A)
add explicit include globs for this file in pyproject.toml under
[tool.hatch.build.targets.wheel] include and [tool.hatch.build.targets.sdist]
include (pointing to "mcp_server.py" or the proper path) so the built
distributions contain the file and the entrypoint remains valid, or (B) move
mcp_server.py into the packaged source tree
(src/mcp_github_pr_review_spec_maker/) and update the package
entrypoint/console_script reference to the new module path so it is included
automatically by the existing packaging rules.
| 3. **GraphQL Rate Limit Handling**: | ||
| - Location: `mcp_server.py` lines 260-279 | ||
| - Location: `src/mcp_github_pr_review_spec_maker/server.py` lines 260-279 | ||
| - Currently missing: Add rate limit detection and handling | ||
| - Apply same logic as REST API | ||
|
|
There was a problem hiding this comment.
GraphQL rate‑limit handling not implemented.
Spec calls for RL handling; server.py GraphQL path lacks it. Implement or update docs to “planned”.
Also applies to: 228-236
🧰 Tools
🪛 LanguageTool
[grammar] ~176-~176: There might be a mistake here.
Context: ...ct) 3. GraphQL Rate Limit Handling: - Location: `src/mcp_github_pr_review_spec...
(QB_NEW_EN)
[uncategorized] ~177-~177: The official name of this software platform is spelled with a capital “H”.
Context: ...L Rate Limit Handling**: - Location: src/mcp_github_pr_review_spec_maker/server.py lines 2...
(GITHUB)
[grammar] ~177-~177: There might be a mistake here.
Context: ...view_spec_maker/server.py` lines 260-279 - Currently missing: Add rate limit detect...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In prompts/GITHUB_API_ROBUSTNESS.md around lines 176-180, and in
src/mcp_github_pr_review_spec_maker/server.py around lines 228-236 and 260-279,
the GraphQL request path lacks rate‑limit detection/handling described for the
REST path; implement the same logic for GraphQL by detecting GraphQL rate‑limit
responses (or applicable headers), logging the condition, and applying
retry/backoff behavior (exponential backoff and honoring reset/retry-after when
present) and error propagation; alternatively, if not implementing now, update
the doc text to mark GraphQL rate‑limit handling as "planned" and reference the
REST implementation for parity.
| def mock_rest_client() -> Generator[AsyncMock, None, None]: | ||
| """Fixture to mock httpx.AsyncClient for REST GET requests.""" | ||
| with patch("mcp_server.httpx.AsyncClient") as mock_client_class: | ||
| with patch("mcp_github_pr_review_spec_maker.server.httpx.AsyncClient") as mock_client_class: |
There was a problem hiding this comment.
Fix line length violation.
This line exceeds the 88-character limit (96 characters).
Apply this diff to fix the line length:
- with patch("mcp_github_pr_review_spec_maker.server.httpx.AsyncClient") as mock_client_class:
+ with patch(
+ "mcp_github_pr_review_spec_maker.server.httpx.AsyncClient"
+ ) as mock_client_class:🤖 Prompt for AI Agents
In tests/test_enterprise_url_support.py around line 68, the with patch(...) line
is 96 chars and exceeds the 88-char limit; shorten it by splitting the patch
target into a short variable or by breaking the call across lines. For example,
assign the long target string to a local name (e.g. target =
"mcp_github_pr_review_spec_maker.server.httpx.AsyncClient") and then use with
patch(target) as mock_client_class:, or wrap the patch call over two lines using
parentheses so the line length stays ≤88.
| monkeypatch.setattr("mcp_github_pr_review_spec_maker.git_pr_resolver._graphql_find_pr_number", fake_graphql) | ||
| mock_http_client.add_get_response( |
There was a problem hiding this comment.
Fix ruff E501 (line-length) on monkeypatch targets.
Break arguments across lines to satisfy 88-char limit.
Apply:
- monkeypatch.setattr("mcp_github_pr_review_spec_maker.git_pr_resolver._graphql_find_pr_number", fake_graphql)
+ monkeypatch.setattr(
+ "mcp_github_pr_review_spec_maker.git_pr_resolver._graphql_find_pr_number",
+ fake_graphql,
+ )
- monkeypatch.setattr(
- "mcp_github_pr_review_spec_maker.git_pr_resolver.httpx.AsyncClient", lambda *a, **k: FakeClient(*a, **k)
- )
+ monkeypatch.setattr(
+ "mcp_github_pr_review_spec_maker.git_pr_resolver.httpx.AsyncClient",
+ lambda *a, **k: FakeClient(*a, **k),
+ )
- monkeypatch.setattr(
- "mcp_github_pr_review_spec_maker.git_pr_resolver.httpx.AsyncClient", lambda *a, **k: FakeClient(*a, **k)
- )
+ monkeypatch.setattr(
+ "mcp_github_pr_review_spec_maker.git_pr_resolver.httpx.AsyncClient",
+ lambda *a, **k: FakeClient(*a, **k),
+ )
- monkeypatch.setattr(
- "mcp_github_pr_review_spec_maker.git_pr_resolver.httpx.AsyncClient", lambda *a, **k: FakeClient(*a, **k)
- )
+ monkeypatch.setattr(
+ "mcp_github_pr_review_spec_maker.git_pr_resolver.httpx.AsyncClient",
+ lambda *a, **k: FakeClient(*a, **k),
+ )
- monkeypatch.setattr("mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo", lambda cwd: mock_repo)
+ monkeypatch.setattr(
+ "mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo",
+ lambda cwd: mock_repo,
+ )
- monkeypatch.setattr("mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo", lambda cwd: mock_repo)
+ monkeypatch.setattr(
+ "mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo",
+ lambda cwd: mock_repo,
+ )Optional: import the module and patch attributes directly to avoid long strings:
import mcp_github_pr_review_spec_maker.git_pr_resolver as gpr
monkeypatch.setattr(gpr.httpx, "AsyncClient", FakeClient)Also applies to: 98-99, 165-166, 279-281, 407-408, 431-432
🧰 Tools
🪛 GitHub Actions: Lint
[error] 80-80: E501 Line too long (112 > 88) in test_git_pr_resolver.py.
🤖 Prompt for AI Agents
In tests/test_git_pr_resolver.py around lines 80-81, the monkeypatch.setattr
call exceeds the 88-char line length; break the monkeypatch.setattr arguments
across multiple lines (one per argument) to satisfy ruff E501, or preferably
import the module and patch attributes directly (e.g., import
mcp_github_pr_review_spec_maker.git_pr_resolver as gpr and use
monkeypatch.setattr(gpr, "_graphql_find_pr_number", fake_graphql) or
monkeypatch.setattr(gpr.httpx, "AsyncClient", FakeClient)); apply the same
line-wrapping or direct-import-and-patch fix to the other occurrences at 98-99,
165-166, 279-281, 407-408, and 431-432.
| # Mock git repository setup | ||
| with tempfile.TemporaryDirectory() as temp_repo: | ||
| with patch("git_pr_resolver._get_repo") as mock_get_repo: | ||
| with patch("mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo") as mock_get_repo: |
There was a problem hiding this comment.
Fix line length violation.
This line exceeds the 88-character limit (101 characters).
Apply this diff to fix the line length:
- with patch("mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo") as mock_get_repo:
+ with patch(
+ "mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo"
+ ) as mock_get_repo:📝 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.
| with patch("mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo") as mock_get_repo: | |
| with patch( | |
| "mcp_github_pr_review_spec_maker.git_pr_resolver._get_repo" | |
| ) as mock_get_repo: |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 144-144: E501 Line too long (101 > 88) in test_integration.py.
🤖 Prompt for AI Agents
tests/test_integration.py around line 144: the with patch(...) line is 101 chars
and exceeds the 88-char limit; shorten it by moving the long target string into
a local variable or constant above the with, then call with patch(target) as
mock_get_repo:, or import the symbol or module at the top and patch using the
shorter name; update the single line into two lines (definition of target then
the with statement) so the line lengths are within 88 chars.
| return [{"id": 1}] | ||
|
|
||
| monkeypatch.setattr("mcp_server.fetch_pr_comments_graphql", mock_fetch) | ||
| monkeypatch.setattr("mcp_github_pr_review_spec_maker.server.fetch_pr_comments_graphql", mock_fetch) |
There was a problem hiding this comment.
Fix line length violation.
Line 119 exceeds the 88-character limit (103 characters). As per coding guidelines, all Python code must adhere to a maximum line length of 88 characters.
Apply this diff to fix the line length:
- monkeypatch.setattr("mcp_github_pr_review_spec_maker.server.fetch_pr_comments_graphql", mock_fetch)
+ monkeypatch.setattr(
+ "mcp_github_pr_review_spec_maker.server.fetch_pr_comments_graphql",
+ mock_fetch,
+ )📝 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.
| monkeypatch.setattr("mcp_github_pr_review_spec_maker.server.fetch_pr_comments_graphql", mock_fetch) | |
| monkeypatch.setattr( | |
| "mcp_github_pr_review_spec_maker.server.fetch_pr_comments_graphql", | |
| mock_fetch, | |
| ) |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 119-119: E501 Line too long (103 > 88) in test_mcp_server_tools.py.
🤖 Prompt for AI Agents
In tests/test_mcp_server_tools.py around line 119, the monkeypatch.setattr call
uses a single long dotted-string target that exceeds the 88-char line length;
replace it with the object+attribute form to shorten the line: import or
reference mcp_github_pr_review_spec_maker.server (or assign it to a short local
variable) and call monkeypatch.setattr(<server_module>,
"fetch_pr_comments_graphql", mock_fetch) so the line fits under 88 characters.
| mock_client.__aexit__.return_value = None | ||
|
|
||
| with patch("mcp_server.httpx.AsyncClient", return_value=mock_client): | ||
| with patch("mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", return_value=mock_client): |
There was a problem hiding this comment.
Fix line length violation (exceeds 88 characters).
This line exceeds the maximum line length of 88 characters specified in the coding guidelines and flagged by the pipeline.
Apply this diff to fix the formatting:
- with patch("mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", return_value=mock_client):
+ with patch(
+ "mcp_github_pr_review_spec_maker.server.httpx.AsyncClient",
+ return_value=mock_client,
+ ):📝 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.
| with patch("mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", return_value=mock_client): | |
| with patch( | |
| "mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", | |
| return_value=mock_client, | |
| ): |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 56-56: E501 Line too long (101 > 88) in test_rest_error_handling.py.
🤖 Prompt for AI Agents
In tests/test_rest_error_handling.py around line 56, the with patch(...)
statement exceeds the 88-character line limit; split the patch call across
multiple lines by placing the patched target string and the return_value keyword
on separate lines inside the parentheses (and keep the closing parenthesis and
colon on their own line) so the line lengths stay under 88 characters while
preserving the same behavior.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @petems. * #41 (comment) The following files were modified: * `src/mcp_github_pr_review_spec_maker/cli.py` * `src/mcp_github_pr_review_spec_maker/server.py` * `tests/test_enterprise_url_support.py` * `tests/test_git_pr_resolver.py` * `tests/test_mcp_server_tools.py` * `tests/test_rest_error_handling.py`
Docstrings generation was requested by @petems. * #41 (comment) The following files were modified: * `src/mcp_github_pr_review_spec_maker/cli.py` * `src/mcp_github_pr_review_spec_maker/server.py` * `tests/test_enterprise_url_support.py` * `tests/test_git_pr_resolver.py` * `tests/test_mcp_server_tools.py` * `tests/test_rest_error_handling.py` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
tests/test_rest_error_handling.py (1)
68-68: Fix line length violation (exceeds 88 characters).This line exceeds the maximum line length of 88 characters.
Apply this diff to fix the formatting:
- with patch("mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", return_value=mock_client): + with patch( + "mcp_github_pr_review_spec_maker.server.httpx.AsyncClient", + return_value=mock_client, + ):src/mcp_github_pr_review_spec_maker/server.py (2)
386-388: Add GraphQL rate-limit handling (403/429 + Retry-After/Reset).The REST path in
fetch_pr_commentshandles rate limiting (429/403 responses with Retry-After/X-RateLimit-Reset headers), but the GraphQL path does not. This could lead to failures when rate-limited.Apply a similar status handler for GraphQL requests:
+ async def handle_gql_status( + resp: httpx.Response, attempt: int + ) -> str | None: + if resp.status_code in (403, 429): + retry_after = 60 + h = resp.headers + ra = h.get("Retry-After") + if ra: + try: + retry_after = int(ra) + except ValueError: + retry_after = 60 + else: + reset = h.get("X-RateLimit-Reset") + if reset: + import time + try: + retry_after = max(int(reset) - int(time.time()), 1) + except ValueError: + retry_after = 60 + print( + f"GraphQL rate limited. Sleeping {retry_after}s...", + file=sys.stderr, + ) + await asyncio.sleep(retry_after) + return "retry" + return None + response = await _retry_http_request( - make_graphql_request, max_retries_v + make_graphql_request, max_retries_v, status_handler=handle_gql_status )
612-616: Guard HTTPStatusError when response is None.The exception handler assumes
e.responseis not None and directly accessese.response.status_code, which will raiseAttributeErrorif the response is None.Apply this diff to add a guard:
except httpx.HTTPStatusError as e: + status = getattr(e.response, "status_code", None) # On exhausted 5xx retries, return None per test expectations - if 500 <= e.response.status_code < 600: + if isinstance(status, int) and 500 <= status < 600: return None raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/mcp_github_pr_review_spec_maker/cli.py(1 hunks)src/mcp_github_pr_review_spec_maker/server.py(1 hunks)tests/test_enterprise_url_support.py(4 hunks)tests/test_git_pr_resolver.py(29 hunks)tests/test_mcp_server_tools.py(13 hunks)tests/test_rest_error_handling.py(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_git_pr_resolver.py
- tests/test_enterprise_url_support.py
- tests/test_mcp_server_tools.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Ruff and adhere to a maximum line length of 88 characters
Ensure imports are sorted per Ruff’s configuration
Code must be compatible with and target Python 3.10+ features/configuration
Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval)
Pass static type checks with mypy; add/maintain type annotations as needed
Python files must compile without SyntaxError (compile-check must pass)
**/*.py: Code must pass ruff linting with zero violations (ruff check .)
Code must be formatted with ruff format .
Static type checking must pass with mypy
Use type hints for all function parameters and return values
Follow async/await patterns for I/O operations (GitHub API calls, file operations)
Handle errors gracefully with proper exception handling and logging
Log errors to stderr using print(..., file=sys.stderr)
Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces
Implement retry logic for transient failures honoring HTTP_MAX_RETRIES
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (e.g., exclusive create, path validation)
Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage
Files:
src/mcp_github_pr_review_spec_maker/server.pysrc/mcp_github_pr_review_spec_maker/cli.pytests/test_rest_error_handling.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest for all tests and keep tests under the tests/ directory
Avoid live GitHub calls in tests by default; use mocks/fixtures, and keep real-call integration tests token-gated/conditionally skipped
Files:
tests/test_rest_error_handling.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Place test files under tests/ with filenames matching tests/test_.py
Name test functions with the test_ prefix
Use pytest-asyncio for async test functions
Files:
tests/test_rest_error_handling.py
🧬 Code graph analysis (3)
src/mcp_github_pr_review_spec_maker/server.py (1)
src/mcp_github_pr_review_spec_maker/git_pr_resolver.py (4)
api_base_for_host(130-158)git_detect_repo_branch(82-127)graphql_url_for_host(300-346)resolve_pr_url(161-297)
src/mcp_github_pr_review_spec_maker/cli.py (1)
src/mcp_github_pr_review_spec_maker/server.py (2)
ReviewSpecGenerator(783-1216)run(1187-1216)
tests/test_rest_error_handling.py (1)
src/mcp_github_pr_review_spec_maker/server.py (1)
fetch_pr_comments(459-672)
🪛 GitHub Actions: Lint
src/mcp_github_pr_review_spec_maker/server.py
[error] 44-44: E501 Line too long (96 > 88)
src/mcp_github_pr_review_spec_maker/cli.py
[error] 27-27: E501 Line too long (128 > 88)
[error] 43-43: E501 Line too long (121 > 88)
[error] 87-89: E501 Line too long (189 > 88) / 90:89 / 89:?? (multiple long lines in docstring).
tests/test_rest_error_handling.py
[error] 20-20: E501 Line too long (119 > 88)
🔇 Additional comments (15)
tests/test_rest_error_handling.py (2)
9-9: LGTM!Import path correctly updated to reflect the new package structure.
86-86: LGTM!Patch targets correctly updated to the new module path and remain within line length limits.
Also applies to: 113-113, 132-132, 147-147, 170-170, 191-191, 216-216, 232-232, 254-254, 270-270
src/mcp_github_pr_review_spec_maker/cli.py (3)
1-14: LGTM!Imports are clean and correctly reference the server module.
97-100: LGTM!Correctly uses
load_dotenvwith appropriate override semantics based on whether an explicit env file is provided.
102-110: LGTM!Environment variable overrides are applied conditionally and correctly converted to strings.
src/mcp_github_pr_review_spec_maker/server.py (10)
1-37: LGTM!Imports are well-organized and the module-level
load_dotenv()call ensures environment variables are available for the server configuration.
63-93: LGTM!Configuration loading with bounds checking and override support is correctly implemented with defensive error handling.
96-117: LGTM!Float configuration loading with bounds checking is correctly implemented.
142-154: LGTM!Exponential backoff with jitter is correctly implemented; the
# noqa: S311is appropriate since cryptographic randomness is not required for backoff jitter.
157-216: LGTM!The retry mechanism with configurable status handler is well-designed and correctly implements exponential backoff for transient failures.
675-777: LGTM!Markdown generation properly escapes all user content to prevent XSS, and the dynamic fence selection logic correctly avoids collisions with backticks in the content.
783-930: LGTM!Tool definitions are comprehensive with clear descriptions and well-defined input schemas including appropriate validation bounds.
932-1105: LGTM!Tool dispatch and parameter validation are correctly implemented with comprehensive type checking and range validation. The explicit rejection of boolean values (line 1002) is appropriate given Python's bool-is-int subclass relationship.
1107-1185: LGTM!PR URL resolution and comment fetching logic is correctly implemented with appropriate error handling that degrades gracefully to error objects.
1187-1227: LGTM!MCP server lifecycle implementation follows the standard pattern with proper stdio server setup and capability negotiation.
| def _positive_int(value: str) -> int: | ||
| """ | ||
| Validate and convert a string to a positive integer. | ||
|
|
||
| Parameters: | ||
| value (str): String to parse as an integer. | ||
|
|
||
| Returns: | ||
| int: The parsed integer greater than zero. | ||
|
|
||
| Raises: | ||
| argparse.ArgumentTypeError: If `value` cannot be parsed as an integer or if the parsed integer is not greater than zero. | ||
| """ | ||
| try: | ||
| ivalue = int(value) | ||
| except ValueError as exc: # pragma: no cover - argparse handles messaging | ||
| raise argparse.ArgumentTypeError("must be an integer") from exc | ||
| if ivalue <= 0: | ||
| raise argparse.ArgumentTypeError("must be positive") | ||
| return ivalue |
There was a problem hiding this comment.
Fix line length violation in docstring (exceeds 88 characters).
Line 27 exceeds the 88-character limit.
Apply this diff to fix the formatting:
Raises:
- argparse.ArgumentTypeError: If `value` cannot be parsed as an integer or if the parsed integer is not greater than zero.
+ argparse.ArgumentTypeError: If `value` cannot be parsed as an integer
+ or if the parsed integer is not greater than zero.
"""📝 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.
| def _positive_int(value: str) -> int: | |
| """ | |
| Validate and convert a string to a positive integer. | |
| Parameters: | |
| value (str): String to parse as an integer. | |
| Returns: | |
| int: The parsed integer greater than zero. | |
| Raises: | |
| argparse.ArgumentTypeError: If `value` cannot be parsed as an integer or if the parsed integer is not greater than zero. | |
| """ | |
| try: | |
| ivalue = int(value) | |
| except ValueError as exc: # pragma: no cover - argparse handles messaging | |
| raise argparse.ArgumentTypeError("must be an integer") from exc | |
| if ivalue <= 0: | |
| raise argparse.ArgumentTypeError("must be positive") | |
| return ivalue | |
| def _positive_int(value: str) -> int: | |
| """ | |
| Validate and convert a string to a positive integer. | |
| Parameters: | |
| value (str): String to parse as an integer. | |
| Returns: | |
| int: The parsed integer greater than zero. | |
| Raises: | |
| argparse.ArgumentTypeError: If `value` cannot be parsed as an integer | |
| or if the parsed integer is not greater than zero. | |
| """ | |
| try: | |
| ivalue = int(value) | |
| except ValueError as exc: # pragma: no cover - argparse handles messaging | |
| raise argparse.ArgumentTypeError("must be an integer") from exc | |
| if ivalue <= 0: | |
| raise argparse.ArgumentTypeError("must be positive") | |
| return ivalue |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 27-27: E501 Line too long (128 > 88)
🤖 Prompt for AI Agents
In src/mcp_github_pr_review_spec_maker/cli.py around lines 16 to 35, the
docstring for _positive_int contains a line (line 27) that exceeds the
88-character limit; wrap or reflow the long docstring line(s) so no line is
longer than 88 characters (e.g., break long sentences into multiple shorter
lines maintaining paragraph/indentation), then run your formatter/linters to
ensure the file now complies with line-length rules.
| def parse_args(argv: list[str] | None = None) -> argparse.Namespace: | ||
| """ | ||
| Parse command-line arguments for the MCP GitHub PR Review Spec server. | ||
|
|
||
| Parameters: | ||
| argv (list[str] | None): Optional list of argument strings to parse. If None, the program's arguments are parsed. | ||
|
|
||
| Returns: | ||
| argparse.Namespace: Parsed arguments with fields: | ||
| - env_file (Path | None): Path to a .env file to load, if provided. | ||
| - max_pages (int | None): Override for PR_FETCH_MAX_PAGES. | ||
| - max_comments (int | None): Override for PR_FETCH_MAX_COMMENTS. | ||
| - per_page (int | None): Override for HTTP_PER_PAGE. | ||
| - max_retries (int | None): Override for HTTP_MAX_RETRIES. | ||
| """ | ||
| parser = argparse.ArgumentParser( | ||
| prog="mcp-github-pr-review-spec-maker", | ||
| description="Run the GitHub PR Review Spec MCP server over stdio.", | ||
| ) | ||
| parser.add_argument( | ||
| "--env-file", | ||
| type=Path, | ||
| help="Optional path to a .env file to load before starting the server.", | ||
| ) | ||
| parser.add_argument( | ||
| "--max-pages", | ||
| type=_positive_int, | ||
| help="Override PR_FETCH_MAX_PAGES for this process.", | ||
| ) | ||
| parser.add_argument( | ||
| "--max-comments", | ||
| type=_positive_int, | ||
| help="Override PR_FETCH_MAX_COMMENTS for this process.", | ||
| ) | ||
| parser.add_argument( | ||
| "--per-page", | ||
| type=_positive_int, | ||
| help="Override HTTP_PER_PAGE for this process.", | ||
| ) | ||
| parser.add_argument( | ||
| "--max-retries", | ||
| type=_positive_int, | ||
| help="Override HTTP_MAX_RETRIES for this process.", | ||
| ) | ||
| return parser.parse_args(argv) |
There was a problem hiding this comment.
Fix line length violation in docstring (exceeds 88 characters).
Line 43 exceeds the 88-character limit.
Apply this diff to fix the formatting:
Parameters:
- argv (list[str] | None): Optional list of argument strings to parse. If None, the program's arguments are parsed.
+ argv (list[str] | None): Optional list of argument strings to parse.
+ If None, the program's arguments are parsed.
Returns:📝 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.
| def parse_args(argv: list[str] | None = None) -> argparse.Namespace: | |
| """ | |
| Parse command-line arguments for the MCP GitHub PR Review Spec server. | |
| Parameters: | |
| argv (list[str] | None): Optional list of argument strings to parse. If None, the program's arguments are parsed. | |
| Returns: | |
| argparse.Namespace: Parsed arguments with fields: | |
| - env_file (Path | None): Path to a .env file to load, if provided. | |
| - max_pages (int | None): Override for PR_FETCH_MAX_PAGES. | |
| - max_comments (int | None): Override for PR_FETCH_MAX_COMMENTS. | |
| - per_page (int | None): Override for HTTP_PER_PAGE. | |
| - max_retries (int | None): Override for HTTP_MAX_RETRIES. | |
| """ | |
| parser = argparse.ArgumentParser( | |
| prog="mcp-github-pr-review-spec-maker", | |
| description="Run the GitHub PR Review Spec MCP server over stdio.", | |
| ) | |
| parser.add_argument( | |
| "--env-file", | |
| type=Path, | |
| help="Optional path to a .env file to load before starting the server.", | |
| ) | |
| parser.add_argument( | |
| "--max-pages", | |
| type=_positive_int, | |
| help="Override PR_FETCH_MAX_PAGES for this process.", | |
| ) | |
| parser.add_argument( | |
| "--max-comments", | |
| type=_positive_int, | |
| help="Override PR_FETCH_MAX_COMMENTS for this process.", | |
| ) | |
| parser.add_argument( | |
| "--per-page", | |
| type=_positive_int, | |
| help="Override HTTP_PER_PAGE for this process.", | |
| ) | |
| parser.add_argument( | |
| "--max-retries", | |
| type=_positive_int, | |
| help="Override HTTP_MAX_RETRIES for this process.", | |
| ) | |
| return parser.parse_args(argv) | |
| def parse_args(argv: list[str] | None = None) -> argparse.Namespace: | |
| """ | |
| Parse command-line arguments for the MCP GitHub PR Review Spec server. | |
| Parameters: | |
| argv (list[str] | None): Optional list of argument strings to parse. | |
| If None, the program's arguments are parsed. | |
| Returns: | |
| argparse.Namespace: Parsed arguments with fields: | |
| - env_file (Path | None): Path to a .env file to load, if provided. | |
| - max_pages (int | None): Override for PR_FETCH_MAX_PAGES. | |
| - max_comments (int | None): Override for PR_FETCH_MAX_COMMENTS. | |
| - per_page (int | None): Override for HTTP_PER_PAGE. | |
| - max_retries (int | None): Override for HTTP_MAX_RETRIES. | |
| """ | |
| parser = argparse.ArgumentParser( | |
| prog="mcp-github-pr-review-spec-maker", | |
| description="Run the GitHub PR Review Spec MCP server over stdio.", | |
| ) | |
| parser.add_argument( | |
| "--env-file", | |
| type=Path, | |
| help="Optional path to a .env file to load before starting the server.", | |
| ) | |
| parser.add_argument( | |
| "--max-pages", | |
| type=_positive_int, | |
| help="Override PR_FETCH_MAX_PAGES for this process.", | |
| ) | |
| parser.add_argument( | |
| "--max-comments", | |
| type=_positive_int, | |
| help="Override PR_FETCH_MAX_COMMENTS for this process.", | |
| ) | |
| parser.add_argument( | |
| "--per-page", | |
| type=_positive_int, | |
| help="Override HTTP_PER_PAGE for this process.", | |
| ) | |
| parser.add_argument( | |
| "--max-retries", | |
| type=_positive_int, | |
| help="Override HTTP_MAX_RETRIES for this process.", | |
| ) | |
| return parser.parse_args(argv) |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 43-43: E501 Line too long (121 > 88)
🤖 Prompt for AI Agents
In src/mcp_github_pr_review_spec_maker/cli.py around lines 38 to 82, the
function docstring contains a line (around line 43) that exceeds the
88-character limit; reflow the docstring so no line is longer than 88 characters
by breaking long sentences into shorter lines and/or moving the long "Returns"
description fields onto separate indented lines, keeping the same content and
indentation and preserving the triple-quoted docstring format.
| def main(argv: list[str] | None = None) -> int: | ||
| """ | ||
| Run the CLI entrypoint: load environment variables (optionally from a file), apply any CLI-provided overrides, start the ReviewSpecGenerator server, and return an appropriate exit code. | ||
|
|
||
| Parameters: | ||
| argv (list[str] | None): Command-line arguments to parse; if None, defaults to typical CLI invocation (parse_args will use sys.argv). | ||
|
|
||
| Returns: | ||
| int: Process exit code — `0` on normal completion, `130` if interrupted by KeyboardInterrupt; other exceptions are re-raised. | ||
| """ | ||
| args = parse_args(argv) | ||
|
|
||
| if args.env_file: | ||
| load_dotenv(args.env_file, override=True) | ||
| else: | ||
| load_dotenv(override=False) | ||
|
|
||
| env_overrides = { | ||
| "PR_FETCH_MAX_PAGES": args.max_pages, | ||
| "PR_FETCH_MAX_COMMENTS": args.max_comments, | ||
| "HTTP_PER_PAGE": args.per_page, | ||
| "HTTP_MAX_RETRIES": args.max_retries, | ||
| } | ||
| for key, value in env_overrides.items(): | ||
| if value is not None: | ||
| os.environ[key] = str(value) | ||
|
|
||
| server = ReviewSpecGenerator() | ||
| try: | ||
| asyncio.run(server.run()) | ||
| except KeyboardInterrupt: | ||
| return 130 | ||
| except Exception: # pragma: no cover - surfaces unexpected errors | ||
| print("Unexpected server error", file=sys.stderr) | ||
| raise | ||
| return 0 | ||
|
|
There was a problem hiding this comment.
Fix line length violation in docstring (exceeds 88 characters).
Line 87 significantly exceeds the 88-character limit.
Apply this diff to fix the formatting:
"""
- Run the CLI entrypoint: load environment variables (optionally from a file), apply any CLI-provided overrides, start the ReviewSpecGenerator server, and return an appropriate exit code.
+ Run the CLI entrypoint: load environment variables (optionally from a file),
+ apply any CLI-provided overrides, start the ReviewSpecGenerator server, and
+ return an appropriate exit code.
Parameters:📝 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.
| def main(argv: list[str] | None = None) -> int: | |
| """ | |
| Run the CLI entrypoint: load environment variables (optionally from a file), apply any CLI-provided overrides, start the ReviewSpecGenerator server, and return an appropriate exit code. | |
| Parameters: | |
| argv (list[str] | None): Command-line arguments to parse; if None, defaults to typical CLI invocation (parse_args will use sys.argv). | |
| Returns: | |
| int: Process exit code — `0` on normal completion, `130` if interrupted by KeyboardInterrupt; other exceptions are re-raised. | |
| """ | |
| args = parse_args(argv) | |
| if args.env_file: | |
| load_dotenv(args.env_file, override=True) | |
| else: | |
| load_dotenv(override=False) | |
| env_overrides = { | |
| "PR_FETCH_MAX_PAGES": args.max_pages, | |
| "PR_FETCH_MAX_COMMENTS": args.max_comments, | |
| "HTTP_PER_PAGE": args.per_page, | |
| "HTTP_MAX_RETRIES": args.max_retries, | |
| } | |
| for key, value in env_overrides.items(): | |
| if value is not None: | |
| os.environ[key] = str(value) | |
| server = ReviewSpecGenerator() | |
| try: | |
| asyncio.run(server.run()) | |
| except KeyboardInterrupt: | |
| return 130 | |
| except Exception: # pragma: no cover - surfaces unexpected errors | |
| print("Unexpected server error", file=sys.stderr) | |
| raise | |
| return 0 | |
| def main(argv: list[str] | None = None) -> int: | |
| """ | |
| Run the CLI entrypoint: load environment variables (optionally from a file), | |
| apply any CLI-provided overrides, start the ReviewSpecGenerator server, and | |
| return an appropriate exit code. | |
| Parameters: | |
| argv (list[str] | None): Command-line arguments to parse; if None, defaults to typical CLI invocation (parse_args will use sys.argv). | |
| Returns: | |
| int: Process exit code — `0` on normal completion, `130` if interrupted by KeyboardInterrupt; other exceptions are re-raised. | |
| """ | |
| args = parse_args(argv) | |
| if args.env_file: | |
| load_dotenv(args.env_file, override=True) | |
| else: | |
| load_dotenv(override=False) | |
| env_overrides = { | |
| "PR_FETCH_MAX_PAGES": args.max_pages, | |
| "PR_FETCH_MAX_COMMENTS": args.max_comments, | |
| "HTTP_PER_PAGE": args.per_page, | |
| "HTTP_MAX_RETRIES": args.max_retries, | |
| } | |
| for key, value in env_overrides.items(): | |
| if value is not None: | |
| os.environ[key] = str(value) | |
| server = ReviewSpecGenerator() | |
| try: | |
| asyncio.run(server.run()) | |
| except KeyboardInterrupt: | |
| return 130 | |
| except Exception: # pragma: no cover - surfaces unexpected errors | |
| print("Unexpected server error", file=sys.stderr) | |
| raise | |
| return 0 |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 87-89: E501 Line too long (189 > 88) / 90:89 / 89:?? (multiple long lines in docstring).
🤖 Prompt for AI Agents
In src/mcp_github_pr_review_spec_maker/cli.py around lines 85 to 121, the
module-level docstring for main has a line that exceeds the 88-character limit;
reflow the long paragraph and parameter/returns lines so no line is longer than
88 characters (wrap the long sentence into multiple shorter lines, keep the same
text and indentation inside the existing triple-quoted docstring, and ensure
parameter descriptions remain aligned and readable).
| def escape_html_safe(text: Any) -> str: | ||
| """ | ||
| Escape HTML entities to prevent XSS while preserving readability. | ||
|
|
||
| Parameters: | ||
| text (Any): Value to convert to string and escape; if `None`, a placeholder is returned. | ||
|
|
||
| Returns: | ||
| str: HTML-escaped string safe for inclusion in markdown/HTML, or `"N/A"` when `text` is `None`. | ||
| """ | ||
| if text is None: | ||
| return "N/A" | ||
| return html.escape(str(text), quote=True) |
There was a problem hiding this comment.
Fix line length violation in docstring (exceeds 88 characters).
Line 44 exceeds the 88-character limit.
Apply this diff to fix the formatting:
Parameters:
- text (Any): Value to convert to string and escape; if `None`, a placeholder is returned.
+ text (Any): Value to convert to string and escape; if `None`, a
+ placeholder is returned.
Returns:📝 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.
| def escape_html_safe(text: Any) -> str: | |
| """ | |
| Escape HTML entities to prevent XSS while preserving readability. | |
| Parameters: | |
| text (Any): Value to convert to string and escape; if `None`, a placeholder is returned. | |
| Returns: | |
| str: HTML-escaped string safe for inclusion in markdown/HTML, or `"N/A"` when `text` is `None`. | |
| """ | |
| if text is None: | |
| return "N/A" | |
| return html.escape(str(text), quote=True) | |
| def escape_html_safe(text: Any) -> str: | |
| """ | |
| Escape HTML entities to prevent XSS while preserving readability. | |
| Parameters: | |
| text (Any): Value to convert to string and escape; if `None`, a | |
| placeholder is returned. | |
| Returns: | |
| str: HTML-escaped string safe for inclusion in markdown/HTML, or `"N/A"` when `text` is `None`. | |
| """ | |
| if text is None: | |
| return "N/A" | |
| return html.escape(str(text), quote=True) |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 44-44: E501 Line too long (96 > 88)
🤖 Prompt for AI Agents
In src/mcp_github_pr_review_spec_maker/server.py around lines 39 to 51, the
docstring has a line exceeding the 88-character limit (the "Returns:"
description). Reflow the docstring so no line is longer than 88 characters:
break the long "Returns:" sentence into two shorter lines (e.g., split after
"HTML-escaped string safe for inclusion in markdown/HTML," and continue "or
\"N/A\" when `text` is `None`."), keeping the same wording and indentation, and
ensure the overall triple-quoted block preserves PEP257 style.
| """ | ||
| Create a MagicMock that simulates an HTTP response with configurable status, headers, JSON payload, and optional raise_for_status behavior. | ||
|
|
||
| Parameters: | ||
| status (int): HTTP status code to set on the response. | ||
| json_value (Any, optional): Value to be returned by response.json(); defaults to an empty list when None. | ||
| headers (dict[str, str] | None, optional): Headers to set on the response; defaults to an empty dict when None. | ||
| raise_error (Exception | None, optional): Exception to be raised by response.raise_for_status(); if None, raise_for_status() does nothing. | ||
|
|
||
| Returns: | ||
| MagicMock: A mock response object with attributes `status_code`, `headers`, a `json()` method, and a `raise_for_status()` method configured as specified. | ||
| """ |
There was a problem hiding this comment.
Fix line length violation in docstring (exceeds 88 characters).
Line 20 exceeds the 88-character limit.
Apply this diff to fix the formatting:
"""
- Create a MagicMock that simulates an HTTP response with configurable status, headers, JSON payload, and optional raise_for_status behavior.
+ Create a MagicMock simulating an HTTP response with configurable status,
+ headers, JSON payload, and optional raise_for_status behavior.
Parameters:📝 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.
| """ | |
| Create a MagicMock that simulates an HTTP response with configurable status, headers, JSON payload, and optional raise_for_status behavior. | |
| Parameters: | |
| status (int): HTTP status code to set on the response. | |
| json_value (Any, optional): Value to be returned by response.json(); defaults to an empty list when None. | |
| headers (dict[str, str] | None, optional): Headers to set on the response; defaults to an empty dict when None. | |
| raise_error (Exception | None, optional): Exception to be raised by response.raise_for_status(); if None, raise_for_status() does nothing. | |
| Returns: | |
| MagicMock: A mock response object with attributes `status_code`, `headers`, a `json()` method, and a `raise_for_status()` method configured as specified. | |
| """ | |
| """ | |
| Create a MagicMock simulating an HTTP response with configurable status, | |
| headers, JSON payload, and optional raise_for_status behavior. | |
| Parameters: | |
| status (int): HTTP status code to set on the response. | |
| json_value (Any, optional): Value to be returned by response.json(); defaults to an empty list when None. | |
| headers (dict[str, str] | None, optional): Headers to set on the response; defaults to an empty dict when None. | |
| raise_error (Exception | None, optional): Exception to be raised by response.raise_for_status(); if None, raise_for_status() does nothing. | |
| Returns: | |
| MagicMock: A mock response object with attributes `status_code`, `headers`, a `json()` method, and a `raise_for_status()` method configured as specified. | |
| """ |
🧰 Tools
🪛 GitHub Actions: Lint
[error] 20-20: E501 Line too long (119 > 88)
🤖 Prompt for AI Agents
In tests/test_rest_error_handling.py around lines 19 to 30, the docstring
contains a line exceeding the 88-character limit; split long description lines
into shorter sentences or wrap at natural breaks (e.g., split parameter
descriptions and the return description across multiple lines) so no line is
longer than 88 characters, preserving wording and indentation for the docstring
block.
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor