Validate original_hash in link_versions --reviewed; document follow-ups (#194)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
51600f900d
commit
aba6467cde
3 files changed
+67
-8
No files matched your search
@@ -379,8 +379,10 @@ for the `link_versions` binary).
|
|||||||
(1) a PNG's XMP `xmpMM:DerivedFrom/stRef:documentID = blake3:<hex>` (iTXt chunk
|
(1) a PNG's XMP `xmpMM:DerivedFrom/stRef:documentID = blake3:<hex>` (iTXt chunk
|
||||||
`XML:com.adobe.xmp`, read by hand, no decode); (2) the file-name convention next
|
`XML:com.adobe.xmp`, read by hand, no decode); (2) the file-name convention next
|
||||||
to an indexed original in the same library (same dir, or the parent of
|
to an indexed original in the same library (same dir, or the parent of
|
||||||
`aiedits/`). Name links are retried every tick (`sweep_name_links`), so an
|
`aiedits/`). Name links are retried every tick (`sweep_name_links`, paged by a
|
||||||
original that wasn't hashed yet, or arrives later, still links. Only *new* rows
|
per-library resume cursor that wraps, so candidates that never resolve can't starve
|
||||||
|
the ones behind them), so an original that wasn't hashed yet, or arrives later,
|
||||||
|
still links. Only *new* rows
|
||||||
are examined, so files ingested before this existed need the `link_versions`
|
are examined, so files ingested before this existed need the `link_versions`
|
||||||
binary once.
|
binary once.
|
||||||
- **Date/GPS/camera copy**: a version row takes its original's date triple, GPS and
|
- **Date/GPS/camera copy**: a version row takes its original's date triple, GPS and
|
||||||
@@ -397,16 +399,23 @@ for the `link_versions` binary).
|
|||||||
normal photo, so nothing disappears silently. Applied in `/photos` (SQL for the
|
normal photo, so nothing disappears silently. Applied in `/photos` (SQL for the
|
||||||
recursive listing, `hidden_version_rows` for the folder walk and tag branch),
|
recursive listing, `hidden_version_rows` for the folder walk and tag branch),
|
||||||
`/memories` and reels, `/photos/exif`, `/photos/gps-summary`, unified search, the
|
`/memories` and reels, `/photos/exif`, `/photos/gps-summary`, unified search, the
|
||||||
camera counts and the media gauges; `include_versions=true` lists them.
|
camera counts and the media gauges; `include_versions=true` lists them. The non-recursive `/photos` `dirs` also drops a
|
||||||
|
folder whose indexed contents are all hidden versions (typically `aiedits/`;
|
||||||
|
`dirs_holding_only_hidden_versions`, one indexed range scan per dir).
|
||||||
- **Display file** is fetched through the original's library, so the display
|
- **Display file** is fetched through the original's library, so the display
|
||||||
choice only counts when the chosen version has a row there (else the original
|
choice only counts when the chosen version has a row there (else the original
|
||||||
shows; `PUT /image/versions/display` answers 400). Recording a link also drops
|
shows; `PUT /image/versions/display` answers 400). Recording a link also drops
|
||||||
the version's `face_detections`, CLIP columns and `clip_failures`.
|
the version's `face_detections`, CLIP columns and `clip_failures`.
|
||||||
- **Tags and favorites** written or read on a version path bind to the original
|
- **Tags and favorites** written or read on a version path bind to the original
|
||||||
(`canonical_rel_path` in the tag/favorite DAOs, `get_rel_paths_sharing_content`
|
(`canonical_rel_path` in the tag/favorite DAOs, `get_rel_paths_sharing_content`
|
||||||
for reads). Insight generation on a version path runs against the original. A
|
for reads). Annotations are keyed by rel_path alone, so a path that also names a
|
||||||
*manual date* set on a version path is overwritten by the copy above: send the
|
non-version file in any library is never redirected. Linking merges favorites and
|
||||||
original's path.
|
tags already recorded on the version's own path into the original (union, duplicates
|
||||||
|
dropped; `link_version`). Insight generation on a version path runs against the
|
||||||
|
original. A *manual date* set or cleared on a version path (`POST /image/exif/date`,
|
||||||
|
`/clear`) is applied to the original's row instead, as the copy above would overwrite
|
||||||
|
it. Precomputed reels are not invalidated by a display change: reels render the
|
||||||
|
original's path, never the display file.
|
||||||
- **Excluded from derived work** (`NOT_A_VERSION_CLAUSE`): face backlog and ingest
|
- **Excluded from derived work** (`NOT_A_VERSION_CLAUSE`): face backlog and ingest
|
||||||
candidates, face stats (numerator and denominator), CLIP backlog and index, and
|
candidates, face stats (numerator and denominator), CLIP backlog and index, and
|
||||||
exact/perceptual duplicate grouping. Thumbnails and previews *are* generated.
|
exact/perceptual duplicate grouping. Thumbnails and previews *are* generated.
|
||||||
@@ -421,7 +430,8 @@ for the `link_versions` binary).
|
|||||||
TSV. `exact` rows (XMP, file name) are what ingest would have linked and apply
|
TSV. `exact` rows (XMP, file name) are what ingest would have linked and apply
|
||||||
with `--apply`; `review` rows (ComfyUI prompt `LoadImage` input, Upscayl-style
|
with `--apply`; `review` rows (ComfyUI prompt `LoadImage` input, Upscayl-style
|
||||||
names, nearest pHash) apply only from a reviewed TSV (`--reviewed`), each checked
|
names, nearest pHash) apply only from a reviewed TSV (`--reviewed`), each checked
|
||||||
against the current hash; `unresolved` rows are listed, never guessed. `--apply`
|
against the current version hash and the TSV's `original_hash` (stale rows are refused
|
||||||
|
and listed); `unresolved` rows are listed, never guessed. `--apply`
|
||||||
takes a `VACUUM INTO` snapshot first.
|
takes a `VACUUM INTO` snapshot first.
|
||||||
|
|
||||||
### Enhance on demand (#192)
|
### Enhance on demand (#192)
|
||||||
|
|||||||
@@ -179,6 +179,9 @@ fn main() -> anyhow::Result<()> {
|
|||||||
Arc::new(Mutex::new(Box::new(SqliteVersionDao::new())));
|
Arc::new(Mutex::new(Box::new(SqliteVersionDao::new())));
|
||||||
let mut guard = dao.lock().unwrap();
|
let mut guard = dao.lock().unwrap();
|
||||||
let report = backfill::apply(guard.as_mut(), &proposals, &reviewed, &rows);
|
let report = backfill::apply(guard.as_mut(), &proposals, &reviewed, &rows);
|
||||||
|
for why in &report.stale_details {
|
||||||
|
eprintln!(" stale reviewed row refused: {why}");
|
||||||
|
}
|
||||||
let synced = guard
|
let synced = guard
|
||||||
.sync_version_metadata()
|
.sync_version_metadata()
|
||||||
.map_err(|e| anyhow::anyhow!("{e:?}"))?;
|
.map_err(|e| anyhow::anyhow!("{e:?}"))?;
|
||||||
|
|||||||
@@ -110,6 +110,8 @@ pub struct ReviewedLink {
|
|||||||
pub method: String,
|
pub method: String,
|
||||||
pub library_id: i32,
|
pub library_id: i32,
|
||||||
pub version_path: String,
|
pub version_path: String,
|
||||||
|
/// The original's path as the dry run proposed it (may be empty).
|
||||||
|
pub original_path: String,
|
||||||
pub kind: String,
|
pub kind: String,
|
||||||
pub version_hash: String,
|
pub version_hash: String,
|
||||||
pub original_hash: String,
|
pub original_hash: String,
|
||||||
@@ -140,6 +142,7 @@ pub fn parse_reviewed(tsv: &str) -> (Vec<ReviewedLink>, usize) {
|
|||||||
method: f[1].to_string(),
|
method: f[1].to_string(),
|
||||||
library_id: f[2].parse().unwrap(),
|
library_id: f[2].parse().unwrap(),
|
||||||
version_path: f[3].to_string(),
|
version_path: f[3].to_string(),
|
||||||
|
original_path: f[4].to_string(),
|
||||||
kind: f[6].to_string(),
|
kind: f[6].to_string(),
|
||||||
version_hash: f[7].to_string(),
|
version_hash: f[7].to_string(),
|
||||||
original_hash: f[8].to_string(),
|
original_hash: f[8].to_string(),
|
||||||
@@ -634,6 +637,8 @@ pub struct ApplyReport {
|
|||||||
pub refused: usize,
|
pub refused: usize,
|
||||||
/// Reviewed rows whose version row no longer matches the file in the TSV.
|
/// Reviewed rows whose version row no longer matches the file in the TSV.
|
||||||
pub stale: usize,
|
pub stale: usize,
|
||||||
|
/// One line per stale reviewed row, saying what no longer matches.
|
||||||
|
pub stale_details: Vec<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Write the links. `exact` proposals apply as they are; `reviewed` come
|
/// Write the links. `exact` proposals apply as they are; `reviewed` come
|
||||||
@@ -651,6 +656,8 @@ pub fn apply(
|
|||||||
.map(|r| ((r.library_id, r.rel_path.as_str()), r.content_hash.as_str()))
|
.map(|r| ((r.library_id, r.rel_path.as_str()), r.content_hash.as_str()))
|
||||||
.collect();
|
.collect();
|
||||||
|
|
||||||
|
let live_hashes: HashSet<&str> = rows.iter().map(|r| r.content_hash.as_str()).collect();
|
||||||
|
|
||||||
let mut links: Vec<NewVersionLink> = Vec::new();
|
let mut links: Vec<NewVersionLink> = Vec::new();
|
||||||
for p in exact.iter().filter(|p| p.status == Status::Exact) {
|
for p in exact.iter().filter(|p| p.status == Status::Exact) {
|
||||||
links.push(NewVersionLink {
|
links.push(NewVersionLink {
|
||||||
@@ -669,8 +676,32 @@ pub fn apply(
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
for r in reviewed {
|
for r in reviewed {
|
||||||
if current.get(&(r.library_id, r.version_path.as_str())) != Some(&r.version_hash.as_str()) {
|
let stale = if current.get(&(r.library_id, r.version_path.as_str()))
|
||||||
|
!= Some(&r.version_hash.as_str())
|
||||||
|
{
|
||||||
|
Some("version file now holds different bytes (or is gone)")
|
||||||
|
} else if !live_hashes.contains(r.original_hash.as_str()) {
|
||||||
|
Some("original_hash is not in the index any more")
|
||||||
|
} else if !r.original_path.is_empty()
|
||||||
|
&& current
|
||||||
|
.get(&(r.library_id, r.original_path.as_str()))
|
||||||
|
.is_some_and(|h| *h != r.original_hash.as_str())
|
||||||
|
{
|
||||||
|
Some("original path now holds different bytes than original_hash")
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
if let Some(why) = stale {
|
||||||
report.stale += 1;
|
report.stale += 1;
|
||||||
|
report.stale_details.push(format!(
|
||||||
|
"{} -> {}: {why}",
|
||||||
|
r.version_path,
|
||||||
|
if r.original_path.is_empty() {
|
||||||
|
&r.original_hash
|
||||||
|
} else {
|
||||||
|
&r.original_path
|
||||||
|
}
|
||||||
|
));
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
links.push(NewVersionLink {
|
links.push(NewVersionLink {
|
||||||
@@ -977,6 +1008,19 @@ mod tests {
|
|||||||
stale_rows[2].content_hash = "changed".into();
|
stale_rows[2].content_hash = "changed".into();
|
||||||
let report = apply(&mut dao, &[], &reviewed, &stale_rows);
|
let report = apply(&mut dao, &[], &reviewed, &stale_rows);
|
||||||
assert_eq!(report.stale, 1);
|
assert_eq!(report.stale, 1);
|
||||||
|
|
||||||
|
// So is one whose original changed since the dry run: its hash is
|
||||||
|
// gone from the index, or its path holds other bytes.
|
||||||
|
let mut gone = rows.clone();
|
||||||
|
gone[0].content_hash = "ha-edited".into();
|
||||||
|
let report = apply(&mut dao, &[], &reviewed, &gone);
|
||||||
|
assert_eq!(report.stale, 1);
|
||||||
|
assert_eq!(report.linked + report.already_linked, 0);
|
||||||
|
assert!(report.stale_details[0].contains("d/aiedits/o_00001_.png"));
|
||||||
|
let mut swapped = rows.clone();
|
||||||
|
swapped[0].content_hash = "hb".into(); // d/a.jpg now holds hb's bytes
|
||||||
|
let report = apply(&mut dao, &[], &reviewed, &swapped);
|
||||||
|
assert_eq!(report.stale, 1);
|
||||||
}
|
}
|
||||||
#[test]
|
#[test]
|
||||||
fn apply_drops_derived_data_of_an_already_ingested_version() {
|
fn apply_drops_derived_data_of_an_already_ingested_version() {
|
||||||
@@ -1000,11 +1044,13 @@ mod tests {
|
|||||||
method: "phash".into(),
|
method: "phash".into(),
|
||||||
library_id: 1,
|
library_id: 1,
|
||||||
version_path: "v.png".into(),
|
version_path: "v.png".into(),
|
||||||
|
original_path: "o.jpg".into(),
|
||||||
kind: "edit".into(),
|
kind: "edit".into(),
|
||||||
version_hash: "v".into(),
|
version_hash: "v".into(),
|
||||||
original_hash: "o".into(),
|
original_hash: "o".into(),
|
||||||
references: vec![],
|
references: vec![],
|
||||||
}];
|
}];
|
||||||
|
let rows = vec![row("v.png", "v", None), row("o.jpg", "o", None)];
|
||||||
assert_eq!(apply(&mut dao, &[], &reviewed, &rows).linked, 1);
|
assert_eq!(apply(&mut dao, &[], &reviewed, &rows).linked, 1);
|
||||||
|
|
||||||
#[derive(diesel::QueryableByName)]
|
#[derive(diesel::QueryableByName)]
|
||||||
|
|||||||
Reference in new issue
Block a user