Skip to content

fix(sync): canonicalize dependency import defaults - #468

Open
lifeofgurpreet wants to merge 1 commit into
Dicklesworthstone:mainfrom
lifeofgurpreet:fix/dependency-default-import-verification
Open

fix(sync): canonicalize dependency import defaults#468
lifeofgurpreet wants to merge 1 commit into
Dicklesworthstone:mainfrom
lifeofgurpreet:fix/dependency-default-import-verification

Conversation

@lifeofgurpreet

Copy link
Copy Markdown

Problem

Legacy JSONL may omit dependency created_by, metadata, and thread_id.
The importer and SQLite schema persist those omissions as "import", "{}",
and "". Strict post-import verification therefore compared a sparse source
shape with its hydrated storage shape and could reject a lossless first import.
The equal-timestamp path could also fail to certify the second import as a
no-op and arm needs_flush.

Change

  • Extend the existing persisted-default canonicalizer to dependency fields.
  • Use the same canonical form in both rebuilt-import verification and skipped
    equal-timestamp certification.
  • Preserve every explicit non-default value via get_or_insert_with.
  • Add a full importer regression covering omitted JSON fields, hydrated storage
    values, explicit-value preservation, strict verification, a certified second
    no-op, export hashes, and needs_flush != true.

This does not weaken semantic comparison: it compares against the exact values
the existing writer/schema already persist.

Behavioral example

A sparse dependency such as:

{"issue_id":"bd-dependent","depends_on_id":"bd-target","type":"blocks"}

is verified against its established persisted representation:

{"created_by":"import","metadata":"{}","thread_id":""}

Explicit source values remain byte-for-byte unchanged.

Verification

Passing patch-specific and safety evidence on exact base
d2393c99ddcf337bd77d4ba61ce29ea8cdbe715b:

  • focused release regression: 1 passed, 0 failed (2,888 filtered)
  • explicit-value preservation and second-import assertions: created 0,
    updated 0, skipped 3, export hashes 3, needs_flush != true
  • cargo check --all-targets --locked
  • cargo clippy --all-targets --locked -- -D warnings
  • nightly rustfmt check on both changed Rust modules
  • git diff --check
  • release sync-safety group: 19 unit tests passed
  • PATH/git sync E2E passed
  • direct runtime dependency guard passed
  • real Hub mirrors for authentik, NFC Cards, Reka Slackbot, and Twenty each
    converged on first import and certified a second no-op

The full release library run completed with 2,876 passed, 4 failed, and 8
ignored. The four failures are outside the changed behavior:

  • doctor byte-identity assertion: independently reproduced at exact upstream
    base d2393c99
  • logging verbosity assertion: fails in an isolated release rerun with
    RUST_LOG unset because release code returns error while the untouched test
    expects the debug-build filter
  • two vacuum/sidecar tests: both pass in isolated exact release reruns and only
    failed during the parallel full-suite run due another database opener

UBS v5.3.13 also completed with a clean private ast-grep v0.45.3; its cargo
fmt, clippy, check, and test-build sections passed. Its aggregate exit remains
nonzero on pre-existing whole-file heuristics in these very large modules (232
critical / 4,417 warning), so tracker beads_rust-w4xl intentionally remains
open rather than falsely claiming the literal UBS-pass criterion.

Scope boundaries

No schema migration, export-format change, dependency rewrite, verifier bypass,
or Git-side behavior is introduced.

Refs: beads_rust-w4xl

Canonicalize omitted dependency persistence fields before strict import verification and equal-timestamp no-op certification. Preserve explicit values and cover first-import plus second-import behavior.\n\nRefs: beads_rust-w4xl
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.

1 participant