Two follow-ups from the review of #985, filed together because they are the same shape: correct by construction, guarded on a subset. Neither is a live defect. Both are cheap.
1. The reporting witness covers one inference path of two
_resolve_project_dir (src/synapt/recall/archive.py:47) derives the workspace root from project_data_dir(None), so the archive verbs ride every resolution path the rest of recall rides: the SYNAPT_RECALL_ROOT override, git-worktree inference, and gripspace inference.
test_export_reports_the_data_dir_it_read_not_the_path_it_was_given binds the git-worktree path with a real repo and a linked worktree, including a liveness control so an inert fixture fails rather than passes. Nothing binds the gripspace path.
Both are correct today, and correct for the same reason: they flow through one resolver, so a regression in the shared seam would fail the existing witness. The gap is narrower than it sounds and is worth closing anyway, because the two paths are separate functions with separate failure modes, and "the other one is covered" is exactly the reasoning that leaves a path unguarded until it is not.
Measured during review, with a control asserting inference was live before anything was read: --path at a sub-repo of a gripspace resolves the store to the gripspace root, not to the sub-repo.
Suggested fix: a sibling of the existing witness, fixture built from a gripspace marker rather than a linked worktree.
2. source_project is derived from the caller's argument
_archive_manifest records:
"data_dir": str(data_dir),
"source_project": project_dir.name,(src/synapt/recall/archive.py:193-194, quoted verbatim)
data_dir is the resolved directory, recorded after inference. source_project is the name of whatever the caller passed in.
data_dir was corrected in #985 precisely because recording the argument let an inference-redirected --path report a store it had not read. source_project was not part of that change and still names the argument.
Measured: --path at a sub-repo of a gripspace yields source_project: "subrepo" while data_dir correctly names the gripspace store. The two fields describe different workspaces, one line apart.
This predates #985 and is not the field the reporting contract depends on, so nothing reads it today in a way that misleads. It is filed because a field whose neighbour was just fixed for being argument-derived is the most likely place for the same confusion to reappear, and because a reader comparing the two fields in a manifest has no way to tell which one to believe.
Suggested fix: derive it from the resolved root, or drop it in favour of the field that is already correct. Whichever, it wants a witness that would fail if it went back to the argument.
Why one issue rather than two
Both were raised as non-blocking notes on #985 and neither blocks anything. Splitting them would put two low-priority issues on the board describing one class. Closing this issue means both have a witness that fails when the behaviour regresses.
Two follow-ups from the review of #985, filed together because they are the same shape: correct by construction, guarded on a subset. Neither is a live defect. Both are cheap.
1. The reporting witness covers one inference path of two
_resolve_project_dir(src/synapt/recall/archive.py:47) derives the workspace root fromproject_data_dir(None), so the archive verbs ride every resolution path the rest of recall rides: theSYNAPT_RECALL_ROOToverride, git-worktree inference, and gripspace inference.test_export_reports_the_data_dir_it_read_not_the_path_it_was_givenbinds the git-worktree path with a real repo and a linked worktree, including a liveness control so an inert fixture fails rather than passes. Nothing binds the gripspace path.Both are correct today, and correct for the same reason: they flow through one resolver, so a regression in the shared seam would fail the existing witness. The gap is narrower than it sounds and is worth closing anyway, because the two paths are separate functions with separate failure modes, and "the other one is covered" is exactly the reasoning that leaves a path unguarded until it is not.
Measured during review, with a control asserting inference was live before anything was read:
--pathat a sub-repo of a gripspace resolves the store to the gripspace root, not to the sub-repo.Suggested fix: a sibling of the existing witness, fixture built from a gripspace marker rather than a linked worktree.
2.
source_projectis derived from the caller's argument_archive_manifestrecords:(
src/synapt/recall/archive.py:193-194, quoted verbatim)data_diris the resolved directory, recorded after inference.source_projectis the name of whatever the caller passed in.data_dirwas corrected in #985 precisely because recording the argument let an inference-redirected--pathreport a store it had not read.source_projectwas not part of that change and still names the argument.Measured:
--pathat a sub-repo of a gripspace yieldssource_project: "subrepo"whiledata_dircorrectly names the gripspace store. The two fields describe different workspaces, one line apart.This predates #985 and is not the field the reporting contract depends on, so nothing reads it today in a way that misleads. It is filed because a field whose neighbour was just fixed for being argument-derived is the most likely place for the same confusion to reappear, and because a reader comparing the two fields in a manifest has no way to tell which one to believe.
Suggested fix: derive it from the resolved root, or drop it in favour of the field that is already correct. Whichever, it wants a witness that would fail if it went back to the argument.
Why one issue rather than two
Both were raised as non-blocking notes on #985 and neither blocks anything. Splitting them would put two low-priority issues on the board describing one class. Closing this issue means both have a witness that fails when the behaviour regresses.