Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions .claude/skills/ci-push-checklist/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,54 @@ name: ci-push-checklist
description: The required CI validation gauntlet for shipping a code fix to GitGalaxy's core engine (language_standards.py, detector.py, prism.py). Use when preparing to push an issue fix, generating a PR, or validating any change to core parsing logic.
---

When shipping a code fix or addressing a discrepancy in GitGalaxy, the following full validation chain MUST be executed before opening the PR. This ensures parsing accuracy is preserved and data-driven artifacts are synchronized.
When shipping a code fix or addressing a discrepancy in GitGalaxy, the following full validation chain MUST be executed before opening the PR. This ensures parsing accuracy is preserved and data-driven artifacts are synchronized. Rough time budget per section is noted -- most of this is I/O-bound (venv builds, corpus scans) and safe to background while you do something else, not something to babysit synchronously.

## -1. Orient before reading code cold (~1 min)
Before grepping or opening a file to gauge "how big/risky is this," check GitGalaxy's own self-scan of itself first -- it exists specifically to make this near-zero-token:
* **`docs/gitgalaxy_architecture_brief.md`** -- repo-wide blast-radius/risk framing (auto-committed on every merge to main, so it's always close to current HEAD).
* **`docs/self_scan/gitgalaxy_master.db`** (SQLite) -- targeted per-file/function complexity queries. Regenerate with `python tests/tools/self_scan.py` if missing/stale (gitignored on purpose). See the `self-scan-query` skill for query patterns.
See CLAUDE.md's "Using GitGalaxy's self-scan output for orientation" section for the full detail (query examples, full-precision dependency requirements) -- not repeated here.

## 0. Clean Working Directory
* **Untracked Files:** Before committing, ALWAYS run `git status` and review untracked files. Do not use `git add .` blindly. Accidentally committing local virtual environments (e.g. `venv/`, `venv_zero/`) or temporary Python scratch scripts will immediately trigger pipeline failures from the X-Ray Inspector (flagging checked-in binaries) or CodeQL (flagging dirty script code).
* **Explicit Adds:** Prefer `git add <file>` for specific files.

## 1. Local & Unit Validation
## 1. Local & Unit Validation (~1 min)
* **Standalone Regex Re-test:** Isolate the target regex (e.g., `func_start`) against the failing corpus file manually to ensure false positives and negatives are resolved without affecting real matches.
* **Extraction Gauntlet & Strict Tests:** Run `pytest tests/extraction/languages/test_<lang>.py` and `test_<lang>_strict.py` for the language you modified.

## 2. Static Analysis & Linting
## 2. Static Analysis & Linting (~15s)
* **Ruff Formatting & Linting:** `python tests/ruff_audit.py --ci`
* **Mypy Type Checking:** `python tests/mypy_audit.py --ci`

## 3. Global Golden Master Verification
## 3. Global Golden Master Verification (~2-5 min, first run of a fresh venv adds ~30-60s)
* **Run the Crucible Check (Mandatory):** Execute `python tests/tools/crucible_check.py` against the full ~80-repo corpus.
* **Re-Bless Golden Masters:** If `crucible_check.py` shows expected, accurately traced diffs resulting from your fix, bless the new state:
`python tests/tools/crucible_check.py --update --yes`
* **CRITICAL CORPUS WARNING:** NEVER clone a fresh, temporary copy of `language-crucible` inside the `gitgalaxy` workspace just to bypass sandbox or path restrictions. A fresh internal clone alters absolute path metadata and Git footprints, which shifts the entire graph topology and generates massive, invalid diffs that fail in CI. Always point `LANGUAGE_CRUCIBLE_PATH` to the existing pristine sibling directory (e.g., `../language-crucible`) and run with bypass sandbox privileges if needed.
* **Isolate exactly what your change touched, independent of whether the committed fixture is even current:** `python tests/tools/scope_check.py --expect <lang>[,<lang2>]` scans your working tree AND a comparison ref (default `origin/main`) fresh, in separate venvs, and buckets every difference by language -- fails loudly if anything outside `--expect` changed. This answers "is my diff actually scoped to what I meant to touch" directly, without needing the committed golden master to be current first (useful mid-investigation, or after `main` has moved and the committed fixture reflects a bunch of OTHER PRs' legitimate changes you didn't make). Costs roughly 2x a single `crucible_check.py` run (it builds and scans two venvs, not one) -- background it.
* **On the old "never clone a fresh corpus copy" folklore:** a fresh `language-crucible` clone is fine, and both `crucible_check.py` and `scope_check.py` do this routinely (the latter clones a temporary comparison-ref worktree every run). What actually causes massive, invalid-looking diffs is one of two SPECIFIC, now-automatically-checked things, not "metadata" in general (verified by direct repro, PR #2518, 2026-08-30/31 -- see `crucible_check.py`'s own module docstring for the full incident writeups):
1. **Wrong pin.** The corpus isn't on the tag `tests/_crucible_pin.py` names. `crucible_check.py` now warns about this automatically (`_check_corpus_pin`) -- if you see the warning, `git fetch --tags && git checkout <tag>` in the corpus checkout.
2. **Unsafe path.** The corpus's OWN absolute path contains an `IGNORED_DIRECTORIES` name (e.g. `tmp`) as ANY path component, anywhere in the ancestry -- not just the leaf directory name. `guidestar_lens.py`'s documentation-coverage scoring silently skips every such directory, zeroing out that field for the ENTIRE corpus with no error message. `crucible_check.py` now warns about this automatically too (`_check_unsafe_corpus_path`) -- if you see it, move the clone somewhere without `tmp`/`temp`/`cache`/etc. in the path (a true sibling of the repo checkout is always safe).
If you're pointing `LANGUAGE_CRUCIBLE_PATH` at something other than the standard sibling location, these two checks are the actual thing to verify, not a blanket "always reuse the one pristine directory" rule.
* **Two more environment gotchas, both now handled automatically by `crucible_check.py` --
worth knowing if you're extending these tools or writing your own scan comparison, since
neither produces an error message on its own:**
1. **Python version drift.** CI pins a specific interpreter (`.github/workflows/
golden-crucible.yml`'s `python-version:`) -- building a venv with a DIFFERENT version
(e.g. your system default) can resolve different releases of unpinned optional deps
(`networkx`/`pandas`/etc.), producing numeric drift unrelated to your actual change.
`crucible_check.py` now prefers a `uv`-managed interpreter matching CI's pin automatically
(`_find_ci_python`; install `uv` with `curl -LsSf https://astral.sh/uv/install.sh | sh` if
it's not already on PATH -- one-time, ~10s to fetch the pinned Python version after that).
2. **`PYTHONPATH` leaking into a venv-specific subprocess.** If the calling shell/script has
`PYTHONPATH` set to anything containing a real `gitgalaxy/` package (e.g. this repo's own
root -- an easy thing to have set for an unrelated one-off `python -c` import), a
subprocess that inherits the full environment resolves `import gitgalaxy` against THAT
path instead of the venv you actually invoked, silently scanning the wrong code. Every
subprocess in `crucible_check.py`/`scope_check.py` that must run as a specific venv now
goes through `_venv_env()`, which strips it. If you add a new subprocess call that invokes
a venv's python directly, route it through `_venv_env(py)` rather than passing `env=`
ad hoc or omitting `env=` (which inherits everything, unfiltered).

## 4. Discrepancy Ledgers & Tri-Comparison
* **CI now gates this automatically if you touched `detector.py`/`prism.py`/`language_standards.py`:**
Expand Down Expand Up @@ -63,9 +92,10 @@ If `main` advances and causes merge conflicts in `tri_comparison_ledger.json`, `
`git checkout origin/main -- docs/self_scan/tri_comparison_chart.svg docs/self_scan/tri_comparison_ledger.json`
* **CRITICAL LEDGER WARNING**: `tri_comparison_ledger.json` contains *manual annotations* (`status`, `verdict`, `credit_tools`). If you manually validated shapes on your branch, checking out `origin/main` will erase your validations! You MUST back up your manual changes (e.g. write a short Python script to re-apply them), check out `origin/main`, run your script to re-apply your verdicts, and *then* regenerate.
2. Re-run the relevant regen scripts (`crucible_check.py --update --yes`, `tri_comparison_chart.py --all --write`, etc.). The scripts will cleanly recalculate and overwrite the files using your latest code and the upstream's latest ledger baseline.
3. After regenerating, run `python tests/tools/scope_check.py --expect <lang>` once more against `origin/main` (the ref you just merged) to confirm the freshly-regenerated fixture's ONLY real difference from current `main` is your own change -- catches a bad conflict resolution (e.g. accidentally keeping a stale hunk) that a clean regen run alone wouldn't necessarily surface, since regen always "succeeds" even if it baked in something wrong.

## 7. Continuous Integration Monitoring (Agentic)
After pushing your branch and/or opening the PR, you MUST monitor the CI pipeline to ensure it passes.
1. Run `gh run watch` in the background (e.g., using your `run_command` tool with `WaitMsBeforeAsync` set so it detaches to the background).
1. Run `gh run watch` in the background (e.g., using your `run_command` tool with `WaitMsBeforeAsync` set so it detaches to the background) -- Claude Code equivalent: `gh pr checks --watch` via the `Bash` tool with `run_in_background: true`.
2. Do not wait in a polling loop. Once the background task finishes, the system will automatically wake you up with the results.
3. If the CI fails, read the logs, fix the issue, and push the update.
63 changes: 46 additions & 17 deletions gitgalaxy/core/detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,37 @@ def _resolve_class_start_match(match: re.Match, groups_count: int) -> tuple[Opti
{"async", "static", "public", "private", "protected", "abstract", "readonly", "override", "get", "set"}
)

# #2547: satellite names the structural slicer synthesizes for languages/modes with
# no real same-file call graph -- Mode D's (_slice_by_keywords) top-level loose-code
# bucket ("__global_context__", see ~5063) and Mode E's (_slice_by_terminator)
# per-statement-type bucket ("Declarative_Block", see ~5176/5265, or a
# "<KEYWORD>_Statement" name derived from the igniter match, see ~5235). None of
# these are ever real, callable identifiers, so they must never be eligible for
# orphan/duplicate classification below -- their name can never legitimately appear
# a second time in the file, and treating that as "orphaned" or "duplicated" just
# measures the slicer's own bucketing instead of real dead/copy-pasted code. Both
# `Main` and `Anonymous_Block` (Mode D's own top-of-scope/fallback names) plus
# `Unknown_Sat` (legacy) round out the same family. The slicer also appends
# `_[Truncated]`/`_[Unterminated]` to several of these when a scope runs off the end
# of a block (~5039, ~5043, ~5277), so those suffixes are stripped before matching.
_SYNTHETIC_SATELLITE_NAMES = frozenset(
{"Unknown_Sat", "Anonymous_Block", "Main", "Declarative_Block", "__global_context__"}
)
_SYNTHETIC_SATELLITE_SUFFIXES = ("_[Truncated]", "_[Unterminated]")


def _is_synthetic_satellite_name(name: str) -> bool:
base = name
for suffix in _SYNTHETIC_SATELLITE_SUFFIXES:
if base.endswith(suffix):
base = base[: -len(suffix)]
break
if base in _SYNTHETIC_SATELLITE_NAMES:
return True
# Mode E never captures a real identifier for SQL's igniter-based naming --
# it always synthesizes "<IGNITER-KEYWORD>_Statement" (~5235).
return bool(re.fullmatch(r"[A-Z0-9]+_Statement", base))


class StructuralExtractor:
"""
Expand Down Expand Up @@ -1300,23 +1331,21 @@ def splice(
func_name = func.get("name", "")
usage_status = 0 # 0 = Normal

# Check for Duplicates: same name AND materially the same body,
# defined multiple times in the same file.
if (
func_name
and func_name_counts[func_name] > 1
and body_hash_counts[(func_name, func_body_hashes[id(func)])] > 1
):
usage_status = 2 # 2 = Duplicate
duplicate_count += 1
elif len(func_name) > 3 and func_name not in {
"Unknown_Sat",
"Anonymous_Block",
"Main",
"Declarative_Block",
}:
# If the function name only exists where it was defined, it's an orphan
if token_counts[func_name] <= 1:
# #2547: synthetic slicer bucket names (Mode D's "__global_context__",
# Mode E's "<KEYWORD>_Statement"/"Declarative_Block", etc.) are never
# real callable identifiers -- skip them for BOTH the duplicate and
# orphan checks below, not just the orphan one.
if func_name and not _is_synthetic_satellite_name(func_name):
# Check for Duplicates: same name AND materially the same body,
# defined multiple times in the same file.
if (
func_name_counts[func_name] > 1
and body_hash_counts[(func_name, func_body_hashes[id(func)])] > 1
):
usage_status = 2 # 2 = Duplicate
duplicate_count += 1
elif len(func_name) > 3 and token_counts[func_name] <= 1:
# If the function name only exists where it was defined, it's an orphan
orphan_count += 1
usage_status = 1 # 1 = Orphan / Unused

Expand Down
70 changes: 70 additions & 0 deletions tests/core_engine/test_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,76 @@ def test_detector_duplicate_logic_is_scope_blind_to_shadowed_same_name_helpers()
)


def test_detector_orphan_census_excludes_synthetic_slicer_names():
"""
Regression test for #2547: languages sliced by Mode D (_slice_by_keywords) or
Mode E (_slice_by_terminator) synthesize bucket names for structural chunks that
were never real, callable functions -- Mode D's "__global_context__" for
top-level loose code sitting before the first real scope, and Mode E's
"<KEYWORD>_Statement"/"Declarative_Block" per-statement buckets for SQL. These
must never be eligible for orphan/duplicate classification: a synthetic name can
never legitimately appear a second time in the file, so without this exclusion
they were ALWAYS flagged "orphaned", inflating orphaned_logic with non-function
shapes instead of real dead code.
"""
# Mode D: shell. `. ./b.sh` is real top-level code (not a comment) preceding the
# first function -- gets bucketed into a synthetic "__global_context__" satellite.
shell_detector = StructuralExtractor("shell", MOCK_LANG_DEFS)
shell_code = (
". ./b.sh\n"
"\n"
"active_helper() {\n"
" echo hi\n"
"}\n"
"\n"
"forgotten_orphan() {\n"
" echo bye\n"
"}\n"
"\n"
"main_process() {\n"
" active_helper\n"
"}\n"
)
shell_result = shell_detector.splice(shell_code, "")
shell_names = [f["name"] for f in shell_result["functions"]]
assert "__global_context__" in shell_names, "Test setup didn't reproduce the synthetic bucket -- fixture drifted"

synthetic_flagged = [
f["name"]
for f in shell_result["functions"]
if f["name"] == "__global_context__" and f.get("usage_status") != 0
]
assert synthetic_flagged == [], "__global_context__ (non-function slicer bucket) was flagged as orphan/duplicate!"

real_orphans = [f["name"] for f in shell_result["functions"] if f.get("usage_status") == 1]
assert set(real_orphans) == {"forgotten_orphan", "main_process"}, f"Real orphan detection regressed: {real_orphans}"
assert shell_result["equations"].get("orphaned_logic", 0) == 2, (
"orphaned_logic should count only the 2 real uncalled functions, not the synthetic bucket!"
)

# Mode E: sql. Every top-level statement becomes its own satellite, named
# generically from its leading keyword ("SELECT_Statement", "CREATE_Statement",
# ...) -- never a real captured identifier, so none should be orphan-eligible.
sql_detector = StructuralExtractor("sql", MOCK_LANG_DEFS)
sql_code = (
"SELECT * FROM users;\n"
"INSERT INTO users (id) VALUES (1);\n"
"CREATE INDEX idx_users_id ON users (id);\n"
)
sql_result = sql_detector.splice(sql_code, "")
sql_names = [f["name"] for f in sql_result["functions"]]
assert any(name.endswith("_Statement") for name in sql_names), (
"Test setup didn't reproduce Mode E's synthetic per-statement bucket -- fixture drifted"
)

assert all(f.get("usage_status") == 0 for f in sql_result["functions"]), (
f"A synthetic Mode E statement bucket was flagged as orphan/duplicate: {sql_result['functions']}"
)
assert sql_result["equations"].get("orphaned_logic", 0) == 0, (
"orphaned_logic should be 0 -- SQL statements have no real callable names to be orphaned!"
)


def test_detector_c_macro_dead_branch_shield():
"""
Proves the Mode B Preprocessor Shield successfully blanks out dead
Expand Down
Loading
Loading