Skip to content

Upload image metadata json data as part of snapshots job #3163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rbro112 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Upload image metadata json data as part of snapshots job #3163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9f3fdf
Upload image metadata json data as part of snapshots job
rbro112 Feb 27, 2026
ef67db9
Cleanups
rbro112 Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/api/data_types/snapshots.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,6 @@ pub struct ImageMetadata {
pub image_file_name: String,
pub width: u32,
pub height: u32,
#[serde(skip_serializing_if = "Option::is_none")]
pub display_name: Option<String>,
}
67 changes: 66 additions & 1 deletion src/commands/build/snapshots.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use secrecy::ExposeSecret as _;
use sha2::{Digest as _, Sha256};
use walkdir::WalkDir;

use serde::Deserialize;

use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest};
use crate::config::{Auth, Config};
use crate::utils::args::ArgExt as _;
Expand Down Expand Up @@ -95,7 +97,8 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
style(images.len()).yellow(),
if images.len() == 1 { "file" } else { "files" }
);
let manifest_entries = upload_images(images, &org, &project)?;
let display_names = collect_display_names(dir_path);
let manifest_entries = upload_images(images, &display_names, &org, &project)?;

// Build manifest from discovered images
let manifest = SnapshotsManifest {
Expand Down Expand Up @@ -190,6 +193,7 @@ fn is_image_file(path: &Path) -> bool {

fn upload_images(
images: Vec<ImageInfo>,
display_names: &HashMap<String, String>,
org: &str,
project: &str,
) -> Result<HashMap<String, ImageMetadata>> {
Expand Down Expand Up @@ -247,12 +251,14 @@ fn upload_images(
.unwrap_or_default()
.to_string_lossy()
.into_owned();
let display_name = display_names.get(&image_file_name).cloned();
Comment on lines 251 to +254
Copy link

@sentry sentry bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code uses only the filename to look up an image's display name, but the manifest is keyed by the full relative path, causing lookups for images in subdirectories to fail silently.
Severity: MEDIUM

Suggested Fix

Use the full image.relative_path as a string when performing the lookup in the display_names map. Do not call .file_name() on the path, as this discards the subdirectory information needed to find a match.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/commands/build/snapshots.rs#L251-L254

Potential issue: When discovering images, the code correctly identifies a relative path
like `ui/button.png`. However, to look up the corresponding `display_name` from the
manifest, it extracts only the filename (`button.png`) using `.file_name()`. The
manifest data, stored in the `display_names` map, is keyed by the full relative path
provided by the user. This key mismatch causes the lookup to silently fail. As a result,
the `display_name` is omitted from the uploaded metadata for any images organized in
subdirectories, even when correctly specified by the user.

manifest_entries.insert(
hash,
ImageMetadata {
image_file_name,
width: image.width,
height: image.height,
display_name,
},
);
}
Expand Down Expand Up @@ -280,3 +286,62 @@ fn upload_images(
}
}
}

/// Input format for user-provided JSON manifest files.
#[derive(Deserialize)]
struct ManifestFile {
images: HashMap<String, ManifestFileEntry>,
}

#[derive(Deserialize)]
struct ManifestFileEntry {
image_file_name: String,
display_name: Option<String>,
}

/// Collects `image_file_name -> display_name` mappings from JSON manifest files in a directory.
fn collect_display_names(dir: &Path) -> HashMap<String, String> {
let mut display_names = HashMap::new();
let entries = WalkDir::new(dir)
.follow_links(true)
.into_iter()
.filter_entry(|e| !is_hidden(dir, e.path()));

for entry in entries.flatten() {
if !entry.file_type().is_file() {
continue;
}
let path = entry.path();
let is_json = path
.extension()
.and_then(|ext| ext.to_str())
.map(|ext| ext.eq_ignore_ascii_case("json"))
.unwrap_or(false);
if !is_json {
continue;
}

debug!("Reading manifest file: {}", path.display());
let contents = match fs::read_to_string(path) {
Ok(c) => c,
Err(err) => {
warn!("Failed to read manifest file {}: {err}", path.display());
continue;
}
};
let manifest: ManifestFile = match serde_json::from_str(&contents) {
Ok(m) => m,
Err(err) => {
warn!("Failed to parse manifest file {}: {err}", path.display());
continue;
}
};

for entry in manifest.images.into_values() {
if let Some(display_name) = entry.display_name {
display_names.insert(entry.image_file_name, display_name);
}
}
}
display_names
}