From 5f72fcc44a52a4d8cadf0998a6e74abc8a782957 Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Wed, 7 Oct 2026 17:13:59 +0200 Subject: [PATCH 1/2] chore: mark the synced baseline as generated and explain it in the sync PR A team that keeps the baseline on its branch gets .codeboarding/ in every diff and in the repository's language stats. deliver-sync now appends `.codeboarding/** linguist-generated=true` to .gitattributes inside the same sync commit, for both the push and pull_request strategies, unless a line already decides the attribute for those files (an opt-out included). Other lines are never touched. The rolling sync PR body now says what the files are (generated diagram data and an analysis cache that makes the next run incremental) and that merging lets reviews start from the saved diagram. Co-Authored-By: Claude Opus 5.5 --- scripts/action/deliver-sync.sh | 10 +++- tests/test_action_sync.py | 95 ++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/scripts/action/deliver-sync.sh b/scripts/action/deliver-sync.sh index bbc454c..6559525 100755 --- a/scripts/action/deliver-sync.sh +++ b/scripts/action/deliver-sync.sh @@ -64,6 +64,14 @@ classify_push_failure() { } "$ACTION_PATH/scripts/action/install-sync.sh" > "$GENERATED_PATHS" +# Marked generated, so GitHub collapses the baseline's diffs and leaves it out of +# the language stats. Only when nothing in .gitattributes decides it already: an +# explicit opt-out is as deliberate as an opt-in, and other lines are not ours. +if [ "$(git check-attr linguist-generated -- .codeboarding/analysis.json | awk '{print $NF}')" = unspecified ]; then + [ ! -s .gitattributes ] || [ -z "$(tail -c 1 .gitattributes)" ] || printf '\n' >> .gitattributes + printf '.codeboarding/** linguist-generated=true\n' >> .gitattributes + echo .gitattributes >> "$GENERATED_PATHS" +fi stage_paths=() while IFS= read -r path; do if [ -e "$path" ] || git ls-files --error-unmatch "$path" >/dev/null 2>&1; then @@ -123,7 +131,7 @@ fi if [ -z "$pr_json" ]; then gh pr create --repo "$REPOSITORY" --base "$TARGET_BRANCH" --head "$SYNC_BRANCH" \ --title 'chore(codeboarding): sync analysis baseline' \ - --body "Updates the versioned CodeBoarding analysis for \`$TARGET_BRANCH\`." + --body "Updates the CodeBoarding files in \`.codeboarding/\` for \`$TARGET_BRANCH\`. They are generated, not written by hand: the data behind the architecture diagram, and an analysis cache that lets the next run analyze only what changed. Merging this lets pull request reviews start from the saved diagram instead of building one first." pr_json="$(gh api --method GET "repos/$REPOSITORY/pulls" \ -f state=open -f base="$TARGET_BRANCH" \ -f head="${REPOSITORY%%/*}:$SYNC_BRANCH" --jq '.[0]')" diff --git a/tests/test_action_sync.py b/tests/test_action_sync.py index b0b87aa..2860faa 100644 --- a/tests/test_action_sync.py +++ b/tests/test_action_sync.py @@ -410,3 +410,98 @@ def test_it_publishes_the_analysis_under_both_commits(self) -> None: self.assertEqual(values["baseline_sha"], baseline_sha) self.assertEqual(values["analyzed_sha"], self.analyzed_sha) self.assertNotEqual(values["baseline_sha"], values["analyzed_sha"]) + + def _deliver(self, strategy: str = "push", path: str = os.environ["PATH"]) -> subprocess.CompletedProcess: + output = self.root / "github-output" + output.write_text("", encoding="utf-8") + result = subprocess.run( + [str(ROOT / "scripts" / "action" / "deliver-sync.sh")], + env={ + "PATH": path, + "PYTHONPATH": str(self.core), + "ACTION_PATH": str(ROOT), + "ANALYSIS_DIR": str(self.analysis), + "CHECKOUT_DIR": str(self.checkout), + "GITHUB_OUTPUT": str(output), + "RUNNER_TEMP": str(self.root), + "GITHUB_SERVER_URL": str(self.root), + "GITHUB_TOKEN": "unused", + "GH_HOST": "github.com", + "REPOSITORY": "owner/repo", + "TARGET_BRANCH": "main", + "SYNC_STRATEGY": strategy, + }, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + return result + + def _committed_attributes(self, ref: str = "HEAD") -> str: + return self._git(self.checkout, "show", f"{ref}:.gitattributes") + "\n" + + def _commit_attributes(self, content: str) -> None: + (self.checkout / ".gitattributes").write_text(content, encoding="utf-8") + self._git(self.checkout, "add", ".gitattributes") + self._git(self.checkout, "commit", "-m", "attributes") + self._git(self.checkout, "push", "--quiet", "origin", "main") + + def test_the_baseline_is_marked_generated_in_the_sync_commit(self) -> None: + # GitHub collapses generated files in a diff and leaves them out of the + # language stats, which is what a reviewer wants from a baseline. + self._deliver() + + self.assertEqual(self._committed_attributes(), ".codeboarding/** linguist-generated=true\n") + changed = self._git(self.checkout, "show", "--name-only", "--format=", "HEAD").splitlines() + self.assertIn(".gitattributes", changed) + self.assertIn(".codeboarding/analysis.json", changed) + + def test_existing_attributes_are_kept_and_appended_to(self) -> None: + self._commit_attributes("*.png binary\n*.sh text eol=lf") + + self._deliver() + + self.assertEqual( + self._committed_attributes(), "*.png binary\n*.sh text eol=lf\n.codeboarding/** linguist-generated=true\n" + ) + + def test_a_line_that_already_covers_it_is_left_alone(self) -> None: + self._commit_attributes("/.codeboarding/* linguist-generated\n") + + self._deliver() + + self.assertEqual(self._committed_attributes(), "/.codeboarding/* linguist-generated\n") + + def test_an_explicit_opt_out_is_respected(self) -> None: + self._commit_attributes(".codeboarding/** -linguist-generated\n") + + self._deliver() + + self.assertEqual(self._committed_attributes(), ".codeboarding/** -linguist-generated\n") + + def test_the_sync_pull_request_says_what_the_files_are(self) -> None: + bin_dir = self.root / "bin" + bin_dir.mkdir() + calls = self.root / "gh-calls" + created = self.root / "gh-created" + pr = '{"html_url": "https://github.com/owner/repo/pull/1", "number": 1, "base": {"ref": "main"}}' + (bin_dir / "gh").write_text( + "#!/usr/bin/env bash\n" + f'printf "%s\\n----\\n" "$*" >> "{calls}"\n' + 'case "$1" in\n' + f' pr) touch "{created}" ;;\n' + f" api) [ ! -f \"{created}\" ] || echo '{pr}' ;;\n" + "esac\n", + encoding="utf-8", + ) + (bin_dir / "gh").chmod(0o755) + + self._deliver("pull_request", path=f"{bin_dir}:{os.environ['PATH']}") + + create = next(c for c in calls.read_text().split("\n----\n") if c.startswith("pr create")) + self.assertIn("They are generated, not written by hand", create) + self.assertIn("analysis cache", create) + self.assertIn("start from the saved diagram", create) + self._git(self.checkout, "fetch", "--quiet", "origin", "codeboarding/sync") + self.assertEqual(self._committed_attributes("FETCH_HEAD"), ".codeboarding/** linguist-generated=true\n") From fc000957607b03a6d24d04c0c5d79ca1b8cc464c Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Wed, 7 Oct 2026 17:33:56 +0200 Subject: [PATCH 2/2] fix: mark only generated baseline files, and only alongside a real change Review follow-ups on the attributes line: - It is added after the unchanged check, so it never makes a commit of its own: the push strategy no longer pushes an attributes-only commit, and the pull_request strategy no longer re-creates a sync PR for it. - It names the generated files (analysis.json, fingerprint.json, static_analysis.pkl, static_analysis.sha, codeboarding_version.json, health/health_report.json) instead of .codeboarding/**, which also covered .codeboardingignore and the health configuration people write. Each path some line already decides is left alone. Co-Authored-By: Claude Opus 5.5 --- scripts/action/deliver-sync.sh | 27 +++++++++++++------ tests/test_action_sync.py | 49 +++++++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/scripts/action/deliver-sync.sh b/scripts/action/deliver-sync.sh index 6559525..d0ce858 100755 --- a/scripts/action/deliver-sync.sh +++ b/scripts/action/deliver-sync.sh @@ -64,14 +64,6 @@ classify_push_failure() { } "$ACTION_PATH/scripts/action/install-sync.sh" > "$GENERATED_PATHS" -# Marked generated, so GitHub collapses the baseline's diffs and leaves it out of -# the language stats. Only when nothing in .gitattributes decides it already: an -# explicit opt-out is as deliberate as an opt-in, and other lines are not ours. -if [ "$(git check-attr linguist-generated -- .codeboarding/analysis.json | awk '{print $NF}')" = unspecified ]; then - [ ! -s .gitattributes ] || [ -z "$(tail -c 1 .gitattributes)" ] || printf '\n' >> .gitattributes - printf '.codeboarding/** linguist-generated=true\n' >> .gitattributes - echo .gitattributes >> "$GENERATED_PATHS" -fi stage_paths=() while IFS= read -r path; do if [ -e "$path" ] || git ls-files --error-unmatch "$path" >/dev/null 2>&1; then @@ -105,6 +97,25 @@ if git diff --cached --quiet || git diff --cached --quiet -I '"generated_at"' -I exit 0 fi +# Marked generated, so GitHub collapses the baseline's diffs and leaves it out of +# the language stats. Added only alongside a real baseline change, so it never +# makes a commit or a sync pull request of its own. Each generated path is named, +# since .codeboarding/ also holds configuration people write; a path some line +# already decides (an opt-out included) is left to that line, and no other line +# is touched. +mark_generated() { + local path missing=() + for path in .codeboarding/analysis.json .codeboarding/fingerprint.json .codeboarding/static_analysis.pkl \ + .codeboarding/static_analysis.sha .codeboarding/codeboarding_version.json .codeboarding/health/health_report.json; do + [ "$(git check-attr linguist-generated -- "$path" | awk '{print $NF}')" != unspecified ] || missing+=("$path") + done + [ "${#missing[@]}" -gt 0 ] || return 0 + [ ! -s .gitattributes ] || [ -z "$(tail -c 1 .gitattributes)" ] || printf '\n' >> .gitattributes + printf '%s linguist-generated=true\n' "${missing[@]}" >> .gitattributes + git add .gitattributes +} +mark_generated + git commit -m 'chore(codeboarding): sync analysis baseline' >/dev/null if [ "$SYNC_STRATEGY" = push ]; then diff --git a/tests/test_action_sync.py b/tests/test_action_sync.py index 2860faa..dff8696 100644 --- a/tests/test_action_sync.py +++ b/tests/test_action_sync.py @@ -438,6 +438,18 @@ def _deliver(self, strategy: str = "push", path: str = os.environ["PATH"]) -> su self.assertEqual(result.returncode, 0, result.stderr or result.stdout) return result + GENERATED = "".join( + f".codeboarding/{name} linguist-generated=true\n" + for name in ( + "analysis.json", + "fingerprint.json", + "static_analysis.pkl", + "static_analysis.sha", + "codeboarding_version.json", + "health/health_report.json", + ) + ) + def _committed_attributes(self, ref: str = "HEAD") -> str: return self._git(self.checkout, "show", f"{ref}:.gitattributes") + "\n" @@ -452,7 +464,7 @@ def test_the_baseline_is_marked_generated_in_the_sync_commit(self) -> None: # language stats, which is what a reviewer wants from a baseline. self._deliver() - self.assertEqual(self._committed_attributes(), ".codeboarding/** linguist-generated=true\n") + self.assertEqual(self._committed_attributes(), self.GENERATED) changed = self._git(self.checkout, "show", "--name-only", "--format=", "HEAD").splitlines() self.assertIn(".gitattributes", changed) self.assertIn(".codeboarding/analysis.json", changed) @@ -462,16 +474,18 @@ def test_existing_attributes_are_kept_and_appended_to(self) -> None: self._deliver() - self.assertEqual( - self._committed_attributes(), "*.png binary\n*.sh text eol=lf\n.codeboarding/** linguist-generated=true\n" - ) + self.assertEqual(self._committed_attributes(), "*.png binary\n*.sh text eol=lf\n" + self.GENERATED) def test_a_line_that_already_covers_it_is_left_alone(self) -> None: self._commit_attributes("/.codeboarding/* linguist-generated\n") self._deliver() - self.assertEqual(self._committed_attributes(), "/.codeboarding/* linguist-generated\n") + # The top-level files are covered already; only the health report is not. + self.assertEqual( + self._committed_attributes(), + "/.codeboarding/* linguist-generated\n.codeboarding/health/health_report.json linguist-generated=true\n", + ) def test_an_explicit_opt_out_is_respected(self) -> None: self._commit_attributes(".codeboarding/** -linguist-generated\n") @@ -504,4 +518,27 @@ def test_the_sync_pull_request_says_what_the_files_are(self) -> None: self.assertIn("analysis cache", create) self.assertIn("start from the saved diagram", create) self._git(self.checkout, "fetch", "--quiet", "origin", "codeboarding/sync") - self.assertEqual(self._committed_attributes("FETCH_HEAD"), ".codeboarding/** linguist-generated=true\n") + self.assertEqual(self._committed_attributes("FETCH_HEAD"), self.GENERATED) + + def test_attributes_alone_never_make_a_commit(self) -> None: + # The baseline is already committed and unchanged; without this the run + # would push an attributes-only commit, or open a sync PR for it each time. + board = self.checkout / ".codeboarding" + for name in ("analysis.json", "fingerprint.json", "static_analysis.pkl"): + (board / name).write_text((self.analysis / name).read_text(), encoding="utf-8") + self._git(self.checkout, "add", "-A") + self._git(self.checkout, "commit", "-m", "baseline") + self._git(self.checkout, "push", "--quiet", "origin", "main") + before = self._git(self.checkout, "rev-parse", "HEAD") + + self._deliver() + + self.assertEqual(self._git(self.remote, "rev-parse", "main"), before) + self.assertFalse((self.checkout / ".gitattributes").exists()) + + def test_files_people_write_are_not_marked_generated(self) -> None: + self._deliver() + + for path in (".codeboarding/.codeboardingignore", ".codeboarding/health/health_config.json"): + attribute = self._git(self.checkout, "check-attr", "linguist-generated", "--", path) + self.assertTrue(attribute.endswith(": unspecified"), attribute)