Skip to content

fix(cli): apply --no-stats to keep-marker stats line (#1706)#1765

Merged
magyargergo merged 4 commits into
abhigyanpatwari:mainfrom
oddFEELING:fix/no-stats-keep-section-1706
May 22, 2026
Merged

fix(cli): apply --no-stats to keep-marker stats line (#1706)#1765
magyargergo merged 4 commits into
abhigyanpatwari:mainfrom
oddFEELING:fix/no-stats-keep-section-1706

Conversation

@oddFEELING

@oddFEELING oddFEELING commented May 21, 2026

Copy link
Copy Markdown
Contributor

What

Closes #1706.

gitnexus analyze --no-stats is documented as "Omit volatile file/symbol counts from AGENTS.md and CLAUDE.md", but it did not strip the counts from the index-summary line when the block carries a <!-- gitnexus:keep --> marker.

The regular (no keep marker) path already respects --no-stats via generateGitNexusContent (#1477). The keep-marker branch of upsertGitNexusSection, however, rebuilt the stats line itself and always re-injected (N symbols, N relationships, N execution flows)noStats was never passed to that function. For teams that commit a trimmed AGENTS.md / CLAUDE.md with a keep marker, every analyze rewrote that line with different numbers, producing recurring no-value merge conflicts between branches.

How

  • Thread noStats into upsertGitNexusSection.
  • In the keep-marker branch, under --no-stats the stats line becomes Indexed as **<name>** — no parenthetical counts.
  • The project name still refreshes, so a repo rename still propagates.
  • statsPattern's parenthetical is now optional, so a count-free line left by a prior --no-stats run still matches — the name keeps refreshing, and counts return if --no-stats is later dropped.
  • generateAIContextFiles passes options?.noStats into both upsertGitNexusSection calls.

Tests

  • Updated the #1508 review-F3 test: under noStats + keep marker it now asserts the volatile counts are stripped (previously it expected them rewritten), while keeping its Always-Do-tuple corruption guards.
  • Added a test confirming the project name still refreshes under --no-stats + keep when the existing line has no parenthetical to match.

Full test/unit suite is green; prettier --check and eslint clean on the touched files.

Risk / rollback

Low. upsertGitNexusSection is a private function with only the two in-file callers updated here. No public CLI/MCP contract changes — the --no-stats help text already promised this behavior. Revert is a single-commit revert.

…#1706)

The keep-marker branch of upsertGitNexusSection rebuilt the index-summary
line on every analyze and always re-injected the volatile counts,
ignoring --no-stats. For teams that commit a trimmed AGENTS.md/CLAUDE.md
with a gitnexus:keep marker, that produced recurring no-value merge
conflicts — exactly what --no-stats exists to prevent.

Thread noStats into upsertGitNexusSection. Under --no-stats the keep-path
stats line becomes "Indexed as **<name>**" with no (N symbols, ...)
parenthetical; the project name still refreshes so renames propagate.
The statsPattern parenthetical is now optional so a count-free line left
by a prior --no-stats run still matches.
@vercel

vercel Bot commented May 21, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 5m 14s —— View job


PR #1765 Production-Readiness Review

Tasks

  • Post initial comment
  • Read required docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
  • Verify branch hygiene and diff
  • Inspect ai-context.ts — trace noStats propagation
  • Inspect test file — map tests to failure modes
  • Lane A: CLI option propagation
  • Lane B: keep-marker stats-line replacement logic
  • Lane C: generated-template no-stats behavior
  • Lane D: AGENTS.md / CLAUDE.md parity
  • Lane E: repository history regressions (feat: gitnexus:keep marker preserves custom context sections (resubmit of #605) #1508 F1–F5)
  • Lane F: CI and local verification (tests, lint, typecheck, bidi scan)
  • Final verdict

Review bar for this PR

--no-stats must consistently suppress volatile stats counts in every relevant AI-context generation path, including keep-marker updates, without corrupting custom sections, losing suffix prose, freezing repo renames, reintroducing prior keep-marker bugs, or creating misleading CLI file-status output.


Problem being solved

gitnexus analyze --no-stats already stripped counts from freshly-generated AGENTS.md/CLAUDE.md via generateGitNexusContent (non-keep path). The keep-marker path in upsertGitNexusSection was not receiving noStats — it always rebuilt the stats line with live counts, producing count churn on every analyze for teams that commit a lean keep-section. This PR wires noStats into upsertGitNexusSection and makes the parenthetical optional in the matching regex so count-free lines remain updatable.


Current PR state

  • Open. Branch: oddFEELING:fix/no-stats-keep-section-1706main.
  • 2 commits: d1b22a8 (fix), cb6abfd (merge-from-main).
  • Changed files: gitnexus/src/cli/ai-context.ts (+28/-6), gitnexus/test/unit/ai-context.test.ts (+44/-8).
  • CI: 3/6 workflows completed. Gitleaks ✅, Dependency Review ✅, PR Autofix ✅. CodeQL, Docker Build, and CI are still in progress at review time. The unit-test-and-typecheck pass (CI workflow) is not yet confirmed.

Merge status and mergeability

CI is in progress. Cannot confirm merge-readiness until the CI workflow completes. Vercel authorization is preview-deployment-only and is not a merge gate. No unresolved review threads visible.


Repository history considered


Branch hygiene assessment

The branch contains a merge-from-main commit (cb6abfd Merge branch 'main' into fix/no-stats-keep-section-1706). The merge introduces zero code changes itself; it only brings in main's history. The fix commit (d1b22a8) touches exactly two files, both in the CLI+test domain. No release churn, no lockfile, no unrelated domains. Structurally this is clean aside from the merge commit, which repo maintainers may require to be rebased to a linear history.


Understanding of the change

ai-context.ts:

  1. upsertGitNexusSection gains a noStats?: boolean parameter (line 204).
  2. In the keep-marker branch, statsLine is now conditional:
    • noStats=trueIndexed as **${projectName}** (parenthetical dropped)
    • noStats=false/undefinedIndexed as **${projectName}** (${newStatsInner}) (unchanged behavior)
  3. statsPattern changes from /^(?:Indexed as|indexed by GitNexus as) \*\*[^*]+\*\* \([^)]+\)/m to /^(?:Indexed as|indexed by GitNexus as) \*\*[^*]+\*\*(?: \([^)]+\))?/m — the parenthetical group becomes optional.
  4. Both upsertGitNexusSection call sites in generateAIContextFiles now pass options?.noStats.

ai-context.test.ts:

  • Existing noStats + keep test (#1508 review F3) retitled to #1706 and updated: previously expected 42 symbols (was the wrong behavior, not actually testing the bug); now asserts no count parenthetical.
  • New test: noStats + keep marker: project name still refreshes when counts are stripped — seeds a count-free line, runs with noStats: true and a new name, asserts name updated, counts absent, suffix preserved.

Findings

F1 — No explicit test for counts returning after --no-stats is dropped

Risk: A prior --no-stats run leaves Indexed as **Name** (count-free). A later run without --no-stats should restore counts. If not, the flag becomes sticky. This is the "count-free existing line + noStats: false" scenario.

Evidence: Code analysis confirms the path is correct: the optional (?: \([^)]+\))? in statsPattern matches a count-free line (the optional group matches nothing), statsLine includes the parenthetical when noStats is falsy, and .replace(statsPattern, statsLine) appends the counts. The suffix (. Custom.) is preserved because the pattern has no $ anchor. No test exercises this path explicitly.

From the test suite: the rename test (line 674) seeds a count-free line with noStats: true — this confirms the optional group matches correctly. But no test seeds a count-free line and runs with noStats: false to assert counts return.

Recommended fix: Add one test: seed Indexed as **Name**. Custom., run generateAIContextFiles with noStats unset (or false), assert Indexed as **Name** (N symbols, ...). Custom. appears. Fix this →

Blocks merge: maybe. Code is correct; test coverage gap is the only issue.


F2 — New #1706 noStats+keep tests cover only CLAUDE.md, not AGENTS.md

Risk: Both call sites in generateAIContextFiles were updated symmetrically. A future asymmetry wouldn't be caught if tests only assert CLAUDE.md.

Evidence: The two new tests (noStats + keep marker: stats line drops the volatile counts at line 629 and noStats + keep marker: project name still refreshes at line 674) read only CLAUDE.md. The pre-existing AGENTS.md keep-path test at line 532 tests the normal (with-stats) update path for AGENTS.md, confirming the function works for both files. But no test explicitly asserts noStats=true + keep-marker on AGENTS.md.

Both call sites are clearly identical:

// AGENTS.md call site (line 402–408):
await upsertGitNexusSection(agentsPath, content, projectName, stats, options?.noStats);
// CLAUDE.md call site (line 413–419):
await upsertGitNexusSection(claudePath, content, projectName, stats, options?.noStats);

The function is the same; the only difference is the file path. This makes the risk low.

Recommended fix: Extend one new test to also assert AGENTS.md (add agentsPath write/read alongside claudePath in the same tempdir). Fix this →

Blocks merge: maybe. Code is correct; test parity is the gap.


F3 — Merge-from-main commit in branch

Risk: cb6abfd is a merge-from-main commit. The diff of the fix commit is clean (+72/-14, two files, one domain). No code change is in the merge commit itself.

Evidence: git log --oneline HEAD~5..HEAD shows cb6abfd Merge branch 'main' into fix/no-stats-keep-section-1706 as the HEAD commit, ahead of the actual fix commit. The PR was updated with a rebase-to-main merge to resolve any conflicts with recent main commits.

Recommended fix: Rebase onto current main before merge (git rebase origin/main) to linearize history, if repo policy requires it. This is mechanical.

Blocks merge: depends on repo policy. Not a behavioral regression; purely hygiene.


F4 — CI CI workflow not yet confirmed green

Risk: The unit test suite, typecheck, and lint results are unknown at review time.

Evidence: CI summary shows in_progress for the main CI workflow (run #26253272756).

Recommended fix: Wait for CI to complete before merging.

Blocks merge: pending confirmation.


PR-specific assessment sections

Lane A — CLI option propagation: ✅ Confirmed correct. analyze.ts:926 and :1036 both translate options.stats === false to noStats: true. This bridge is untouched and guarded by analyze-no-stats-bridge.test.ts. Both generateAIContextFiles call sites in the keep path now receive options?.noStats. run-analyze.ts:956 passes options.noStats directly. The entire CLI chain is wired end-to-end.

Lane B — keep-marker stats-line replacement: ✅ Confirmed correct. statsLine construction is clean: conditional on noStats, using caller-provided projectName and stats (no fragile re-extraction). Suffix preservation is intact: no $ anchor means . Custom. and similar text after the matched span is untouched. The optional parenthetical (?: \([^)]+\))? makes a count-free line matchable without over-broadening: it requires the full (text) form (needs closing )) and is anchored to line-start. Manual trace confirms counts return correctly when noStats is dropped. Pattern cannot match the verbose-template "This project is indexed by GitNexus as" form — this is a pre-existing limitation predating this PR, not introduced here.

Lane C — generated-template no-stats behavior: ✅ Unchanged. generateGitNexusContent at line 134 correctly handles noStats with its existing ternary. This PR does not touch it.

Lane D — AGENTS.md / CLAUDE.md parity: ✅ Both call sites updated symmetrically. Minor test parity gap noted (F2).

Lane E — repository history regressions:

  • F1 (honest return value when no stats line matches): guarded at test line 704. ✅
  • F2 (structured projectName/stats, no fragile extraction): code uses caller-provided values. ✅
  • F3 (no Always-Do tuple corruption): explicitly guarded with /\(\{target:/ and /direction:\s*"upstream"/ assertions at line 660–661. ✅
  • F4 (regex scope): ^ + m flag prevents mid-paragraph matches. No $ preserves suffix. ✅
  • F5 (test coverage breadth): keep-scope, AGENTS parity, idempotency, CRLF, markdown-sensitive names, noStats+keep, preserved return semantics all covered. Count-return and AGENTS.md parity for noStats have minor gaps (F1, F2).

Lane F — CI / bidi / hygiene:

  • git diff --check on the fix commit: clean (no whitespace errors).
  • file reports both changed files as UTF-8 text with no encoding anomalies.
  • The GitHub bidi warning visible on the diff page is a rendering artifact for Unicode text files in general; no actual bidi control characters were found in the changed content.
  • CI: 3/6 complete (Gitleaks ✅, Dependency Review ✅, PR Autofix ✅). Main CI workflow still in progress.

Back-and-forth avoided by verifying

  • "This project is indexed by GitNexus as" not matched by statsPattern: Confirmed pre-existing; not introduced by this PR. Keep-marker sections use Indexed as **Name** form by convention (verified in all existing tests).
  • The Optional parenthetical causes zero-length match concerns: Confirmed safe — the optional group requires the full (...) balanced form; empty match is only the outer zero-repetition, which is correct behavior.
  • Merge-from-main commit: Confirmed it introduces no code changes; it is purely a history update.
  • Vercel authorization comment: Confirmed preview-deployment-only; not a merge gate.

Open questions

None unavoidable. The two test gaps (F1, F2) are clearly non-blocking given the code correctness is confirmed by trace. CI completion (F4) is the only hard gate.


Final verdict

production-ready with minor follow-ups

The core fix is correct end-to-end: noStats is threaded through all call sites, the optional regex correctly allows name refresh and count return on count-free lines, suffix text is preserved, #1508 regression guards are intact, and the CLI translation bridge is untouched. Two minor test-coverage gaps exist — no test for counts returning after --no-stats is dropped (F1), and the new noStats+keep regression test does not assert AGENTS.md parity (F2) — but the code is demonstrably correct for both cases. The merge-from-main commit should be rebased to linearize history per repo convention. CI must complete before merge.


| CI workflow

@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
9461 9460 0 1 482s

✅ All 9460 tests passed

1 test(s) skipped — expand for details
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 78.21% 31230/39927 N/A% 🟢 ███████████████░░░░░
Branches 66.76% 19900/29804 N/A% 🟢 █████████████░░░░░░░
Functions 82.24% 3183/3870 N/A% 🟢 ████████████████░░░░
Lines 81.58% 28167/34523 N/A% 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@magyargergo

Copy link
Copy Markdown
Collaborator

@oddFEELING can you please look into claude's actionable findings?

@oddFEELING

Copy link
Copy Markdown
Contributor Author

@oddFEELING can you please look into claude's actionable findings?

Giving it a look now

…p path

Addresses review findings F1 and F2 on PR abhigyanpatwari#1765:

- F1: add a test that counts RETURN when --no-stats is dropped after a
  prior count-free run — guards against the flag becoming sticky.
- F2: extend the noStats+keep "drops the volatile counts" test to assert
  AGENTS.md alongside CLAUDE.md, so a future asymmetry between the two
  upsertGitNexusSection call sites is caught.
@oddFEELING

oddFEELING commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the actionable findings in 3726182a:

  • F1: Added noStats + keep marker: counts return when --no-stats is dropped after a count-free run — seeds a count-free keep-section line, runs analyze without --no-stats, and asserts the (N symbols, …) parenthetical comes back. Confirms the flag isn't sticky.
  • F2: Extended the noStats + keep marker: stats line drops the volatile counts test to seed and assert both AGENTS.md and CLAUDE.md (looped, with per-file assertion messages), so a future asymmetry between the two upsertGitNexusSection call sites is caught.

F3 (merge-from-main commit): left as-is for now since the repo squash-merges PRs, so it collapses on merge. Happy to rebase to a linear history if you'd prefer.

F4: CI was already green (9459/9459) on the prior run.

ai-context.test.ts is now 27/27 locally; typecheck, prettier, and eslint all clean.

@magyargergo magyargergo merged commit 954b184 into abhigyanpatwari:main May 22, 2026
32 of 33 checks passed
@magyargergo

Copy link
Copy Markdown
Collaborator

Thank you for your PR!

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.

--no-stats does not strip index-summary symbol counts from AGENTS.md / CLAUDE.md

2 participants