Skip to content

fix(review): make repo-doc refresh diff-aware and preserve manual edits - #3056

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
fix/repo-doc-diff-aware-refresh
Jul 4, 2026
Merged

fix(review): make repo-doc refresh diff-aware and preserve manual edits#3056
loopover-orb[bot] merged 1 commit into
mainfrom
fix/repo-doc-diff-aware-refresh

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Adds refreshGeneratedDoc(currentContent, generatedSection, markers) (src/review/generated-doc-refresh.ts), a single, generic marker-block refresh primitive: generate (no current file), no-change (a valid marker block already matches the fresh section byte-for-byte), replace (a valid marker block differs -- everything outside it is preserved verbatim), or manual-review-required (no marker block, or a malformed one -- missing start/end, duplicated, or out of order -- fails closed instead of guessing).
  • src/review/repo-doc-render.ts now wraps its ENTIRE output in a start/end marker pair (REPO_DOC_MARKER_START/REPO_DOC_MARKER_END, exported together as REPO_DOC_MARKERS) instead of the single marker comment feat(review): generate CLAUDE.md/AGENT.md from the repo profile, delivered as a PR #3000 shipped, and drops the rendered content's embedded generatedAt timestamp -- that timestamp made the SAME profile render different content on every call, which would have made the no-change comparison never match across refreshes even when nothing meaningful changed. A dedicated test proves the same profile now renders byte-identical output regardless of generatedAt.
  • openRepoDocPullRequest (src/github/repo-doc-pr.ts) now fetches the current AGENTS.md from the target repo's default branch (treating a 404 as first-run, and rethrowing any other failure rather than guessing) and runs it through refreshGeneratedDoc before doing anything else: no-change and manual-review-required both short-circuit with zero branch/commit/PR creation; generate/replace proceed using the resolved content for both the AGENTS.md tree entry and the CLAUDE.md copy-mode fallback.

Part of #2993 (repo-doc generation roadmap). Closes #3004.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint (not run -- no workflow files touched)
  • npm run typecheck
  • npm run test:coverage -- scoped to the changed files: 100% statements/branches/functions/lines on src/review/generated-doc-refresh.ts, src/review/repo-doc-render.ts, and src/github/repo-doc-pr.ts. Full unsharded test:coverage left to CI per this repo's own "don't duplicate CI locally" convention; ran the affected suites directly instead (test/unit/generated-doc-refresh.test.ts, test/unit/repo-doc-render.test.ts, test/unit/repo-doc-pr.test.ts, plus test/unit/repo-profile.test.ts and test/unit/no-direct-octokit.test.ts as regression checks, and npm run test:changed against origin/main to confirm scope -- 97/97 passing).
  • npm run test:workers (not run -- no Cloudflare-Workers-pool-specific code touched)
  • npm run build:mcp / npm run test:mcp-pack (not run -- no MCP package changes)
  • npm run ui:openapi:check / npm run ui:lint / npm run ui:typecheck / npm run ui:build (not run -- no apps/gittensory-ui changes, no API/OpenAPI surface changed)
  • npm audit --audit-level=moderate (not run locally -- no dependency changes; CI's dependency-review job covers this)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Skipped checks above are for UI/MCP/Workers surfaces this PR does not touch. CI's validate job runs them as a backstop.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics. (Unchanged from feat(review): generate CLAUDE.md/AGENT.md from the repo profile, delivered as a PR #3000 -- the generated content and PR body describe only repo-derived facts.)
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (This PR adds a new GitHub read (Contents API) ahead of the existing write chain; covered by tests for a 404 first-run, a non-404 rethrow, a missing-.content-field response, no-change, manual-review-required, and a real refresh that preserves surrounding manual content.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A -- no API/OpenAPI/MCP surface changed.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A -- no UI changes.)
  • Visible UI changes include a UI Evidence section below with screenshots. (N/A -- no visible UI changes.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A.)

Notes

  • A real bug surfaced while writing the round-trip test: the marker-block extraction originally stopped exactly at the end marker's own text, but a renderer's output always carries one trailing newline as part of what it considers "the generated section" -- so a freshly re-extracted section could never equal a freshly rendered one, and no-change would never fire for any real content. Fixed by having the marker match consume one immediately-following newline; a regression test renders real content via renderRepoDocContent and asserts it round-trips as no-change against itself, both alone and with manual content wrapped around it.
  • refreshGeneratedDoc is intentionally generic (marker pair passed in, not hardcoded) so feat(review): generate repo-specific Claude Code / Codex skill files #3001's future skill-file generator and feat(selfhost): scheduled + on-demand refresh for repo-doc generation #3003's future scheduled-refresh no-meaningful-change check can both call this SAME function rather than growing a second, divergent diff implementation, per the issue's own requirement.
  • CLAUDE.md is not independently diffed -- per feat(review): generate CLAUDE.md/AGENT.md from the repo profile, delivered as a PR #3000's design, it is always fully derived from whatever AGENTS.md content refresh resolves to (a symlink pointer, or a byte-identical copy), so there is no separate "did CLAUDE.md drift" question to answer.

Wraps generated AGENTS.md content in a start/end marker pair
(src/review/generated-doc-refresh.ts) and teaches openRepoDocPullRequest
to recompute only that span against the current file on the default
branch: unchanged content skips the PR entirely, a changed section is
replaced while everything outside the markers is preserved byte-for-byte,
and a missing or malformed marker block fails closed with a reason
instead of guessing. Also drops the rendered content's embedded
timestamp, since it would otherwise make the no-change comparison never
match across refreshes of an unchanged profile.

Closes #3004. Part of #2993.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 4, 2026
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 4, 2026
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.11%. Comparing base (b791587) to head (1b345b4).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3056   +/-   ##
=======================================
  Coverage   96.11%   96.11%           
=======================================
  Files         263      264    +1     
  Lines       28960    29001   +41     
  Branches    10537    10549   +12     
=======================================
+ Hits        27834    27875   +41     
  Misses        492      492           
  Partials      634      634           
Files with missing lines Coverage Δ
src/github/repo-doc-pr.ts 100.00% <100.00%> (ø)
src/review/generated-doc-refresh.ts 100.00% <100.00%> (ø)
src/review/repo-doc-render.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-04 11:57:27 UTC

6 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The change adds a deterministic marker-block refresh primitive and wires repo-doc PR creation through it before creating a branch, preserving manual content outside the generated section and skipping no-op refreshes. The new renderer output is stable across generatedAt changes, and the tests cover the main production paths including first-run, no-change, malformed/no-marker fail-closed, and preserving surrounding content. I do not see a reachable correctness defect in the provided changed files.

Nits — 5 non-blocking
  • nit: src/review/generated-doc-refresh.ts:21 should defensively reject empty marker strings because countOccurrences would never advance if a future caller passed an empty start or end marker.
  • nit: src/github/repo-doc-pr.ts:59 treats a Contents API response without string .content as first-run, but a directory/submodule response at AGENTS.md is not the same as an absent file; consider returning a manual-review-required reason instead of attempting to replace an ambiguous path.
  • src/review/generated-doc-refresh.ts:21 add an early marker validation branch such as `if (!markers.start || !markers.end) return { action: "manual-review-required", reason: "generated-content markers must be non-empty" };` or keep findMarkerBlock private to known constants only.
  • src/github/repo-doc-pr.ts:65 distinguish array/directory Contents API responses from 404 so the caller fails closed when AGENTS.md is not a regular file.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3004
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 59 registered-repo PR(s), 50 merged, 454 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 59 PR(s), 454 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 59 PR(s), 454 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit acba256 into main Jul 4, 2026
10 checks passed
@loopover-orb
loopover-orb Bot deleted the fix/repo-doc-diff-aware-refresh branch July 4, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

fix(review): make repo-doc refresh diff-aware and preserve manual edits

1 participant