From d7878a5883e1d489ddc8bfe4121fd1d3694e6fd5 Mon Sep 17 00:00:00 2001 From: DreamLinx Date: Sat, 15 Aug 2026 23:19:33 +0800 Subject: [PATCH] fix: address codex review findings on #168-#172 range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 (docs, #169 residual): active user-facing surfaces still advertising the removed post-commit workflow — codeindex init's final output, README_zh.md (bullet/workflow/skill-table; EN was clean), examples/ai-integration-guide.md (whole Post-commit section + doc-only scenario), team-workflow-guide ('Three hooks'). All removed/updated; version-history mentions intentionally kept. P2 (#171): _has_js_test_files walked .venv and duplicated the skip-set — now shares _WALK_SKIP_DIRS with detect_languages (venv/.tox/.eggs pruned), uncapped-walk rationale documented. P2 (#172): leftover-hook read_text now catches OSError — status is informational, must not crash on an unreadable file. --- CHANGELOG.md | 15 ++++++ README_zh.md | 3 -- docs/development/team-workflow-guide.md | 2 +- examples/ai-integration-guide.md | 63 +------------------------ src/codeindex/cli_config.py | 2 +- src/codeindex/cli_hooks.py | 5 +- src/codeindex/init_wizard.py | 16 +++++-- tests/test_cli_hooks.py | 15 ++++++ tests/test_init_wizard_exclude.py | 13 +++++ 9 files changed, 64 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dab2dee..89d3e48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Post-commit removal swept the remaining active docs** (codex review of + #167): `codeindex init`'s final output no longer promises "Auto-update + README_AI.md on commit"; `README_zh.md` (feature bullet, workflow block, + plugin-skill table row — the English README was already clean), + `examples/ai-integration-guide.md` (10+ mentions incl. a whole + Post-commit section and doc-only scenario), and + `docs/development/team-workflow-guide.md` ("Three hooks" → two) no + longer advertise the removed workflow. +- **`hooks status` survives an unreadable leftover hook** (codex review): + the leftover check's `read_text` now catches `OSError` instead of + crashing the informational status output. +- **JS-test detection prunes venvs and shares the walk skip-set** (codex + review of #165): `_has_js_test_files` no longer walks `.venv`/`venv`/ + `.tox`/`.eggs`, and the skip list is one shared constant with + `detect_languages` so the two walkers cannot drift. - **Leftover post-commit hooks surfaced** (GH #167 follow-up). A `.git/hooks/post-commit` installed by codeindex < 0.37 survives the upgrade silently — the dead wrapper's errors go to diff --git a/README_zh.md b/README_zh.md index 699fdca..6d1fd9a 100644 --- a/README_zh.md +++ b/README_zh.md @@ -42,7 +42,6 @@ codeindex 是一个**开源 CLI**，通过**两阶段流水线**把任意代码 - **两阶段文档流水线**（v0.23.0）——阶段 1：通过 SmartWriter 生成结构化 README_AI.md；阶段 2：AI 为每个模块生成一行式功能描述。AI agent 可以浏览 README_AI.md 层级结构并找到正确的模块，**无需 grep**。 - **智能索引**——分层文档（概览 → 导航 → 详细），为 AI agent 优化，每个文件 ≤10KB（导航索引，而非技术文档——见 [ADR-005](docs/architecture/adr/005-navigation-disclaimer-and-readme-size-cap.md)） - **自动 AI 增强**——当配置了 `ai_command` 时，`scan-all` 会自动启用 AI 模块描述。使用 `--no-ai` 退出 -- **自动更新 hook**——可选的 post-commit hook（`codeindex hooks install`）为变更的目录重新生成 README_AI.md。薄包装模式：`pipx upgrade ai-codeindex` 自动更新 hook 逻辑 ### 解析与分析 @@ -123,7 +122,6 @@ codeindex 是无状态的解析层；LoomGraph 是自包含的知识图谱（SQL # 个人开发者工作流 codeindex init # 配置 CLAUDE.md 集成 codeindex scan-all # 结构化 + AI 描述（自动） -codeindex hooks install post-commit # 提交时自动更新 # Claude Code 读取 README_AI.md → 理解模块用途 → 用 Serena 查细节 ``` @@ -270,7 +268,6 @@ codeindex affected --since HEAD~5 |-------|--------------| | `codeindex:arch` | 从 `README_AI.md` 回答架构 / "X 在哪里" 类问题 | | `codeindex:index` | 引导你完成 `codeindex init` → `scan-all` | -| `codeindex:hooks` | 配置自动更新的 post-commit hook | | `codeindex:update-guide` | 刷新你项目 `CLAUDE.md` 中的 codeindex 章节 | `codeindex init` 还会把 codeindex 章节注入到你项目的 `CLAUDE.md`， diff --git a/docs/development/team-workflow-guide.md b/docs/development/team-workflow-guide.md index 9356800..4a7e66a 100644 --- a/docs/development/team-workflow-guide.md +++ b/docs/development/team-workflow-guide.md @@ -152,7 +152,7 @@ They run in CI but are excluded from the fast loop to keep feedback tight. ## 5. Git Hooks: Automated QA on Every Commit -Three hooks run automatically. You cannot commit or push without passing them. +Two hooks run automatically. You cannot commit or push without passing them. ### pre-commit (runs on `git commit`) Checks **staged Python files only** — fast, targeted. diff --git a/examples/ai-integration-guide.md b/examples/ai-integration-guide.md index 62cca0e..991b061 100644 --- a/examples/ai-integration-guide.md +++ b/examples/ai-integration-guide.md @@ -10,7 +10,6 @@ **codeindex** is a code indexing tool that generates AI-friendly documentation. It provides **Git Hooks management** for automating: - Pre-commit: Code quality checks (lint + debug code detection) -- Post-commit: Automatic README_AI.md updates **Your task**: Update the user's project CLAUDE.md or AGENTS.md so future AI sessions understand how Git Hooks work. @@ -75,37 +74,12 @@ ruff check --fix src/ git commit --no-verify -m "urgent fix" ``` -#### Post-commit Hook - -Runs **after every commit** to keep documentation updated: - -1. Analyzes commit changes automatically -2. Updates `README_AI.md` for affected directories -3. Creates follow-up commit: `docs: auto-update README_AI.md for ` -4. Prevents infinite loops (skips if only doc files changed) - -**Expected behavior**: -```bash -# You commit -git commit -m "feat: add new feature" -# Pre-commit checks... -# ✓ All checks passed! - -# Post-commit runs automatically -# 📝 Post-commit: Analyzing changes... -# → Updating src/README_AI.md -# ✓ Auto-commit created -``` - -**This is normal!** The auto-commit is expected behavior. - ### For AI Code: Important Behaviors **When committing code**: 1. **Hooks run automatically** - No action needed 2. **If lint errors**: Fix errors before retrying commit -3. **Post-commit auto-commits**: Expected, don't be alarmed -4. **Bypass hooks**: Only use `--no-verify` in emergencies +3. **Bypass hooks**: Only use `--no-verify` in emergencies **Typical commit flow**: ```bash @@ -113,8 +87,6 @@ git add src/myfile.py git commit -m "feat: add feature" # → Pre-commit runs (lint + debug check) # → If passed: commit succeeds -# → Post-commit runs (doc update) -# → May create auto-commit ``` **Handling failures**: @@ -224,19 +196,6 @@ Hooks are installed to `.git/hooks/`: - `0` - Pass, commit proceeds - `1` - Fail, commit blocked -### Post-commit: Auto Documentation - -**Runs**: After commit succeeds -**Process**: -1. Analyze commit: `codeindex affected --json` -2. Get affected directories -3. Update README_AI.md for each -4. Create follow-up commit - -**Loop Prevention**: -- Checks if commit only touched doc files -- Skips if only `README_AI.md` or `PROJECT_INDEX.md` changed - --- ## 💡 Common Scenarios @@ -254,11 +213,7 @@ git commit -m "feat: add feature" # ✓ Lint passed # ✓ No debug code -# Post-commit runs -# → Updates src/README_AI.md -# → Creates auto-commit - -# Result: 2 commits (original + auto-doc) +# Result: commit succeeds, docs stay clean ``` ### Scenario 2: Lint Errors @@ -302,20 +257,6 @@ git commit -m "feat: add feature" # ✓ Success ``` -### Scenario 4: Doc-Only Commit - -```bash -# Only update docs -vim docs/guide.md -git commit -m "docs: update guide" - -# Pre-commit runs (no Python files, skipped) -# Post-commit runs -# ✓ Changes below threshold, skipping update - -# Result: 1 commit (no auto-doc needed) -``` - --- ## 🔧 Integration Checklist diff --git a/src/codeindex/cli_config.py b/src/codeindex/cli_config.py index e960105..0b0b7b6 100644 --- a/src/codeindex/cli_config.py +++ b/src/codeindex/cli_config.py @@ -79,7 +79,7 @@ def _print_post_init_message(): console.print(" 2. [cyan]codeindex scan-all[/cyan] → Generate documentation indexes") console.print(" 3. [cyan]codeindex status[/cyan] → Check coverage") console.print("\n[dim]Optional:[/dim]") - console.print(" • [cyan]codeindex hooks install[/cyan] → Auto-update README_AI.md on commit") + console.print(" • [cyan]codeindex hooks install[/cyan] → Lint hooks (pre-commit / pre-push)") console.print( " • [dim]Claude Code:[/dim] [cyan]/plugin install codeindex@codeindex-claude[/cyan]" ) diff --git a/src/codeindex/cli_hooks.py b/src/codeindex/cli_hooks.py index e961275..bf8f41e 100644 --- a/src/codeindex/cli_hooks.py +++ b/src/codeindex/cli_hooks.py @@ -636,7 +636,10 @@ def status(): for name in manager.RETIRED_HOOKS: hook_path = manager.hooks_dir / name if hook_path.exists(): - content = hook_path.read_text() + try: + content = hook_path.read_text() + except OSError: + continue # unreadable — status is informational, don't crash if manager.CODEINDEX_MARKER in content: console.print( f"[yellow]⚠[/yellow] {name}: leftover from a removed " diff --git a/src/codeindex/init_wizard.py b/src/codeindex/init_wizard.py index 6550758..8655775 100644 --- a/src/codeindex/init_wizard.py +++ b/src/codeindex/init_wizard.py @@ -163,6 +163,12 @@ def get_parser_install_guidance(languages: list[str]) -> dict: return result + +# Directories never worth walking: dependencies / caches / venvs. Shared by +# detect_languages and _has_js_test_files so one list cannot drift (GH #165). +_WALK_SKIP_DIRS = {"node_modules", ".git", "__pycache__", ".venv", "venv", ".tox", ".eggs"} + + def detect_languages(project_dir: Path, max_scan_files: int = 1000) -> List[str]: """Auto-detect programming languages in the project. @@ -177,7 +183,7 @@ def detect_languages(project_dir: Path, max_scan_files: int = 1000) -> List[str] scanned_files = 0 # Common directories to skip - skip_dirs = {"node_modules", ".git", "__pycache__", ".venv", "venv", ".tox", ".eggs"} + skip_dirs = _WALK_SKIP_DIRS for root, dirs, files in os.walk(project_dir): # Remove skip directories from walk @@ -270,10 +276,14 @@ def infer_include_patterns(project_dir: Path) -> List[str]: def _has_js_test_files(project_dir: Path) -> bool: - """True if the project contains JS/TS co-located test files or a __tests__ dir.""" + """True if the project contains JS/TS co-located test files or a __tests__ dir. + + Uncapped walk (early-exit on first hit) is fine here: ``codeindex init`` + is a one-shot interactive command and a later scan walks the tree anyway. + """ suffixes = (".spec.ts", ".spec.tsx", ".spec.js", ".test.ts", ".test.tsx", ".test.js") for root, dirs, files in os.walk(project_dir): - dirs[:] = [d for d in dirs if d not in ("node_modules", ".git", "__pycache__")] + dirs[:] = [d for d in dirs if d not in _WALK_SKIP_DIRS] if "__tests__" in dirs: return True if any(f.endswith(suffixes) for f in files): diff --git a/tests/test_cli_hooks.py b/tests/test_cli_hooks.py index d8dee45..89db21e 100644 --- a/tests/test_cli_hooks.py +++ b/tests/test_cli_hooks.py @@ -353,3 +353,18 @@ def test_uninstall_all_removes_leftover(self, tmp_path): assert result.exit_code == 0 assert not (repo / ".git" / "hooks" / "post-commit").exists() + + def test_status_survives_unreadable_leftover(self, tmp_path): + """codex review, #167 follow-up: an unreadable leftover must not + crash status (it is informational output).""" + repo = self._make_repo(tmp_path) + self._write_leftover(repo) + + from unittest.mock import patch as mock_patch + + with mock_patch( + "pathlib.Path.read_text", side_effect=PermissionError("denied") + ): + result = self._run_cli(repo, ["status"]) + + assert result.exit_code == 0 diff --git a/tests/test_init_wizard_exclude.py b/tests/test_init_wizard_exclude.py index 273e203..0d4fff6 100644 --- a/tests/test_init_wizard_exclude.py +++ b/tests/test_init_wizard_exclude.py @@ -51,3 +51,16 @@ def test_explain_exclude_lists_test_patterns() -> None: assert "**/*.spec.ts" in text assert "**/__tests__/**" in text + + +def test_venv_spec_files_do_not_trigger_suggestion(tmp_path: Path) -> None: + """A spec file inside a venv is not the user's test — pruned like + detect_languages prunes venvs (codex review, GH #165 follow-up).""" + (tmp_path / "src").mkdir() + (tmp_path / "src" / "main.py").write_text("x = 1\n") + (tmp_path / ".venv").mkdir() + (tmp_path / ".venv" / "some-lib.spec.ts").write_text("describe('dep');\n") + + excludes = infer_exclude_patterns(tmp_path) + + assert not any(p in excludes for p in JS_TS_TEST_PATTERNS)