Skip to content

fix: Phase 1 bug fixes -- manifest, search index, issue #15#17

Closed
costiash wants to merge 8 commits intomainfrom
fix/phase1-bug-fixes
Closed

fix: Phase 1 bug fixes -- manifest, search index, issue #15#17
costiash wants to merge 8 commits intomainfrom
fix/phase1-bug-fixes

Conversation

@costiash
Copy link
Copy Markdown
Owner

Summary

  • Fix paths_manifest.json never being committed by CI/CD - The update-docs workflow regenerated the manifest every 3 hours but only staged docs/, so the root-level manifest was permanently stale
  • Auto-generate .search_index.json in CI/CD pipeline - Content search only worked if someone manually ran build_search_index.py; now it rebuilds alongside every doc fetch
  • Fix issue Content search fails when helper script is called with absolute path #15: content search fails from different working directories - load_search_index() used a bare relative path Path("docs/.search_index.json") that broke when called from outside the repo root; now resolves via __file__
  • Remove all hardcoded doc counts - Replaced static references like "573 paths" and "571 files" with dynamic lookups or generic language, since the actual count drifts between environments

Test plan

  • All 300 tests pass (295 baseline + 5 new)
  • Helper script works (--version, --status, --help)
  • CI/CD workflow includes paths_manifest.json in staging
  • CI/CD workflow runs build_search_index.py
  • No hardcoded "573" or "571" counts in helper script
  • load_search_index() uses __file__-relative path resolution

Generated with Claude Code

costiash and others added 8 commits February 26, 2026 22:46
Comprehensive design for phased migration from shell-script-based
architecture to native Claude Code Plugin. Covers 4 phases:
- Phase 1: Bug fixes (manifest commit, search index CI/CD)
- Phase 2: Plugin addition (dual installation path)
- Phase 3: Migration nudges for existing users
- Phase 4: Pure plugin (shell script removal)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 1 (fix/phase1-bug-fixes): Bug fixes for manifest commit bug,
search index generation, issue #15, and hardcoded doc counts.

Phase 2 (feat/plugin-modernization): Native Claude Code plugin with
/docs command, auto-discoverable Skill, and SessionStart hook.

Both plans follow TDD with bite-sized tasks and exact file paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The update-docs workflow regenerates paths_manifest.json every 3 hours
but only staged docs/ for commit. The manifest at the repo root was
never committed, causing it to be stale since Dec 2025.

Fix: add paths_manifest.json to the git add command.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add build_search_index.py step to update-docs workflow. The search index
(.search_index.json) was never auto-generated, meaning content search
only worked if someone manually ran the script. Now it rebuilds every
3 hours alongside the doc fetch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
load_search_index() used bare relative Path('docs/.search_index.json')
which fails when the helper script is called from a different working
directory. Now resolves relative to the script's repo root using __file__.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Python scripts used relative paths that broke when the helper was called
from a different working directory. Fix: wrap all Python calls in
subshells that cd to the repo root first.

Also replace all hardcoded '573' doc count references with dynamic
lookups from paths_manifest.json metadata.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The actual number of docs drifts between local (571), installed (586),
and remote (574). Static references like '573 paths' and '571 files'
were always slightly wrong and required manual maintenance.

Replace with dynamic language or dynamic lookups from manifest metadata.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  scripts/lookup
  search.py 137
Project Total  

This report was generated by python-coverage-comment-action

@claude
Copy link
Copy Markdown

claude Bot commented Feb 26, 2026

PR Review: Phase 1 Bug Fixes

@claude
Copy link
Copy Markdown

claude Bot commented Feb 26, 2026

PR Review: Phase 1 Bug Fixes (continued)

Bugs and Correctness

python vs python3 in CI/CD workflow (potential breakage): The new Build search index step uses bare 'python' rather than 'python3'. GitHub Actions ubuntu-latest runners do not have a bare python binary - only python3. The rest of the workflow uses python3 consistently. This could silently fail or invoke Python 2.

wrapped in unit test is fragile: The new test accesses load_search_index.wrapped, which only exists when the function is decorated with a functools.wraps-aware decorator. If the decorator is ever removed, the test raises AttributeError rather than a clean failure. Consider using inspect.getsource(load_search_index) directly instead.

Code Quality

Missing set -e in build step: Without set -e, a failing Python script does not fail the workflow step. Add 'set -e' at the top of the run block so Python exceptions propagate as workflow failures and prevent committing a missing or empty index.

Verbose CI output: build_search_index.py prints a confirmation line for every file indexed (~570+ lines per run, every 3 hours). Consider suppressing per-file output in CI or adding a quiet flag.

Repeated subprocess calls for path count: enhanced_search(), update_all_docs(), and show_version() each spawn a separate Python subprocess to read the same value from paths_manifest.json. Minor overhead but worth caching.

Integration Tests

Pattern-matching tests are brittle: The tests check that strings like 'paths_manifest.json' and 'build_search_index.py' appear anywhere in the workflow YAML, including comments. They do not verify the manifest is staged in the correct git add command or runs in the right job.

What Is Done Well

  • Manifest staging fix: adding paths_manifest.json to git add -A is exactly the right fix.
  • file-relative path resolution in load_search_index() is the canonical approach. The three-level parent chain is correct for scripts/lookup/search.py -> repo root, and works in the installed location too.
  • Dual fix for Issue Content search fails when helper script is called with absolute path #15 (Python file fix plus shell cd wrapping) provides defense in depth.
  • Removing hardcoded counts prevents documentation drift between releases.
  • The safeguard gate on the search index step correctly skips indexing when safeguards abort the sync.

Summary

Issue Severity
python vs python3 in workflow Should fix before merge
Missing set -e in build step Worth fixing
wrapped in unit test Fragile, easy to fix
Verbose CI output Nice to have
Repeated subprocess calls Minor

The manifest staging fix and the file-relative path fix are both correct and address real bugs. The main item to fix before merge is the python vs python3 issue, which could cause silent failures on ubuntu-latest runners.

@costiash
Copy link
Copy Markdown
Owner Author

Closing to address review feedback and clean up commit messages. Will reopen as new PR.

@costiash costiash closed this Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant