fix(upload): resolve symlinks in legacy workspace slug + non-blank read count#76
Merged
Merged
Conversation
…on-blank read lines Two follow-up fixes from the PR #75 review: 1. Symlink parity: the live CI hook stamps a session's workspace via config_resolver._slug_from_working_dir(), which resolves symlinks (Path(working_dir).resolve()) before slugifying. The legacy import's derive_workspace() delegated to the hook's abspath-only low-level slugifier instead, so a working_dir traversing a symlink would import data into a DIFFERENT workspace than the hook wrote — latent drift the 0/3051 corpus sweep never hit because none of those sessions crossed a symlink. Align _slugify_path() to resolve first (preserving the empty -string default-fallback case), update the 4 parity tests to the resolved oracle, and add a real-symlink regression test. 2. Reconciliation arithmetic: _count_lines() counted all physical lines including blanks, but the operator summary's `read` field is documented as a non-blank event-line count and the upload loop skips blanks — so `read` could exceed ingested+skipped+unmapped from trailing blank lines. Count only non-blank lines so the summary reconciles by construction. Verified: 342 passed (ground-truth sweep 3286 ws_diffs=0, 400 data-parity 0 defects); ruff + ruff format clean; pyright 0 errors. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two follow-up fixes from PR #75 review:
Symlink parity: The live CI hook stamps a session's workspace via
config_resolver._slug_from_working_dir(), which resolves symlinks before slugifying. The legacy import'sderive_workspace()delegated to the hook's abspath-only low-level slugifier, so a working_dir traversing a symlink would import data into a DIFFERENT workspace than the hook wrote. This latent drift wasn't caught by the 0/3051 corpus sweep because none of those sessions crossed a symlink. Fixed by aligning_slugify_path()to resolve first (preserving the empty-string default-fallback), updating the 4 parity tests to the resolved oracle, and adding a real-symlink regression test.Reconciliation arithmetic:
_count_lines()counted all physical lines including blanks, but the operator summary'sreadfield is documented as non-blank event-line count and the upload loop skips blanks — soreadcould exceed ingested+skipped+unmapped from trailing blank lines. Fixed by counting only non-blank lines so the summary reconciles by construction.Verification
Scope
Not included: dead-code trim of
legacy_transform.py's file-writing half (transform_session,build_metadata,is_content_superset, etc.). These symbols have real production callers in the separateamplifier-ci-migraterepo and must wait for that tool's (separately-tracked) retirement.