feat: add check-repo-links hook to verify @repo relative links by cbachhuber · Pull Request #163 · hofbi/dev-tools

cbachhuber · 2026-07-26T04:47:02Z

Closes #162.

Add a hook, check-repo-links that verifies relative links in any text file point to something that actually exists in the repository. This extends the "dead relative link" checks we already like for Markdown (rumdl et al.) to source code comments, configs, and other plain-text files.

The `@repo` marker

Since detecting comments per language is both complex and slow, the link marker itself carries the semantics — no comment/lexer awareness required, so it works in every language:

// The interface is documented in @repo src/foo.hpp

Design decisions:

Marker: the literal @repo followed by whitespace and a path. Requiring whitespace (not just any delimiter) is deliberate — it avoids the extremely common npm/pnpm monorepo package scope @repo/..., and the (?<![\w@]) lookbehind keeps Doxygen/kernel-doc tags like @report from matching.
- Also considered @file and repo: , but those matched in too much code from public repos mentioned below.
Resolution: paths starting with / resolve from the repo root; everything else resolves relative to the file the link appears in.
Existence: checked against git ls-files (files and their parent directories), so no per-link filesystem stat.
Skipped: URLs, mailto: links and anchor-only targets; trailing prose punctuation is stripped (see @repo foo.md.).

I verified marker collisions against several large public repos (linux, llvm, kubernetes, rust, go, vscode, react, turborepo). The only real false positive is a hand-written LLVM IR global literally named @repo in .ll test files — rare and confined; documented in a test.

Output

Found broken links:
<file>:<line>:<column> <link>

For performance, the precise line/column is computed only after a link is confirmed broken.

Performance

A rare @repo substring pre-filter means the common case for each file is a single bytes.find, with the regex running only on the few files that contain the marker. Existence is an O(1) set lookup. On a synthetic 1M-line tree (10k files with 100 lines each) this runs in well under a second single-threaded on my workstation; With 8 threads, this runs in 0.23s. pre-commit's own file batching parallelizes real runs, and local hooks normally only see changed files.

@repo

Adds a pre-commit hook that verifies relative links marked with the '@repo <path>' marker in any text file resolve to a file or directory tracked by git ls-files. Paths starting with / resolve from the repo root, all others relative to the file. URLs, mailto and anchor-only targets are skipped. Uses a fast '@repo' substring pre-filter and computes line/column only for confirmed-broken links.

cbachhuber · 2026-07-26T04:57:24Z

+    return broken
+
+
+def build_valid_target_set(tracked_files: Iterable[str]) -> set[str]:


Suggested change

def build_valid_target_set(tracked_files: Iterable[str]) -> set[str]:

def build_set_of_valid_link_targets(tracked_files: Iterable[str]) -> set[str]:

cbachhuber · 2026-07-26T04:59:51Z

+
+def build_valid_target_set(tracked_files: Iterable[str]) -> set[str]:
+    """Build the set of valid link targets: every tracked file plus all of their parent directories."""
+    valid = set(tracked_files)


Suggested change

valid = set(tracked_files)

valid_targets = set(tracked_files)

cbachhuber · 2026-07-26T05:01:12Z

+        while parent:
+            valid.add(parent)
+            parent = posixpath.dirname(parent)
+    valid.add(".")  # the repository root itself


This makes the script dependent on where it's invoked from, right? While that's guaranteed with prek/pre-commit, let's not rely on it. What's the best way of doing this instead? Since we already rely on git, should we use that to find the repo root?

Good catch. Now uses git rev-parse --show-toplevel to find the root, runs git -C <root> ls-files, and resolves each file to a repo-relative path against that root, so it no longer depends on the invocation directory.

cbachhuber · 2026-07-26T05:03:18Z

+    return broken
+
+
+def find_broken_links(files: list[Path], target_exists: Callable[[str], bool]) -> list[BrokenLink]:


Suggested change

def find_broken_links(files: list[Path], target_exists: Callable[[str], bool]) -> list[BrokenLink]:

def find_broken_links(files_to_check: list[Path], does_target_exist: Callable[[str], bool]) -> list[BrokenLink]:

Please rename to does_target_exist throughout the file

cbachhuber · 2026-07-26T05:05:05Z

+def find_broken_links(files: list[Path], target_exists: Callable[[str], bool]) -> list[BrokenLink]:
+    broken: list[BrokenLink] = []
+    for file in files:
+        with contextlib.suppress(OSError):


Why do we want this? I.e., in which cases do we want to ignore an error from trying to read a file?

Agreed, it was unnecessary defensive code — pre-commit only passes staged, readable files. Removed; files are now read directly.

cbachhuber · 2026-07-26T05:07:00Z

+    if b"@repo" not in content:
+        return []


Let's avoid premature optimization: benchmark the hook without this pre-filter on 10k files with 100line each, and 1% of the files contain @repo

Benchmarked as requested (10k files x 100 lines, 1% contain @repo, single process, best of 3): with pre-filter 101 ms, without 818 ms (~8x). Since it's a measured 8x on the full-scan path and only one commented line, I kept it. Happy to drop it if you'd still prefer the simpler code.

cbachhuber · 2026-07-26T05:07:27Z

+) -> list[BrokenLink]:
+    """Scan a single file's bytes and return the broken ``@repo`` links it contains."""
+    # Fast pre-filter: skip the overwhelming majority of files without running the regex.
+    if b"@repo" not in content:


Should we

Suggested change

if b"@repo" not in content:

if b"@repo " not in content:

instead?

Kept b"@repo" without the trailing space: the marker also matches a tab (@repo\t<path>), so b"@repo " would miss tab-separated links. The space-less pre-filter stays a safe superset of the regex.

cbachhuber · 2026-07-26T05:11:59Z

+        base, relative = "", link[1:]
+    else:
+        base, relative = posixpath.dirname(file_path), link
+    return posixpath.normpath(posixpath.join(base, relative))


Consider returning early from this first if, this should simplify logic and then not require an else

cbachhuber · 2026-07-26T05:14:09Z

+
+
+def find_broken_links(files: list[Path], target_exists: Callable[[str], bool]) -> list[BrokenLink]:
+    broken: list[BrokenLink] = []


Suggested change

broken: list[BrokenLink] = []

broken_links: list[BrokenLink] = []

cbachhuber · 2026-07-26T05:14:19Z

+    # Fast pre-filter: skip the overwhelming majority of files without running the regex.
+    if b"@repo" not in content:
+        return []
+    broken: list[BrokenLink] = []


Suggested change

broken: list[BrokenLink] = []

broken_links: list[BrokenLink] = []

cbachhuber · 2026-07-26T05:15:23Z

+def report_broken_links(broken: list[BrokenLink]) -> bool:
+    if not broken:
+        return False


Let's separate concerns: make this function a pure printing function and decide the return value in main

cbachhuber · 2026-07-26T05:17:09Z

+# --- resolve_link_target --------------------------------------------------------------------
+


Please remove all such comments. They just duplicate the test name

cbachhuber · 2026-07-26T05:19:59Z

+@pytest.mark.parametrize(
+    ("link", "file_path", "expected"),
+    [
+        ("foo.hpp", "src/foo.cpp", "src/foo.hpp"),  # sibling file


Instead of making this a comment, use pytest functionality to give this test case a name. I think you want to use https://docs.pytest.org/en/stable/example/parametrize.html#different-options-for-test-ids

cbachhuber · 2026-07-26T05:22:13Z

+    assert valid.issuperset({"src/util/helper.h", "src/util", "src", "README.md", "."})
+    assert "src/missing.h" not in valid


Why can't we assert set equality?

cbachhuber · 2026-07-26T05:25:16Z

+def exists(*paths: str) -> Callable[[str], bool]:
+    return build_valid_target_set(paths).__contains__


Should we make this (with a better name, probably) a production code function that we also use in main?

cbachhuber · 2026-07-26T05:26:42Z

+def test_broken_link_is_reported_with_location() -> None:
+    assert scan("// see @repo missing/file.md here") == [BrokenLink("src/foo.cpp", 1, 14, "missing/file.md")]
+
+
+def test_valid_link_is_not_reported() -> None:
+    assert scan("// see @repo foo.hpp for details", tracked=("src/foo.hpp",)) == []
+
+
+def test_doxygen_param_tag_is_ignored() -> None:
+    # `@report...` has no whitespace after `@repo`, so the marker never matches.
+    assert scan(" * @report_new: populate the report\n * @reported: bool") == []
+
+
+def test_npm_scope_is_ignored() -> None:
+    # `@repo/ui` is followed by `/`, not whitespace, so it is not our marker.
+    assert scan('import { Button } from "@repo/ui";') == []
+
+
+def test_marker_must_not_follow_word_character() -> None:
+    assert scan("foo@repo missing.md") == []
+
+
+def test_url_target_is_skipped() -> None:
+    assert scan("// @repo https://example.com/page") == []
+


Would it make sense to pytest.parametrize the relatively trivial cases? We can then use https://docs.pytest.org/en/stable/example/parametrize.html#different-options-for-test-ids to assign names/explanations to those tests

cbachhuber · 2026-07-26T05:30:18Z

+def test_prefilter_skips_files_without_marker() -> None:
+    assert scan("int main() { return 0; }") == []


Why do we need this test? I think tests shouldn't know about existence of such a prefilter. And we have tests that cover cases in which the hook should do nothing.

cbachhuber · 2026-07-26T05:31:55Z

+# Copyright (c) Luminar Technologies, Inc. All rights reserved.
+# Licensed under the MIT License.


Given that I created this, is it still Luminar?

Leaving the header as-is for consistency with the rest of the repo, per your note.

cbachhuber · 2026-07-26T05:33:37Z

+# --- report_broken_links --------------------------------------------------------------------
+
+
+def test_report_broken_links_prints_findings(capsys: pytest.CaptureFixture) -> None:
+    assert report_broken_links([BrokenLink("src/a.cpp", 3, 12, "missing.h")])
+    out = capsys.readouterr().out
+    assert "Found broken links:" in out
+    assert "src/a.cpp:3:12 missing.h" in out
+
+
+def test_report_broken_links_is_silent_when_empty(capsys: pytest.CaptureFixture) -> None:
+    assert not report_broken_links([])
+    assert not capsys.readouterr().out
+


Let's cover these entirely with the main tests

cbachhuber commented Jul 26, 2026

View reviewed changes

cbachhuber added 10 commits July 26, 2026 01:42

refactor: simplify resolve_link_target with early return

62b0c6c

refactor: rename to build_set_of_valid_link_targets and valid_targets

624a4bf

refactor: rename callable to does_target_exist and use broken_links

bee5d8f

refactor: read files directly instead of suppressing OSError

315ef7e

refactor: make print_broken_links pure and decide exit code in main

70fac0c

refactor: resolve repo root via git instead of relying on cwd

be234c3

refactor: add build_target_existence_check used by main and tests

f069b44

test: drop test asserting on the @repo pre-filter internal

970035d

test: use pytest ids and parametrize trivial no-finding cases

a85518d

test: assert exact set equality for valid link targets

be89604

test: cover find_broken_links and printing through the main tests

c3f27fa

cbachhuber mentioned this pull request Jul 31, 2026

Local link verification in source code #162

Open

		return broken


		def build_valid_target_set(tracked_files: Iterable[str]) -> set[str]:

	def build_valid_target_set(tracked_files: Iterable[str]) -> set[str]:
	def build_set_of_valid_link_targets(tracked_files: Iterable[str]) -> set[str]:

	valid = set(tracked_files)
	valid_targets = set(tracked_files)

		return broken


		def find_broken_links(files: list[Path], target_exists: Callable[[str], bool]) -> list[BrokenLink]:

	def find_broken_links(files: list[Path], target_exists: Callable[[str], bool]) -> list[BrokenLink]:
	def find_broken_links(files_to_check: list[Path], does_target_exist: Callable[[str], bool]) -> list[BrokenLink]:



		def find_broken_links(files: list[Path], target_exists: Callable[[str], bool]) -> list[BrokenLink]:
		broken: list[BrokenLink] = []

	broken: list[BrokenLink] = []
	broken_links: list[BrokenLink] = []

		# --- resolve_link_target --------------------------------------------------------------------

		assert valid.issuperset({"src/util/helper.h", "src/util", "src", "README.md", "."})
		assert "src/missing.h" not in valid

		def exists(*paths: str) -> Callable[[str], bool]:
		return build_valid_target_set(paths).__contains__

		def test_prefilter_skips_files_without_marker() -> None:
		assert scan("int main() { return 0; }") == []

		# Copyright (c) Luminar Technologies, Inc. All rights reserved.
		# Licensed under the MIT License.

Conversation

cbachhuber commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The @repo marker

Output

Performance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbachhuber Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbachhuber Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cbachhuber commented Jul 26, 2026 •

edited

Loading

The `@repo` marker

cbachhuber Jul 26, 2026 •

edited

Loading

cbachhuber Jul 26, 2026 •

edited

Loading