Skip to content

fix(sdk): keep partial suffix lines out of coarse blame - #468

Merged
yifanxuaaa merged 1 commit into
Ephemeral-AI-Lab:mainfrom
Jackmeson1:fix/coarse-blame-suffix
Oct 27, 2026
Merged

yifanxuaaa merged 1 commit into
Ephemeral-AI-Lab:mainfrom
Jackmeson1:fix/coarse-blame-suffix

Conversation

@Jackmeson1

Copy link
Copy Markdown
Contributor

Splitting alpha beta into alpha and beta in a 2,005-line file made coarse fs.blame credit beta to the base Layer, or to an earlier Commit that wrote alpha beta. This change leaves that partial suffix line with the Commit that split it.

Cause and change

At caebd268d, engine/crates/layerfs-store-service/src/read/blame.rs:68–106 computes common byte edges. Its coarse branch at lines 209–217 maps a newer suffix line to new_line + old.lines - new.lines without checking the corresponding older line boundary.

In the reproduction, the prefix is five bytes and the suffix starts at byte 6 in both versions. That byte starts beta\n in the newer version but follows a space in the older version. New line 2 is consequently mapped to old line 1, alpha beta.

equal_edges() now checks whether the older suffix starts at byte zero or immediately after LF. If neither holds, it reduces the suffix length by one byte. That excludes a newer line starting exactly at the original suffix boundary. Later lines remain eligible because their preceding LF is inside the equal suffix in both versions.

The suffix remains byte-based; it is not rounded to a line boundary. The added check makes at most one logical one-byte range request per changed coarse comparison. Underlying content acquisition can read or decode more than one byte. No additional full-file line vector is introduced, and long-history performance was not measured.

api/docs/agent.md and core/docs/architecture/82-sdk-call-channel.md document the suffix-line eligibility rule.

A separate prefix-at-EOF defect remains: a newer unterminated last line that is a shortened older line can be accepted by the prefix branch. Its demonstrated probes and extension are reserved for a separate follow-up.

Validation

I ran the following outside the sandbox on a Colab CPU runtime: Ubuntu 24.04.5, Linux 6.6.159+ x86_64, overlayfs, Rust/Cargo 1.85.1 and cargo-nextest 0.9.150, with LAYERFS_CONSTRUCTION_WORKERS=1.

The existing Rust fixtures explicitly select Disposable persistence and invoke layerfs_api::call. Their bootstrap bypasses normal filesystem admission. These results cover the Rust API behavior, not native-addon, worker, FUSE, or supported-filesystem installation.

The new engine/crates/layerfs-api/tests/sdk_blame_coarse.rs contains eleven tests. It checks every final line's text and origin across bounded pagination.

  • Six fail on caebd268d and pass with the fix: splits at the first, middle and last line; a last-line split without final LF; xalpha cut to alpha; and a split of a line written by an earlier Commit.
  • Five controls pass on both: joining lines, adding/dropping the first line, preserving an unterminated last line, adding final LF, and the same split in a 10-line file.

Commands:

export LAYERFS_CONSTRUCTION_WORKERS=1
cargo +1.85.1 test --locked -p layerfs-api --test sdk_blame_coarse --no-run
timeout 120 cargo +1.85.1 test --locked -p layerfs-api --test sdk_blame_coarse -- --test-threads=1

filter="$(python3 -B tools/ci_tests.py filter fast)"
cargo +1.85.1 nextest run --locked -p layerfs-store-service -p layerfs-api --no-run --profile ci -E "$filter"
timeout 120 cargo +1.85.1 nextest run --locked -p layerfs-store-service -p layerfs-api --profile ci -E "$filter" --no-fail-fast
Check caebd268d plus regression tests With the suffix fix
sdk_blame_coarse 5 passed, 6 failed 11 passed
Standalone issue reproduction 1 failed 1 passed
Saved prop07 check 1 failed 1 passed
sdk_method_golden and method_refusal_order 2 passed 2 passed
Two-crate fast nextest selection 112 passed, 7 failed, 6 skipped 118 passed, 1 failed, 6 skipped

The remaining failure is layerfs-api::local_install import_installs_the_store_in_process. Both runs refuse the Colab overlayfs destination because direct Store access requires ext4, xfs or btrfs. Neither fast-lane run is wholly green.

A throwaway probe, not included in this PR, splits lines at the beginning, middle and end of approximately 1.85-MB files containing about 30,000 lines. Its three cases fail on base and pass with the fix. These files exceed both comparison bounds; this does not isolate the byte-only threshold.

Additional recorded checks:

  • cargo +1.85.1 fmt --all -- --check: passed.
  • cargo +1.85.1 clippy --locked -p layerfs-store-service -p layerfs-api --all-targets -- -D warnings: passed.
  • python3 -B tools/check_product_boundary.py --skip-sdk: passed on clean copies of base and fixed trees.
  • python3 -B tools/check_product_boundary.py: passed on a clean fixed-tree copy.
  • python3 -B -m unittest discover -s tools -p 'test_*.py': 155 tests passed. This recorded invocation lacked an explicit wall timeout.

The boundary guard also produced two path-dependent violations in unchanged files when the checkout was under /root/lfs/src; the clean-copy results above preserve that distinction.

Slow/nightly and mount selections, isolated byte-limit coverage, CRLF/UTF-8-specific cases, and long-history performance were not run. Durable: NOT_RUN — disabled by owner until explicit reauthorization.

No matching coarse-suffix report was found in the saved issue/PR search results. That search is a point-in-time check.

Production LOC: 102,200 -> 102,206 (delta +6)
engine 61,313 -> 61,319 (+6): layerfs-store-service 10,032 -> 10,038 (+6);
core 38,241 and api 2,646 unchanged.
Reference absent (retired R9c; last counted 65,417 at tree 498dd1917812).
Method: tools/production_loc.py
(blob 3341e4d0362c822b6d9a08419c432942b3a13800)
--root --json, using the caebd26 archive and that archive
with this change applied. The documentation wording edits
do not change production source. Scope includes first-party production
Rust, runtime SQL and API JavaScript; excludes tests, inline test code,
documentation, tooling, dependencies and generated artifacts.

No tests deleted.

Fixes #467

AI disclosure: this was prepared with AI assistance (Claude Opus 5.5).

🤖 Generated with Claude Code

https://claude.ai/code/session_019TTGAv6fe859ebbCLLfLHP

When changed file versions exceed blame's 2,000-line or 48-KiB comparison
bounds, the coarse path maps suffix lines by the total line-count
difference. It did not check that the corresponding older suffix byte
started a line. Splitting alpha beta into alpha and beta could therefore
credit beta to the base Layer or an earlier Commit containing alpha beta.

Check the older suffix boundary in equal_edges. If it is neither byte
zero nor immediately after LF, reduce the suffix length by one byte.
This excludes a newer line starting exactly at the original boundary;
later suffix lines retain matching preceding LF bytes in both versions.

The check adds at most one logical one-byte range request per changed
coarse comparison, with no additional full-file line vector. Underlying
object reads can be larger. Update agent.md and 82-sdk-call-channel.md
to describe inherited suffix lines, rather than a line-aligned suffix.

Keep the separately demonstrated prefix-at-EOF defect for a follow-up.

Outside-sandbox validation I ran on a Colab CPU runtime, Ubuntu 24.04.5,
Linux 6.6.159+ x86_64/overlayfs, Rust 1.85.1, nextest 0.9.150:
- sdk_blame_coarse: base 5 passed/6 failed; fixed 11 passed.
- Standalone split reproduction and saved prop07: fail before, pass after.
- sdk_method_golden and method_refusal_order: pass on both.
- Two-crate fast nextest: base 112 passed/7 failed; fixed 118 passed/1
  failed; 6 skipped on each. local_install refuses overlayfs on both.
- Scoped warning-denying Clippy and formatting pass; boundary guards
  pass on clean copies. The path-dependent guard failure is retained.
- Tool self-tests: 155 passed; that invocation lacked a wall timeout.

The Rust fixtures use Disposable Stores and bypass normal filesystem
admission. Slow/mount, native-addon/worker and long-history performance
coverage were not run. Durable remains disabled by owner. No tests deleted.

Production LOC: 102,200 -> 102,206 (delta +6)
  engine 61,313 -> 61,319 (+6): layerfs-store-service 10,032 -> 10,038 (+6);
  core 38,241 and api 2,646 unchanged.
  Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
  Method: tools/production_loc.py
  (blob 3341e4d)
  --root <snapshot> --json on the caebd26 archive and that archive
  with this change applied. Documentation wording edits
  leave production source unchanged. Counts include first-party Rust,
  runtime SQL and API JavaScript, excluding tests and inline test code,
  docs, tooling, dependencies and generated artifacts.

AI disclosure: this was prepared with AI assistance (Claude Opus 5.5).

Signed-off-by: Jackmeson1 <130693642+Jackmeson1@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019TTGAv6fe859ebbCLLfLHP
@yifanxuaaa
yifanxuaaa merged commit d0419f6 into Ephemeral-AI-Lab:main Oct 27, 2026
3 checks passed
yifanxuaaa pushed a commit to Jackmeson1/layerfs that referenced this pull request Oct 27, 2026
Follow-up to Ephemeral-AI-Lab#468. When the newer version of a changed file is a byte
prefix of the older one and ends in an unterminated line, the coarse
comparison's common prefix covers that whole line, so blame credited it
to the older line it was cut from: cutting `alpha beta` to `alpha` at
EOF, or dropping the final newline, kept the older Commit.

A line ending at the common prefix is the same line only if it ends
with LF (inside the prefix, equal in both) or both versions end there.
Lines ending before the prefix end always end with LF. So when the
prefix is the whole newer text, the older text is longer and the newer
text does not end with LF, equal_edges gives up the prefix's last byte
and the line becomes changed text. The suffix is empty in that case,
so the two edges still cannot overlap. At most one more one-byte range
read per changed coarse comparison.

Tests (sdk_blame_coarse): a last line cut short, with and without
a final LF in the older text, and a dropped final newline fail before
and pass after; dropping whole last lines keeps every origin (control).
api/docs/agent.md and 82-sdk-call-channel.md state the prefix rule.

Checks (macOS arm64, Rust 1.85.1): layerfs-store-service and
layerfs-api tests 145 passed in 63 binaries under a 120 s wall limit;
scoped warning-denying Clippy, fmt, check_product_boundary.py
--skip-sdk and git diff --check pass. Linux, mount and nightly lanes
not run. Durable: NOT_RUN - disabled by owner until explicit
reauthorization. No tests deleted.

Production LOC: 102,266 -> 102,269 (delta +3)
  engine 61,365 -> 61,368 (+3): layerfs-store-service 10,060 -> 10,063;
  core 38,255 and api 2,646 unchanged.
  Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
  Method: tools/production_loc.py (blob 3341e4d) --root <snapshot>
  --json on archives of the parent d0419f6 and the staged tree.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
jiangkoumo added a commit to jiangkoumo/layerfs that referenced this pull request Oct 28, 2026
…-AI-Lab#427)

Brings main up to 582bc78 (Ephemeral-AI-Lab#488, Ephemeral-AI-Lab#481 forced unmount after a stalled
Join, Ephemeral-AI-Lab#479, Ephemeral-AI-Lab#471, Ephemeral-AI-Lab#473, Ephemeral-AI-Lab#445 fs modes, Ephemeral-AI-Lab#475, Ephemeral-AI-Lab#478, Ephemeral-AI-Lab#476 Docker API
negotiation, Ephemeral-AI-Lab#396, Ephemeral-AI-Lab#419, Ephemeral-AI-Lab#418, Ephemeral-AI-Lab#415, Ephemeral-AI-Lab#477, Ephemeral-AI-Lab#468).

Two conflicts, both from Ephemeral-AI-Lab#481 (forced unmount of a normal unmount stalled
at Join):
- engine/crates/layerfs-daemon/src/control/force.rs: guard() keeps this
  branch's ExportKept check (an NFS export kept after a failed teardown
  answers with its stored custody first) and takes main's comment and its
  forceable() exception for a retained FUSE session. The non-Linux
  `let _ = token;` stays removed: ExportKept uses the token on every
  platform.
- engine/crates/layerfs-node/src/workspace.rs: unmount() keeps this
  branch's per-Workspace unmount lease (repeat a published outcome, drop
  the admission guard before the daemon call, publish the outcome before
  cleanup) and takes main's `force` option inside it: forced, one
  api.unmount(token, true) whose None answer is Unmounted::Closed;
  otherwise unmount_checked with `discard` as before.

Production LOC: first-parent 9478190 108596 -> 109158
(tools/production_loc.py --root .; the delta is main's own changes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.blame credits a split line to an older version in coarse comparisons

2 participants