refactor: remove deprecated source code linker cache - #803
refactor: remove deprecated source code linker cache#803AlexanderLanin wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new combine-step input resolution can fail with an unhelpful FileNotFoundError when misconfigured, and should provide a clearer, actionable error message.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR removes the deprecated extension-side workspace scanning path for source links and consolidates the source-link generation/parsing flow around the Bazel-generated sourcelinks_json input, simplifying ownership and reducing duplicated logic in the Sphinx extension.
Changes:
- Remove extension-generated
score_source_code_linker_cache.jsongeneration and related live-preview cleanup. - Update the Sphinx extension to consume pre-generated source links via
SCORE_SOURCELINKS/score_sourcelinks_json, and continue gracefully when no input is provided. - Move the source-file parsing helper into
scripts_bazeland update integration tests/docs to match the new flow.
File summaries
| File | Description |
|---|---|
| src/incremental.py | Removes live-preview deletion of the deprecated source-linker cache file. |
| src/extensions/score_source_code_linker/init.py | Drops extension-side scanning/generation and resolves pre-generated source links during combine. |
| src/extensions/score_source_code_linker/generate_source_code_links_json.py | Removes the deprecated extension-side source scanning/generation module. |
| src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py | Updates integration fixture setup to provide pre-generated source links via env var. |
| src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py | Adds a pre-generated source_links.json fixture and wires it into the Sphinx app setup. |
| src/extensions/score_source_code_linker/tests/test_codelink.py | Adjusts unit tests for the new cache/input expectations and adds coverage for “no source links provided”. |
| src/extensions/docs/source_code_linker.md | Updates documentation to reflect the parser move into scripts_bazel. |
| scripts_bazel/source_code_link_parser.py | New shared parser module used by the Bazel source-links generator. |
| scripts_bazel/generate_sourcelinks_cli.py | Switches to the new parser module instead of importing private extension helpers. |
| scripts_bazel/BUILD | Adds the new parser module to the Bazel py_binary sources. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Documentation preview for this pull request is available at: |
There was a problem hiding this comment.
🟡 Changes recommended
Live preview can retain stale derived caches and ignore newly generated source links.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
| "Generating new one" | ||
| ) | ||
| build_and_save_combined_file(app.outdir) | ||
| build_and_save_combined_file(app.outdir, app) |
There was a problem hiding this comment.
We investigated this behavior in detail. The stale-cache observation is technically valid, but it predates this pull request and is unrelated to removing the deprecated source-code-linker cache.
In live preview, incremental.py sets skip_rescanning_via_source_code_linker=1. The existing guards in both setup_combined_linker() and setup_repo_linker() then retain score_scl_grouped_cache.json and score_repo_grouped_scl_cache.json whenever those files already exist. Since _build persists between live-preview runs, changed source-links input can therefore remain unapplied.
Those guards are unchanged on the base branch. PR #803 only removes the extension-side generation/deletion of score_source_code_linker_cache.json; that old file was separate from both derived caches and its deletion never invalidated them. The Bazel-provided SCORE_SOURCELINKS flow was already in place before this PR.
So this is a real cache-lifecycle issue, but not a regression introduced by this cleanup. It would be better handled as a separate follow-up, either by removing the derived JSON handoff entirely and keeping the grouped data in memory, or by adding explicit invalidation for the derived caches.
AlexanderLanin
changed the title
Why
PR #358 introduced the Bazel-generated
sourcelinks_jsoninput and deprecated the extension-generatedscore_source_code_linker_cache.jsonpath. The deprecated path left duplicate source-linking logic in the extension and made it unclear which component owns source scanning. This change simplifies the extension around the current Bazel-generated input.Open question
PR #358 retained the old path for compatibility, although I cannot think of any reason to do so. As its only used internally no-one can notice whether the compatibility layer even exists. So was there a reason? Was there doubt that the bazel way is better? Is it? We can also remove the other way of parsing. But having two parsing paths increases complexity for follow-up work significantly.
What changed
score_source_code_linker_cache.json.scripts_bazel, where its only consumer lives.bazel runandneeds_json.