Main dev - #3
Main dev#3
Conversation
…nonical full-tool list
….json creation - config.py: add settings.json creation in _ensure_default_metadata_files() - organize_issues.py: replace datetime.UTC (Python 3.11+) with timezone.utc (3.10 compat) - manager.py: add dict[str, Any] annotations to issue_data and stats dicts - batch_operations.py: accept token as str | None, fix no-any-return in _get_milestones - issue_data_manager.py: add dict[str, Any] annotation, fix no-any-return - sync_issues.py: add Any import, fix no-redef fallback import, annotate payload - github_helper.py, get_paths.py, create_issue.py, ai_analyzer.py, get_boards.py, get_team_members.py: add type: ignore[no-redef] on fallback imports - ai_helper.py: str() casts for Any returns, fix union-attr, annotate results dict - tests.py: wrap returns in bool() to satisfy no-any-return Fixes: mypy 48 errors -> 0, test_metadata_files_creation failure
There was a problem hiding this comment.
Pull request overview
This PR updates the developer tooling and packaging workflow for sage-github-manager, alongside a set of Python typing/robustness tweaks and documentation cleanups.
Changes:
- Add custom git hooks (
pre-commit,pre-push,post-commit) and simplifyquickstart.shto install them + install editable deps. - Update packaging metadata/extras in
pyproject.toml(version bump, new dev deps). - Minor Python updates: add/adjust typing annotations and normalize return types; fix timezone usage in
organize_issues.py.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/sage_github/tests.py | Normalizes test helper return values to bool. |
| src/sage_github/manager.py | Adds explicit dict typing for parsed issue/stats structures. |
| src/sage_github/issue_data_manager.py | Adds typing to parsed markdown dict; silences json.load return typing. |
| src/sage_github/helpers/sync_issues.py | Adds typing for GraphQL payload; adds mypy ignore on fallback imports. |
| src/sage_github/helpers/organize_issues.py | Replaces datetime.UTC usage with timezone.utc for py3.10 compatibility. |
| src/sage_github/helpers/github_helper.py | Adds mypy ignore on fallback import. |
| src/sage_github/helpers/get_team_members.py | Adds mypy ignore on fallback import. |
| src/sage_github/helpers/get_paths.py | Adds mypy ignore on fallback import (but file structure currently breaks main() definition). |
| src/sage_github/helpers/get_boards.py | Adds mypy ignore on fallback import. |
| src/sage_github/helpers/create_issue.py | Adds mypy ignore on fallback import; silences json.load typing. |
| src/sage_github/helpers/batch_operations.py | Makes token optional and builds headers conditionally. |
| src/sage_github/helpers/ai_helper.py | Ensures string return for title/LLM responses; types results dict. |
| src/sage_github/helpers/ai_analyzer.py | Adds mypy ignore on fallback import. |
| src/sage_github/config.py | Adds settings.json default creation; silences json.load typing on returns. |
| quickstart.sh | Replaces prior interactive installer with hook-copy + editable install flow and doctor mode. |
| pyproject.toml | Version bump to 0.1.1; refactors authors formatting; adds extras/dev deps (incl. publisher). |
| hooks/pre-push | New hook: blocks direct main pushes; attempts version checks/auto-bump; schedules post-push PyPI publish. |
| hooks/pre-commit | New hook: whitespace/conflict/size checks; runs ruff; basic secret/debug detection. |
| hooks/post-commit | New hook: attempts auto-bump version after commit by amending commits (only if _version.py exists). |
| docs/PROJECT_SUMMARY.md | Removes the project summary document. |
| docs/MISSING_FEATURES.md | Fixes status emoji rendering. |
| docs/IMPLEMENTATION_PROGRESS.md | Updates test coverage wording in metrics table. |
| docs/EXTRACTION_SUMMARY.md | Removes extraction summary document. |
| docs/DEVELOPMENT.md | Updates pre-commit usage guidance. |
| DOCUMENTATION_UPDATE_SUMMARY.md | Removes documentation update summary document. |
| .vscode/settings.json | Removes fixed .venv interpreter path. |
| .vscode/README.md | Updates VS Code guidance to use the currently configured Python environment. |
| .github/copilot-instructions.md | Replaces long instructions with a shorter scope/rules document. |
| .github/agents/sage-github.agent.md | Simplifies the agent spec and clarifies repo rules/workflow. |
Comments suppressed due to low confidence (18)
hooks/pre-commit:82
- The hardcoded API key detector uses
grepwithout-E/-P, so\sis treated as a literalsrather than whitespace and the pattern likely never matches. If this hook is intended to block secrets, switch togrep -E(and update the regex accordingly) or use a dedicated secret-scanning tool.
# --- Hardcoded API keys ---
if [ -n "$STAGED_PY" ] && echo "$STAGED_PY" | xargs grep -l "api[_-]key\s*=\s*['\"]sk-" 2>/dev/null | grep -q .; then
echo -e "${RED}✗ Hardcoded API keys detected in staged Python files!${NC}"
exit 1
fi
hooks/pre-commit:72
ruff check .runs a full-repo lint on every commit, which can be slow and will block commits due to unrelated files outside the staged set. Prefer limiting the lint step to staged Python files (or providing a separatepre-push/CI full-repo check) to keep commit-time feedback focused and fast.
echo "🔍 Final lint check on entire repo..."
if ! ruff check .; then
echo -e "${RED}✗ ruff check failed. Run 'ruff check --fix .' to fix.${NC}"
exit 1
fi
echo -e "${GREEN}✓ Lint check passed${NC}"
pyproject.toml:58
devextra includes"sage-github-manager[full]"(a self-referential dependency) whilefull = []is empty. This is redundant at best and can confuse dependency resolution/install tooling. Prefer either (a) moving optional backend deps intofulland duplicating them indev, or (b) removing the self-dependency and leavingfullempty until it has real members.
[project.optional-dependencies]
full = []
dev = [
# dev includes [full] — no need for a separate extras install
"sage-github-manager[full]",
"pytest>=7.0.0",
"pytest-cov>=4.0.0",
"ruff>=0.14.6",
"mypy>=1.11.0",
"pre-commit>=4.0.0",
"types-requests>=2.31.0",
"types-PyYAML>=6.0.0",
"sage-pypi-publisher>=0.1.0",
]
src/sage_github/helpers/get_team_members.py:29
- Same fallback import pattern (
try/except ImportError+sys.pathmutation). If this repo standardizes on fail-fast/no fallbacks, this branch should be removed and the script should require running via the installed package (python -m sage_github.helpers.get_team_members) or CLI.
# Import IssuesConfig robustly whether run as a module or as a script
try:
# Preferred: absolute import via installed package
from sage_github.config import IssuesConfig as Config
except ImportError:
# Fallback: add parent directory to path
sys.path.insert(0, str(Path(__file__).parent.parent))
from config import IssuesConfig as Config # type: ignore[no-redef]
src/sage_github/helpers/get_boards.py:29
- Same fallback import pattern (
try/except ImportError+sys.pathmutation). Since this PR modifies the fallback import line, consider removing this branch and enforcing a single supported execution mode (installed package /python -m ...).
# 动态导入config模块
try:
# 尝试相对导入(当作为模块运行时)
from ..config import IssuesConfig
except ImportError:
# 如果相对导入失败,使用绝对导入
sys.path.insert(0, str(Path(__file__).parent.parent))
from config import IssuesConfig # type: ignore[no-redef]
src/sage_github/helpers/create_issue.py:22
- Same fallback import pattern (
try/except ImportError+sys.pathmutation). Since this PR modifies the fallback import line, consider removing this branch and enforcing a single supported execution mode (installed package /python -m ...).
# 动态导入config模块
try:
# 尝试相对导入(当作为模块运行时)
from ..config import IssuesConfig
except ImportError:
# 如果相对导入失败,使用绝对导入
sys.path.insert(0, str(Path(__file__).parent.parent))
from config import IssuesConfig # type: ignore[no-redef]
quickstart.sh:106
- Hook installation assumes
$PROJECT_ROOT/.git/hooks/exists. If the script is run outside a cloned git repository (or in a worktree without.git/hooks), thecpwill fail and abort due toset -e. Consider adding an explicit check that$PROJECT_ROOT/.git/hooksexists (and a clear error message) before copying hooks.
# ─── Step 2/3: Install Git Hooks ─────────────────────────────────────────────────
echo -e "${YELLOW}${BOLD}Step 2/3: Installing Git hooks${NC}"
if [ -d "$PROJECT_ROOT/hooks" ]; then
installed=0
for hook_src in "$PROJECT_ROOT/hooks"/*; do
hook_name=$(basename "$hook_src")
hook_dst="$PROJECT_ROOT/.git/hooks/$hook_name"
cp "$hook_src" "$hook_dst"
chmod +x "$hook_dst"
echo -e " ${GREEN}✓ $hook_name${NC}"
installed=$((installed + 1))
done
echo -e "${GREEN}✓ $installed hook(s) installed${NC}"
src/sage_github/config.py:155
_load_config_jsoncatches all exceptions, prints a warning, and silently falls back to defaults. This contradicts the repo’s documented rule to avoid fallback logic and fail fast with explicit errors (see.github/copilot-instructions.md“No fallback logic”). Consider letting parse/IO errors propagate or re-raising a clear exception so misconfigured JSON isn’t silently ignored.
try:
with open(config_path, encoding="utf-8") as f:
config = json.load(f)
# print(f"✅ 已加载配置: {config_path}")
return config # type: ignore[no-any-return]
except Exception as e:
print(f"⚠️ 加载配置文件失败 {config_path}: {e}")
src/sage_github/helpers/github_helper.py:22
- Same fallback import pattern (
try/except ImportError+sys.pathmanipulation) as above. Since this PR touches this branch, consider removing the fallback and requiring the helper be executed in a supported way (installed package /python -m ...) to match the repo’s “no fallback logic” rule.
# 动态导入config模块
try:
# 尝试相对导入(当作为模块运行时)
from ..config import IssuesConfig
except ImportError:
# 如果相对导入失败,使用绝对导入
sys.path.insert(0, str(Path(__file__).parent.parent))
from config import IssuesConfig # type: ignore[no-redef]
src/sage_github/helpers/get_paths.py:18
- Same fallback import pattern (
try/except ImportError+sys.pathmutation). Since this PR modifies the fallback import line, consider removing this branch and enforcing a single supported execution mode (installed package /python -m ...).
# 动态导入config模块
try:
# 尝试相对导入(当作为模块运行时)
from ..config import IssuesConfig
except ImportError:
# 如果相对导入失败,使用绝对导入
sys.path.insert(0, str(Path(__file__).parent.parent))
from config import IssuesConfig # type: ignore[no-redef]
hooks/pre-push:165
- The install hint has a typo:
isage-pypi-publisher→sage-pypi-publisher. As written, it will send users to install a non-existent package name.
echo -e "${YELLOW}⚠ sage-pypi-publisher not found, skipping auto-publish${NC}"
echo -e "${DIM} Install: python -m pip install isage-pypi-publisher${NC}"
return
src/sage_github/helpers/get_paths.py:21
main()is currently defined only inside theexcept ImportError:block. If the preferred relative import succeeds, the module will still callmain()underif __name__ == "__main__", causing aNameError. Move themain()definition out of the import fallback block so it is always defined.
try:
# 尝试相对导入(当作为模块运行时)
from ..config import IssuesConfig
except ImportError:
# 如果相对导入失败,使用绝对导入
sys.path.insert(0, str(Path(__file__).parent.parent))
from config import IssuesConfig # type: ignore[no-redef]
def main():
config = IssuesConfig()
src/sage_github/helpers/sync_issues.py:47
- This
try/except ImportError+sys.pathmutation is an explicit fallback import path. The repo’s documented convention is “No fallback logic: fail fast with explicit error messages” (see.github/copilot-instructions.md). Consider removing the fallback branch and requiring the script be run as an installed module (e.g.,python -m sage_github.helpers.sync_issues) or via the CLI entrypoint.
# 动态导入config模块
try:
# 尝试相对导入(当作为模块运行时)
from ..config import IssuesConfig
from ..issue_data_manager import IssueDataManager
except ImportError:
# 如果相对导入失败,使用绝对导入
sys.path.insert(0, str(SCRIPT_DIR.parent))
from config import IssuesConfig # type: ignore[no-redef]
from issue_data_manager import IssueDataManager # type: ignore[no-redef]
hooks/pre-push:239
- This hook relies on GNU-specific tooling/flags (
grep -P,sed -iwithout a backup suffix). On macOS/BSD environments this commonly fails, which would block pushes. Consider rewriting version parsing/updating in Python (reusingpython3) or making the commands POSIX/BSD-compatible.
PACKAGE_NAME=$(grep -oP '^name = "\K[^"]+' pyproject.toml 2>/dev/null || echo "unknown")
# Get version: prefer _version.py (dynamic), fallback to pyproject.toml (static)
CURRENT_VERSION=""
while IFS= read -r VERSION_FILE; do
if [ -f "$VERSION_FILE" ]; then
CURRENT_VERSION=$(grep -oP '__version__ = "\K[^"]+' "$VERSION_FILE" 2>/dev/null || true)
[ -n "$CURRENT_VERSION" ] && break
fi
done < <(find_version_files)
if [ -z "$CURRENT_VERSION" ]; then
CURRENT_VERSION=$(grep -oP '^version = "\K[^"]+' pyproject.toml 2>/dev/null || true)
fi
hooks/pre-push:95
update_versionupdatespyproject.tomland_version.pyfiles, but this repo appears to store__version__insrc/sage_github/__init__.py(not_version.py). If the hook bumps onlypyproject.toml, the runtime__version__will drift from the package metadata. Consider either (a) moving version to a single canonical file that the hook updates, or (b) extending the hook to updatesrc/sage_github/__init__.pyas well.
# Update version in pyproject.toml (static) and/or _version.py (dynamic)
update_version() {
local old_version="$1"
local new_version="$2"
local updated=false
if grep -q '^version = "' pyproject.toml 2>/dev/null; then
sed -i "s/version = \"${old_version}\"/version = \"${new_version}\"/" pyproject.toml
git add pyproject.toml
updated=true
fi
while IFS= read -r VERSION_FILE; do
if [ -f "$VERSION_FILE" ] && grep -q '__version__ = "' "$VERSION_FILE" 2>/dev/null; then
sed -i "s/__version__ = \"${old_version}\"/__version__ = \"${new_version}\"/" "$VERSION_FILE"
git add "$VERSION_FILE"
updated=true
fi
done < <(find_version_files)
quickstart.sh:120
- This script uses
pip install ...directly. If multiple Python installations exist,pipmay not correspond topython3(which you validate earlier), leading to installing into the wrong environment. Preferpython3 -m pip install ...to guarantee the interpreter/pip pairing.
# ─── Step 3/3: Install package ────────────────────────────────────────────────────
echo -e "${YELLOW}${BOLD}Step 3/3: Installing package (editable)${NC}"
if [ -n "$EXTRAS" ]; then
echo -e " ${CYAN}pip install -e .$EXTRAS${NC}"
pip install -e ".$EXTRAS"
else
echo -e " ${CYAN}pip install -e .${NC} (standard — no extras)"
pip install -e .
fi
src/sage_github/helpers/ai_analyzer.py:25
- Same fallback import pattern (
try/except ImportError+sys.pathmutation). Since this PR modifies the fallback import line, consider removing this branch and enforcing a single supported execution mode (installed package /python -m ...).
# 动态导入config模块
try:
# 尝试相对导入(当作为模块运行时)
from ..config import IssuesConfig
except ImportError:
# 如果相对导入失败,使用绝对导入
sys.path.insert(0, str(SCRIPT_DIR.parent))
from config import IssuesConfig # type: ignore[no-redef]
config = IssuesConfig()
hooks/post-commit:93
- The post-commit hook uses
sed -iwithout a backup suffix, which is not portable to BSD/macOSsedand will fail in those environments. Consider using a portable edit approach (e.g.,python3 -cfile rewrite) or handling BSD vs GNUsedexplicitly.
# Update _version.py (single source of truth)
sed -i "s/__version__ = \"${current_version}\"/__version__ = \"${new_version}\"/" "$VERSION_FILE"
git add "$VERSION_FILE"
| self.headers: dict[str, str] = { | ||
| "Accept": "application/vnd.github.v3+json", | ||
| } | ||
| if token: | ||
| self.headers["Authorization"] = f"token {token}" |
There was a problem hiding this comment.
BatchOperations now accepts token: str | None and omits the Authorization header when token is missing. IssuesManager.batch_* passes self.config.github_token (which can be None), so batch operations can run unauthenticated and then misreport success (e.g., close_issues increments success_count without checking _update_issue’s return). Consider failing fast in __init__ when token is None/empty (raise a clear error) and/or ensure callers validate token before running batch operations.
| self.headers: dict[str, str] = { | |
| "Accept": "application/vnd.github.v3+json", | |
| } | |
| if token: | |
| self.headers["Authorization"] = f"token {token}" | |
| if token is None or not token.strip(): | |
| raise ValueError( | |
| "GitHub access token is required for batch operations. " | |
| "Please set GITHUB_TOKEN in your environment or configuration." | |
| ) | |
| self.headers: dict[str, str] = { | |
| "Accept": "application/vnd.github.v3+json", | |
| } | |
| self.headers["Authorization"] = f"token {token}" |
| # Amend commit with the version bump (no new prompt) | ||
| git commit --amend --no-edit --no-verify | ||
|
|
There was a problem hiding this comment.
Amending commits automatically in post-commit (git commit --amend) rewrites commit SHAs after the user commits, which can break workflows (e.g., signing, review tooling, or references copied immediately after commit). If version bumping is required, consider moving it to an explicit release command or CI step rather than mutating local history implicitly.
| if [ "$PYPI_CHECK_RESULT" -eq 0 ]; then | ||
| new_version=$(bump_patch "$CURRENT_VERSION") | ||
| old_version="$CURRENT_VERSION" | ||
| echo -e "${YELLOW}⚠ ${PACKAGE_NAME} ${CURRENT_VERSION} already on PyPI — auto-bumping: ${old_version} → ${new_version}${NC}" | ||
| if update_version "$CURRENT_VERSION" "$new_version"; then | ||
| git commit -m "chore: bump version to ${new_version}" | ||
| CURRENT_VERSION="$new_version" | ||
| echo -e "${GREEN}✓ Bumped to ${new_version}${NC}" |
There was a problem hiding this comment.
The pre-push hook makes repo-modifying commits (git commit ...) during git push and then starts a background publish that builds from the local working tree. Git determines the refs/SHA to push before the hook runs, so a commit created here may not be included in the push; that can result in publishing a version that is not actually present on the remote branch. Strongly consider removing auto-commit and any automatic PyPI publishing from pre-push, or gating it behind an explicit opt-in env var/flag and ensuring the published artifact is built from the exact pushed SHA.
| if [ "${_SAGE_PP_RUNNING:-0}" = "1" ]; then exit 0; fi | ||
|
|
||
| # Publish mode: "public" → publish openly; "private" → internal only | ||
| PUBLISH_MODE=public |
There was a problem hiding this comment.
Defaulting PUBLISH_MODE=public in a pre-push hook makes it easy to accidentally publish unreleased/internal code when a PyPI token is present. Consider defaulting to a non-publishing mode and requiring an explicit opt-in signal (env var / flag) for any publish action.
| PUBLISH_MODE=public | |
| # Default to "private"; allow explicit override via environment (e.g. PUBLISH_MODE=public git push) | |
| PUBLISH_MODE="${PUBLISH_MODE:-private}" |
| # Update version in pyproject.toml (static) and/or _version.py (dynamic) | ||
| update_version() { | ||
| local old_version="$1" | ||
| local new_version="$2" | ||
| local updated=false | ||
|
|
||
| if grep -q '^version = "' pyproject.toml 2>/dev/null; then | ||
| sed -i "s/version = \"${old_version}\"/version = \"${new_version}\"/" pyproject.toml | ||
| git add pyproject.toml | ||
| updated=true | ||
| fi | ||
|
|
||
| while IFS= read -r VERSION_FILE; do | ||
| if [ -f "$VERSION_FILE" ] && grep -q '__version__ = "' "$VERSION_FILE" 2>/dev/null; then | ||
| sed -i "s/__version__ = \"${old_version}\"/__version__ = \"${new_version}\"/" "$VERSION_FILE" |
There was a problem hiding this comment.
sed substitutions treat old_version as a regex, so dots in versions (e.g. 0.1.1) match any character and can replace unintended text. This can corrupt pyproject.toml / version files. Escape the version strings before using them in sed, or use a safer tool (e.g., Python/TOML parser) to update the version field.
| # Update version in pyproject.toml (static) and/or _version.py (dynamic) | |
| update_version() { | |
| local old_version="$1" | |
| local new_version="$2" | |
| local updated=false | |
| if grep -q '^version = "' pyproject.toml 2>/dev/null; then | |
| sed -i "s/version = \"${old_version}\"/version = \"${new_version}\"/" pyproject.toml | |
| git add pyproject.toml | |
| updated=true | |
| fi | |
| while IFS= read -r VERSION_FILE; do | |
| if [ -f "$VERSION_FILE" ] && grep -q '__version__ = "' "$VERSION_FILE" 2>/dev/null; then | |
| sed -i "s/__version__ = \"${old_version}\"/__version__ = \"${new_version}\"/" "$VERSION_FILE" | |
| # Escape a string so it can be used as a literal pattern in sed's search part. | |
| # This escapes regex metacharacters and the '/' delimiter. | |
| escape_sed_regex() { | |
| local s="$1" | |
| s=${s//\\/\\\\} | |
| s=${s//\//\\/} | |
| s=${s//./\\.} | |
| s=${s//\*/\\*} | |
| s=${s//\[/\\[} | |
| s=${s//\]/\\]} | |
| s=${s//^/\\^} | |
| s=${s//\$/\\$} | |
| printf '%s' "$s" | |
| } | |
| # Escape a string so it can be used as a literal replacement in sed. | |
| # This escapes '&', '\' and the '/' delimiter. | |
| escape_sed_replacement() { | |
| local s="$1" | |
| s=${s//\\/\\\\} | |
| s=${s//&/\\&} | |
| s=${s//\//\\/} | |
| printf '%s' "$s" | |
| } | |
| # Update version in pyproject.toml (static) and/or _version.py (dynamic) | |
| update_version() { | |
| local old_version="$1" | |
| local new_version="$2" | |
| local updated=false | |
| local old_pat | |
| local new_rep | |
| old_pat="$(escape_sed_regex "$old_version")" | |
| new_rep="$(escape_sed_replacement "$new_version")" | |
| if grep -q '^version = "' pyproject.toml 2>/dev/null; then | |
| sed -i "s/version = \"${old_pat}\"/version = \"${new_rep}\"/" pyproject.toml | |
| git add pyproject.toml | |
| updated=true | |
| fi | |
| while IFS= read -r VERSION_FILE; do | |
| if [ -f "$VERSION_FILE" ] && grep -q '__version__ = "' "$VERSION_FILE" 2>/dev/null; then | |
| sed -i "s/__version__ = \"${old_pat}\"/__version__ = \"${new_rep}\"/" "$VERSION_FILE" |
| if has_pypi_token; then | ||
| echo -e "${BLUE}📦 Auto-publishing ${CURRENT_VERSION} to PyPI (token found)...${NC}" | ||
| WANT_PUBLISH=true | ||
| else | ||
| echo -e "${DIM} (no PyPI token — skipping publish. Add token to ~/.pypirc or set TWINE_PASSWORD)${NC}" | ||
| fi | ||
|
|
||
| if [ "$WANT_PUBLISH" = true ]; then | ||
| schedule_publish "$CURRENT_VERSION" "$PACKAGE_NAME" | ||
| fi |
There was a problem hiding this comment.
Publishing is triggered automatically whenever a PyPI token is detected. This is risky operationally and security-wise (easy to publish unintentionally). Consider gating schedule_publish behind an explicit opt-in environment variable (e.g., SAGE_ENABLE_AUTO_PUBLISH=1) and otherwise skipping publish even if tokens exist.
No description provided.