Skip to content

Duplicated organize and checksum implementations, with mismatched xxhash/xxhash64 labels #120

Description

@dspury

What

docs/FILE-FERRY-PRODUCT-DIRECTION.md §8 is explicit:

The existing capability modules—probe, organize, proxy, verify, Resolve, and logging—should remain the source of domain behavior. Do not reimplement them in the desktop frontend.

The vNext application layer imports nothing from the legacy capability modules except proxy. Verified by grepping application/, persistence/ and service/ for file_ferry.organize / verify / probe / log: zero hits.

Two concrete duplications:

1. Organize. application/organize.py (144 lines) implements its own copy/move/link and collision detection, entirely separate from organize.py (398 lines). Only cli.py and tui.py use the latter.

2. Checksums. Two implementations:

verify.py:69 application/replicas.py:35
signature compute_checksum(path, algo: ChecksumAlgo) compute_checksum(path, algo: str)
chunk size 64 KB 1 MB
accepted labels "xxhash", "sha256" "xxhash64", "sha256"

Only proxy is genuinely shared, via application/proxy_runner.py.

The sharp edge: the label mismatch

The digests are identical — verified:

legacy compute_checksum(p, ChecksumAlgo.XXHASH)  → 26c7827d889f6da3
vnext  compute_checksum(p, 'xxhash64')           → 26c7827d889f6da3

But the labels are not interchangeable:

vnext compute_checksum(p, ChecksumAlgo.XXHASH.value)
  → ValueError: unsupported checksum algorithm: xxhash

_normalize_checksum_algo exists (application/service.py:949) and maps xxhashxxhash64 — but it is applied at exactly one call site, settings.get (line 800). Every other path takes the algo from the caller and hands it straight to compute_checksum:

  • replica.verifyVerifyReplicaParams.checksum_algo
  • reconcile.asset / reconcile.project / reconcile.acceptChangechecksum_algo

So anything that reads the legacy config enum value and passes it to those methods raises. Now that PR #117 has the CLI, TUI and sidecar on one database, a checksum written by the legacy verify path and labelled xxhash sits in the same store the vNext reconcile path reads — and reconcile will reject the label for an algorithm it fully supports.

Suggested shape

Two separable pieces; the second is the urgent one.

Normalize at the boundary, not at one call site. Move the mapping into the pydantic model — a validator on the checksum_algo field of VerifyReplicaParams, ReconcileAssetParams, ReconcileProjectParams and AcceptChangeParams — so no path can receive an un-normalized label. Or make compute_checksum accept both spellings. This is small and should not wait for the refactor.

Collapse the duplicates. Pick one compute_checksum (the vNext one's 1 MB chunks are the better choice for multi-GB media) and have the other delegate. Same for organize: decide whether application/organize.py becomes the single implementation with organize.py delegating, or vice versa. This is a real refactor and needs its own decision about which direction the dependency runs — the direction doc says vNext should call the capability modules, but the vNext one is the implementation the desktop app actually exercises.

Not covered by any suite

Both implementations are individually tested and correct (test_verify.py, test_replicas.py, test_organize.py, test_organize_service.py). Nothing tests them against each other, so the label divergence is invisible.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    debtTechnical debt and maintenancerefactorCode restructuring without behavior change

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions