Invalidate persistent file cache when Finch source changes by Copilot · Pull Request #358 · finch-tensor/finch-tensor-lite

Copilot · 2026-04-02T20:45:59Z

Review new actionable comments from Mateusz
Update cache source root detection to use finchlite.__path__[0]
Refactor cache e2e test to avoid wrapping the whole test body in try/finally
Update repo-root resolution in test to avoid parents[1]
Run targeted tests for cache and codegen behavior
Reply to actionable new comments with commit hash
Run final validation

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/2345968a-d46d-4bdf-8342-58ff0eaf3afe Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

willow-ahrens · 2026-04-02T22:23:28Z

+_finch_source_root = Path(__file__).resolve().parent.parent
+
+
+@functools.cache


set this on load, don't use functool cache

Updated in 197d5a9: removed functools caching and now compute Finch source mtime once at module load (_session_finch_code_mtime_ns).

willow-ahrens · 2026-04-02T22:24:10Z

+    return latest_mtime
+
+
+def _clear_cache_root(cache_root: Path) -> None:


Make a user-accessible version of this and add a nice docstring, since users may want to use it. make it so the user can just say "clear cache" and it clears the current cache root for the current finch version

Implemented in 197d5a9: added user-facing clear_cache() with a docstring, which clears the current Finch version cache root.

willow-ahrens · 2026-04-02T22:29:14Z

+            path.unlink()
+
+
+def _ensure_cache_fresh(cache_root: Path) -> None:


There will only ever be one cache root for a given session. Please simplify accordingly.

Done in 197d5a9: simplified freshness tracking to a single session-level _cache_checked flag, assuming one cache root per session.

I'd prefer to put the if _cache_checked in the cache decorator

Implemented in 9879952: moved the _cache_checked guard into the file_cache decorator and removed that guard from _ensure_cache_fresh. No UI changes (backend-only), so there is no screenshot.

Implemented in 9879952: _cache_checked is now checked in the file_cache decorator before calling _ensure_cache_fresh. No UI changes (backend-only), so there is no screenshot.

willow-ahrens · 2026-04-02T22:33:23Z

+
+def test_ensure_cache_fresh_clears_cache_when_code_changes(tmp_path, monkeypatch):


Instead, do an end-to-end test that invokes multiple python sessions and modifies a finch file.

Addressed in 197d5a9: replaced helper-level tests with an end-to-end test that runs separate Python sessions and modifies a Finch source file mtime to verify invalidation.

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/23dea4a8-d9c1-4f24-aa53-456a48a281ad Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

willow-ahrens · 2026-04-02T23:08:35Z

+            path.unlink()
+
+
+def _ensure_cache_fresh(cache_root: Path) -> None:


I'd prefer to put the if _cache_checked in the cache decorator

willow-ahrens · 2026-04-02T23:09:42Z

@@ -0,0 +1,57 @@
+# AI modified: 2025-01-01T00:00:00Z parent=4f5a2e5021678965ce8d830bb9edecac1dd3fea9


I need an end-to-end test that runs the c_codegen, changes something about c codegen, and then runs it again in a new session and verifies the cache was invalidated and the new output is used.

Implemented in 9879952: replaced the cache test with a c_codegen end-to-end multi-session test that runs codegen, modifies c_codegen.py behavior, runs again in a new session, and verifies the new output is used (7 then 9). No UI changes (backend-only), so there is no screenshot.

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/9922c86e-7a2f-4fa5-87d3-23689718ec19 Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

mtsokol · 2026-04-03T09:42:29Z

+# util/cache.py lives in src/finchlite/util/, so parent.parent is src/finchlite.
+_finch_source_root = Path(__file__).resolve().parent.parent


Well, that might change in the future (placement of this file), which will surprise the person working on it. I believe that:

import finchlite _finch_source_root = finchlite.__path__[0]

Is more bulletproof and we don't hardcode this awful .parent.parent thing.

Implemented in 37d451f: switched Finch source root detection to Path(finchlite.__path__[0]) so it no longer depends on Path(__file__).resolve().parent.parent. No UI changes (backend-only), so screenshot is not applicable.

mtsokol · 2026-04-03T09:45:06Z

+print("RESULT", int(lib.unique_value()))
+"""
+
+    try:


We shouldn't have a whole test in a try block - let's use try for places where we actually expect it to happen and have a recovery in except there (or propagate if that's the case) - the finally contents can be a function that we can reuse.

Implemented in 37d451f: narrowed try/finally to just the part that mutates c_codegen.py and extracted cleanup into _restore_file_state(...) for reuse. No UI changes (backend-only), so screenshot is not applicable.

mtsokol · 2026-04-03T09:47:49Z

+
+
+def test_c_codegen_cache_invalidation_end_to_end_across_sessions(tmp_path):
+    repo_root = Path(__file__).resolve().parents[1]


Why .parents[1] instead of .parent? We can access the repo root same way as I described before.

Implemented in 37d451f: replaced Path(__file__).resolve().parents[1] with Path(finchlite.__path__[0]).parent.parent and added an inline note explaining the path derivation. No UI changes (backend-only), so screenshot is not applicable.

willow-ahrens · 2026-04-03T14:57:31Z

@mtsokol thanks for your help with this! I'm not sure if it's easier to use AI or to just write it myself. Do you think we should actually try to merge this code?

mtsokol · 2026-04-03T15:12:28Z

@mtsokol thanks for your help with this! I'm not sure if it's easier to use AI or to just write it myself. Do you think we should actually try to merge this code?

I think searching for outdated cache files won't get much prettier than this - still, let's make it more thoughtful then it comes to edge cases, so we won't get surprised in a few months. I can iron it out after Monday next week - if you don't need it earlier.

Have you considered setting up Claude Code for a reviewer assistant instead of Copilot? Drawing on my experience Claude was way more "intelligent" with finch lite code: https://code.claude.com/docs/en/github-actions but that requires Claude Pro subscription.

willow-ahrens

Address mateusz's comments

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/50f6f452-e603-4120-a642-04c3a19fddfb Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

Copilot · 2026-04-03T15:45:00Z

Address mateusz's comments

Addressed in 37d451f: applied Mateusz’s requested updates in cache root discovery and the cache e2e test structure/path handling. No UI changes (backend-only), so screenshot is not applicable.

Initial plan

154b5ae

Copilot AI assigned Copilot and willow-ahrens Apr 2, 2026

Copilot started work on behalf of willow-ahrens April 2, 2026 20:46 View session

Copilot AI linked an issue Apr 2, 2026 that may be closed by this pull request

invalidate finch cache automatically when finch code changes #53

Open

Copilot AI and others added 2 commits April 2, 2026 20:49

feat: invalidate cache when finch source changes

06953a7

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/2345968a-d46d-4bdf-8342-58ff0eaf3afe Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

chore: refine cache invalidation robustness and tests

4f5a2e5

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/2345968a-d46d-4bdf-8342-58ff0eaf3afe Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add automatic cache invalidation for Finch on code changes~~ Invalidate persistent file cache when Finch source changes Apr 2, 2026

Copilot finished work on behalf of willow-ahrens April 2, 2026 20:56

Copilot AI requested a review from willow-ahrens April 2, 2026 20:56

willow-ahrens requested changes Apr 2, 2026

View reviewed changes

Copilot started work on behalf of willow-ahrens April 2, 2026 22:33 View session

refactor: simplify cache freshness and add clear_cache API

197d5a9

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/23dea4a8-d9c1-4f24-aa53-456a48a281ad Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

Copilot finished work on behalf of willow-ahrens April 2, 2026 22:44

Copilot AI requested a review from willow-ahrens April 2, 2026 22:44

willow-ahrens requested changes Apr 2, 2026

View reviewed changes

Copilot started work on behalf of willow-ahrens April 2, 2026 23:09 View session

test: add c_codegen e2e cache invalidation scenario

9879952

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/9922c86e-7a2f-4fa5-87d3-23689718ec19 Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

Copilot AI requested a review from willow-ahrens April 2, 2026 23:19

Copilot finished work on behalf of willow-ahrens April 2, 2026 23:19

mtsokol reviewed Apr 3, 2026

View reviewed changes

Merge branch 'main' into copilot/invalidate-finch-cache-automatically

36276c2

willow-ahrens requested changes Apr 3, 2026

View reviewed changes

Copilot started work on behalf of willow-ahrens April 3, 2026 15:36 View session

refactor: polish cache root lookup and test structure

37d451f

Agent-Logs-Url: https://github.com/finch-tensor/finch-tensor-lite/sessions/50f6f452-e603-4120-a642-04c3a19fddfb Co-authored-by: willow-ahrens <5770255+willow-ahrens@users.noreply.github.com>

Copilot finished work on behalf of willow-ahrens April 3, 2026 15:45

Copilot AI requested a review from willow-ahrens April 3, 2026 15:45

		_finch_source_root = Path(__file__).resolve().parent.parent


		@functools.cache

		return latest_mtime


		def _clear_cache_root(cache_root: Path) -> None:

		path.unlink()


		def _ensure_cache_fresh(cache_root: Path) -> None:


		def test_ensure_cache_fresh_clears_cache_when_code_changes(tmp_path, monkeypatch):

		@@ -0,0 +1,57 @@
		# AI modified: 2025-01-01T00:00:00Z parent=4f5a2e5021678965ce8d830bb9edecac1dd3fea9

		# util/cache.py lives in src/finchlite/util/, so parent.parent is src/finchlite.
		_finch_source_root = Path(__file__).resolve().parent.parent



		def test_c_codegen_cache_invalidation_end_to_end_across_sessions(tmp_path):
		repo_root = Path(__file__).resolve().parents[1]

Uh oh!

Conversation

Copilot AI commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

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!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

willow-ahrens commented Apr 3, 2026

Uh oh!

mtsokol commented Apr 3, 2026

Uh oh!

willow-ahrens left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 2, 2026 •

edited

Loading