fix(upload): resolve symlinks in legacy workspace slug + non-blank read count

modules/tool-context-intelligence-upload/amplifier_module_tool_context_intelligence_upload/legacy_transform.py

@@ -177,27 +177,27 @@ def reassemble_event_data(legacy_record: dict[str, Any]) -> tuple[str, dict[str,
 def _slugify_path(path: str) -> str:
     """Convert a working_dir path to the CI workspace slug.
 
-    Delegates directly to the live CI hook's own slugifier,
-    :func:`amplifier_module_hook_context_intelligence.config_resolver._slugify_path`,
-    so migrated data lands in the *exact* same workspace the live hook writes,
-    for every input the hook itself can receive. Reusing the hook's function --
-    rather than re-implementing or partially re-implementing it -- makes
-    divergence structurally impossible: there is a single source of truth for
-    the slug.
-
-    This matters beyond plain POSIX paths: the hook's ``config_resolver``
-    normalises the raw ``session.working_dir`` value (backslash -> hyphen,
-    drive-letter colon stripped, leading-dash guaranteed, empty input falls
-    back to the default project slug) around calling the lower-level
-    ``workspace_slug``. An earlier revision here called bare ``workspace_slug``
-    directly and additionally rejected empty/relative/root input outright --
-    that diverged from the hook on exactly the inputs its own normalisation
-    exists to handle (e.g. a Windows-origin ``working_dir``), silently
-    dropping those sessions instead of placing them where the hook would.
-    There is no separate "correct" derivation to invent here: matching the
-    hook byte-for-byte on every input *is* correct by definition.
+    Mirrors the live CI hook's *default-config* workspace derivation,
+    ``HookConfigResolver._slug_from_working_dir``, which resolves the working
+    directory (symlinks included) before slugifying:
+    ``_hook_slugify_path(str(Path(working_dir).resolve()))``. Matching that
+    branch -- rather than the bare, non-resolving
+    :func:`amplifier_module_hook_context_intelligence.config_resolver._slugify_path`
+    the hook uses internally -- is what makes migrated data land in the
+    *exact* same workspace the live hook writes: the hook always resolves
+    ``session.working_dir`` before deriving a slug from it, so a
+    ``working_dir`` that traverses a symlink must be resolved here too, or
+    the import lands in a different workspace than the hook wrote to.
+
+    The empty-string case is preserved as a special case (rather than being
+    resolved) because ``Path("").resolve()`` returns the current working
+    directory, not "no path" -- resolving it would silently manufacture a
+    bogus slug from the CWD instead of falling through to the hook's
+    documented empty-input fallback (``_DEFAULT_PROJECT_SLUG``).
     """
-    return _hook_slugify_path(path)
+    if not path:
+        return _hook_slugify_path(path)
+    return _hook_slugify_path(str(Path(path).resolve()))
 
 
 def derive_workspace(working_dir: str) -> str:

modules/tool-context-intelligence-upload/amplifier_module_tool_context_intelligence_upload/uploader.py

@@ -99,11 +99,12 @@ def _workspace_from_path(session_dir: Path) -> str:
 
 
 def _count_lines(file_path: Path) -> int:
-    """Count the number of lines in *file_path*."""
+    """Count the number of non-blank lines in *file_path*."""
     count = 0
     with file_path.open(encoding="utf-8") as fh:
-        for _ in fh:
-            count += 1
+        for raw_line in fh:
+            if raw_line.strip():
+                count += 1
     return count
 
 

modules/tool-context-intelligence-upload/tests/test_cli.py

@@ -1192,6 +1192,77 @@ def test_main_reconciliation_read_is_independent_line_count(self, tmp_path, caps
         captured = capsys.readouterr()
         assert "6 read" in captured.err
 
+    def test_main_reconciliation_read_excludes_blank_lines(self, tmp_path, capsys):
+        """read must count non-blank event lines only, so the arithmetic
+        (read == ingested + skipped + unmapped) reconciles exactly even when
+        events.jsonl has interior and trailing blank lines.
+
+        Prior to the fix, _count_lines counted every physical line
+        (including blanks), so `read` could exceed
+        ingested + skipped + unmapped purely from blank/trailing lines --
+        defeating the operator-trust arithmetic this summary exists for.
+        """
+        from amplifier_module_tool_context_intelligence_upload import cli as cli_mod
+        from amplifier_module_tool_context_intelligence_upload.logging_hook_format import (
+            LegacyDiscovery,
+        )
+        from amplifier_module_tool_context_intelligence_upload.uploader import UploadResult
+
+        session_dir = tmp_path / "session1"
+        session_dir.mkdir()
+        # 5 non-blank event lines interleaved with blank/whitespace-only lines
+        # (interior AND trailing) -- these must NOT be counted as "read".
+        (session_dir / "events.jsonl").write_text(
+            '{"line": 0}\n\n{"line": 1}\n{"line": 2}\n   \n{"line": 3}\n{"line": 4}\n\n\n',
+            encoding="utf-8",
+        )
+        metadata = {"session_id": "s1", "format": "logging-hook", "workspace": "ws"}
+        fake_sessions = [(session_dir, metadata)]
+        fake_discovery = LegacyDiscovery(
+            sessions=fake_sessions,
+            candidates_seen=1,
+            live_skipped=0,
+            unresolved_workspace=0,
+        )
+        # ingested + skipped + unmapped == 5, matching the 5 non-blank lines.
+        upload_result = UploadResult(
+            success=True,
+            sessions_uploaded=1,
+            events_uploaded=3,
+            events_skipped=1,
+            events_unmapped=1,
+        )
+
+        with (
+            patch(
+                "sys.argv",
+                [
+                    "context-intelligence-upload",
+                    "--path",
+                    str(tmp_path),
+                    "--server-url",
+                    "http://localhost",
+                    "--api-key",
+                    "key",
+                    "--format",
+                    "logging-hook",
+                ],
+            ),
+            patch.object(cli_mod, "discover_legacy", return_value=fake_discovery),
+            patch.object(cli_mod, "run_upload", return_value=upload_result),
+            patch.object(cli_mod, "ProgressTracker"),
+            pytest.raises(SystemExit),
+        ):
+            cli_mod.main()
+
+        captured = capsys.readouterr()
+        assert "reconciliation:" in captured.err
+        # read == ingested + skipped + unmapped == 3 + 1 + 1 == 5, exactly.
+        assert "5 read" in captured.err
+        assert "3 ingested" in captured.err
+        assert "1 skipped" in captured.err
+        assert "1 unmapped" in captured.err
+
     def test_main_reconciliation_default_format_zero_live_sessions_skipped(self, tmp_path, capsys):
         """The default context-intelligence path has no discovery object, so
         live-sessions-skipped must be 0 without raising."""

modules/tool-context-intelligence-upload/tests/test_legacy_transform.py

@@ -7,6 +7,8 @@
 from __future__ import annotations
 
 import json
+import os
+from pathlib import Path
 
 import pytest
 from amplifier_module_hook_context_intelligence.config_resolver import (
@@ -157,18 +159,30 @@ def test_schema_guard_passes_clean_baseline_silently(capsys: pytest.CaptureFixtu
 
 
 # ---------------------------------------------------------------------------
-# _slugify_path: EXACT parity with the live CI hook's own slugifier,
-# ``amplifier_module_hook_context_intelligence.config_resolver._slugify_path``
-# (which itself delegates to ``context_intelligence.reconstruct.discover.
+# _slugify_path: parity with the live CI hook's *resolve-then-slugify* branch,
+# ``HookConfigResolver._slug_from_working_dir``, which calls
+# ``_hook_slugify_path(str(Path(working_dir).resolve()))``
+# (``amplifier_module_hook_context_intelligence.config_resolver._slugify_path``
+# itself delegates to ``context_intelligence.reconstruct.discover.
 # workspace_slug`` and then applies Windows normalisation + a leading-dash /
-# empty-input fallback). The hook is the ORACLE: legacy_transform must produce
-# byte-identical output to it for every input, including Windows-origin paths
-# and empty input -- there is no separate "correct" derivation, only "matches
-# the hook or doesn't". Ground-truth parity against real hook-written data is
+# empty-input fallback). That resolve-then-slugify branch is the ORACLE:
+# legacy_transform must produce the same output as it for every input --
+# including Windows-origin paths, empty input, and symlinked working_dirs --
+# there is no separate "correct" derivation, only "matches the hook's write-time
+# branch or doesn't". Ground-truth parity against real hook-written data is
 # proven in test_ground_truth_parity.py.
 # ---------------------------------------------------------------------------
 
 
+def _resolved_oracle(raw_path: str) -> str:
+    """Compute the hook's ``_slug_from_working_dir`` oracle for *raw_path*.
+
+    Mirrors ``HookConfigResolver._slug_from_working_dir`` exactly: resolve
+    symlinks/relative components, then slugify via the hook's own slugifier.
+    """
+    return _hook_slugify_path(str(Path(raw_path).resolve()))
+
+
 def test_slugify_absolute_posix_unchanged() -> None:
     assert _slugify_path("/Users/me/project") == "-Users-me-project"
 
@@ -193,11 +207,12 @@ def test_slug_matches_hook_for_windows_path() -> None:
     and drive-letter colons; the old ``legacy_transform._slugify_path``
     instead rejected any non-POSIX-absolute path outright, so migrated
     Windows sessions were silently skipped instead of landing where the hook
-    would put them. This asserts exact parity, using the hook function itself
-    as the oracle (never a hardcoded expected string).
+    would put them. This asserts parity against the resolve-then-slugify
+    oracle (the hook's actual write-time branch), using the hook function
+    itself as the oracle (never a hardcoded expected string).
     """
     windows_path = "C:\\Users\\me\\project"
-    assert derive_workspace(windows_path) == _hook_slugify_path(windows_path)
+    assert derive_workspace(windows_path) == _resolved_oracle(windows_path)
 
 
 @pytest.mark.parametrize(
@@ -210,29 +225,62 @@ def test_slug_matches_hook_for_windows_path() -> None:
     ],
 )
 def test_slug_matches_hook_across_inputs(raw_path: str) -> None:
-    """derive_workspace must equal the hook's own slugifier for every input.
+    """derive_workspace must equal the resolve-then-slugify oracle for every input.
 
-    The hook function is the oracle -- expected values are never hardcoded,
-    only compared against ``config_resolver._slugify_path`` directly, so this
-    test cannot pass by coincidence and cannot drift from the hook.
+    The oracle is the hook's actual write-time branch
+    (``HookConfigResolver._slug_from_working_dir``) -- expected values are
+    never hardcoded, only computed via ``Path(x).resolve()`` +
+    ``config_resolver._slugify_path`` directly, so this test cannot pass by
+    coincidence and cannot drift from the hook.
     """
-    assert derive_workspace(raw_path) == _hook_slugify_path(raw_path)
+    assert derive_workspace(raw_path) == _resolved_oracle(raw_path)
 
 
 def test_slugify_root_matches_hook() -> None:
     # The hook does NOT reject a root path -- workspace_slug("/") == "-", and
     # the hook returns it as-is (truthy, already dash-prefixed). No raise.
-    assert _slugify_path("/") == _hook_slugify_path("/")
+    # "/" resolves to itself, so the resolved oracle is unaffected here.
+    assert _slugify_path("/") == _resolved_oracle("/")
 
 
 def test_derive_workspace_empty_matches_hook_default() -> None:
     # The hook returns its default project slug for empty input rather than
-    # failing -- match that exactly instead of raising.
+    # failing -- match that exactly instead of raising. Empty input is the
+    # one case NOT run through Path(...).resolve() (see _slugify_path
+    # docstring), so the oracle here stays bare (unresolved).
     assert derive_workspace("") == _hook_slugify_path("")
 
 
 def test_derive_workspace_relative_matches_hook() -> None:
-    # The hook resolves relative paths via os.path.abspath (cwd-dependent) --
-    # it does not reject them. Compare against the oracle rather than a
-    # hardcoded string so the test is correct regardless of cwd.
-    assert derive_workspace("me/project") == _hook_slugify_path("me/project")
+    # The hook resolves relative paths (Path(...).resolve(), cwd-dependent)
+    # before slugifying -- it does not reject them. Compare against the
+    # resolved oracle rather than a hardcoded string so the test is correct
+    # regardless of cwd.
+    assert derive_workspace("me/project") == _resolved_oracle("me/project")
+
+
+def test_derive_workspace_resolves_symlinked_working_dir(tmp_path: Path) -> None:
+    """A working_dir that traverses a symlink must land in the SAME workspace
+    the live hook would write to.
+
+    The hook's ``_slug_from_working_dir`` always resolves ``session.working_dir``
+    (``Path(working_dir).resolve()``) before slugifying, so a working_dir that
+    is -- or traverses -- a symlink must resolve to its target here too. This
+    is the regression this fix exists to close: prior to the fix,
+    ``derive_workspace`` matched the hook's *bare* (non-resolving) slugifier,
+    so a symlinked working_dir would import into a different workspace than
+    the one the live hook actually wrote to.
+    """
+    target_dir = tmp_path / "real_project"
+    target_dir.mkdir()
+    link_path = tmp_path / "linked_project"
+    try:
+        os.symlink(target_dir, link_path)
+    except (OSError, NotImplementedError):
+        pytest.skip("OS does not support creating symlinks in this environment")
+
+    expected = _resolved_oracle(str(link_path))
+    assert derive_workspace(str(link_path)) == expected
+    # Sanity: the expected value really is derived from the RESOLVED target,
+    # not the symlink path itself (proves the test would fail pre-fix).
+    assert expected == _hook_slugify_path(str(target_dir.resolve()))

modules/tool-context-intelligence-upload/tests/test_uploader.py

@@ -12,6 +12,7 @@
 
 from amplifier_module_tool_context_intelligence_upload.uploader import (
     UploadResult,
+    _count_lines,
     run_upload,
 )
 
@@ -726,3 +727,45 @@ def test_500_error_message_includes_server_body(self, tmp_path: Path) -> None:
         assert result.error is not None
         assert "500" in result.error
         assert "Internal Server Error" in result.error
+
+
+# ---------------------------------------------------------------------------
+# TestCountLines
+# ---------------------------------------------------------------------------
+
+
+class TestCountLines:
+    """Tests for _count_lines — must count non-blank lines only.
+
+    The upload loop (run_upload) skips blank/whitespace-only lines when
+    processing events.jsonl (``if not line: continue``), so _count_lines must
+    apply the identical blank-check or the reconciliation arithmetic
+    (read == ingested + skipped + unmapped) breaks whenever a file has blank
+    interior or trailing lines.
+    """
+
+    def test_counts_non_blank_lines_only(self, tmp_path: Path) -> None:
+        """Interior and trailing blank lines are excluded from the count."""
+        events_file = tmp_path / "events.jsonl"
+        # 3 non-blank lines, with a blank line between two of them and two
+        # trailing blank lines (whitespace-only counts as blank too).
+        events_file.write_text(
+            "line-one\n\nline-two\nline-three\n\n   \n",
+            encoding="utf-8",
+        )
+
+        assert _count_lines(events_file) == 3
+
+    def test_counts_zero_for_all_blank_file(self, tmp_path: Path) -> None:
+        """A file containing only blank/whitespace lines counts as zero."""
+        events_file = tmp_path / "events.jsonl"
+        events_file.write_text("\n\n   \n\t\n", encoding="utf-8")
+
+        assert _count_lines(events_file) == 0
+
+    def test_counts_all_lines_when_none_are_blank(self, tmp_path: Path) -> None:
+        """A file with no blank lines counts every line (baseline, no regression)."""
+        events_file = tmp_path / "events.jsonl"
+        events_file.write_text("a\nb\nc\n", encoding="utf-8")
+
+        assert _count_lines(events_file) == 3