Skip to content

Commit 1e82188

Browse files
authored
feat(open)!: detect the artifact format inside the viewer importer (#277)
Follow-up to #264 review (johanpel): let the analyzer detect the artifact's serialization format instead of threading it through `quent-open`. `QuentViewer::import_events` and the `model!`-generated `import_events` now take just the context directory and detect the format from the on-disk streams (new `quent_exporter::FileSystemFormat::detect`). The server's `index_query_engines` detects per-context too. The generated viewer wrapper enables all exporter formats so any artifact opens, and `quent-open` drops `Format`/`detect_format` and the format component of its cache key. **Breaking:** `QuentViewer::import_events` / `Model::import_events` drop the `format` argument; `index_query_engines` drops its `format` parameter. Downstream `QuentViewer` impls (e.g. sirius) drop the `format` param and call `Model::import_events(dir)`. **Compatibility:** `quent-open` builds the wrapper against the quent/analyzer commits pinned in `model.qmi` and now emits the one-argument calls, so it can only open artifacts whose pinned commits already include this change. Artifacts pinned to earlier commits (from the pre-release window) won't build a viewer — an accepted consequence of this breaking change; versioning the sidecar/codegen against the pinned API is left as future work. Off `main`, independent of the merged quent-open work (#265) and the open stack (#266/#273/#276). Whichever of this PR and that stack lands second needs a rebase — both touch `quent-open`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Authors: - Matthijs Brobbel (https://github.com/mbrobbel) Approvers: - Johan Peltenburg (https://github.com/johanpel) URL: #277
1 parent b08df6b commit 1e82188

10 files changed

Lines changed: 77 additions & 161 deletions

File tree

crates/exporter/src/lib.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,33 @@ impl TryFrom<&str> for FileSystemFormat {
111111
}
112112
}
113113

114+
#[cfg(filesystem)]
115+
impl FileSystemFormat {
116+
/// Detect the format of a context directory from the first recognized
117+
/// `*.<ext>` event stream in any of its per-entity subdirectories. Returns
118+
/// `None` if no readable stream with a known extension is present.
119+
pub fn detect(context_dir: &std::path::Path) -> Option<Self> {
120+
for entry in std::fs::read_dir(context_dir).ok()?.flatten() {
121+
if !entry.file_type().map(|t| t.is_dir()).unwrap_or(false) {
122+
continue;
123+
}
124+
let Ok(files) = std::fs::read_dir(entry.path()) else {
125+
continue;
126+
};
127+
for file in files.flatten() {
128+
if let Some(format) = std::path::Path::new(&file.file_name())
129+
.extension()
130+
.and_then(|ext| ext.to_str())
131+
.and_then(|ext| Self::try_from(ext).ok())
132+
{
133+
return Some(format);
134+
}
135+
}
136+
}
137+
None
138+
}
139+
}
140+
114141
/// Options for exporting events to the filesystem in the given `format`, under
115142
/// the directory `root`, together with a `model.qmi` provenance sidecar.
116143
#[cfg(filesystem)]

crates/model-macros/src/model_macro.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,10 +341,14 @@ pub fn expand(input: TokenStream) -> syn::Result<TokenStream> {
341341
#[doc = #doc_import]
342342
pub fn import_events(
343343
dir: &std::path::Path,
344-
format: quent_model::exporter::FileSystemFormat,
345344
) -> quent_model::exporter::ImporterResult<
346345
Box<dyn Iterator<Item = quent_model::Event<#event_type>>>,
347346
> {
347+
// Detect the on-disk serialization format from the streams present;
348+
// an empty/unrecognized context yields no events.
349+
let Some(format) = quent_model::exporter::FileSystemFormat::detect(dir) else {
350+
return Ok(Box::new(std::iter::empty()));
351+
};
348352
let mut streams: Vec<
349353
Box<dyn Iterator<Item = quent_model::Event<#event_type>>>,
350354
> = Vec::new();

crates/open/src/error.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,6 @@ pub enum OpenError {
3535
)]
3636
NothingTrusted,
3737

38-
/// No recognized event stream extension was found, so the artifact format is
39-
/// unknown.
40-
#[error(
41-
"could not determine the artifact format under '{root}': no ndjson, msgpack, or postcard event streams found"
42-
)]
43-
UnknownFormat { root: PathBuf },
44-
4538
/// The sidecar lacks git remote/commit provenance needed to fetch a crate for
4639
/// the viewer build.
4740
#[error(

crates/open/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ use std::path::PathBuf;
6262
use quent_build_info::{ArtifactInfo, SIDECAR_FILE_NAME};
6363

6464
pub use crate::error::{OpenError, Result};
65-
pub use crate::spec::{Format, GitPin, ViewerSpec, discover_contexts};
65+
pub use crate::spec::{GitPin, ViewerSpec, discover_contexts};
6666
pub use crate::trust::{Trust, canonicalize_remote};
6767
pub use crate::viewer::ViewerGroup;
6868

@@ -115,7 +115,7 @@ pub async fn run(loader: impl Loader, options: OpenOptions) -> Result<()> {
115115
}
116116

117117
/// Group `contexts` into one viewer per distinct build spec (same analyzer + pinned
118-
/// commits + format), gate each source on [trust](OpenOptions::trust), then build
118+
/// commits), gate each source on [trust](OpenOptions::trust), then build
119119
/// and serve the approved viewers in parallel. Contexts that can't be opened (no
120120
/// analyzer package, unreadable sidecar) are skipped with a warning rather than
121121
/// aborting.
@@ -134,7 +134,7 @@ pub async fn open(contexts: Vec<PathBuf>, options: OpenOptions) -> Result<()> {
134134
path: context.join(SIDECAR_FILE_NAME),
135135
source,
136136
})
137-
.and_then(|info| ViewerSpec::from_artifact(&context, &info))
137+
.and_then(|info| ViewerSpec::from_artifact(&info))
138138
{
139139
Ok(spec) => spec,
140140
Err(e) => {

crates/open/src/spec.rs

Lines changed: 21 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
//! Build a [`ViewerSpec`] from a context's `model.qmi`: pinned git sources,
5-
//! analyzer package, and artifact format for generating/building a viewer.
4+
//! Build a [`ViewerSpec`] from a context's `model.qmi`: the pinned git sources
5+
//! and analyzer package needed to generate/build a viewer.
66
77
use std::hash::{Hash, Hasher};
8-
use std::path::{Path, PathBuf};
8+
use std::path::PathBuf;
99

1010
use quent_build_info::{ArtifactInfo, BuildInfo, SIDECAR_FILE_NAME};
1111
use walkdir::WalkDir;
@@ -67,43 +67,6 @@ fn is_hidden(entry: &walkdir::DirEntry) -> bool {
6767
.is_some_and(|name| name.starts_with('.'))
6868
}
6969

70-
/// Serialization format of an artifact's event streams.
71-
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
72-
pub enum Format {
73-
Ndjson,
74-
Msgpack,
75-
Postcard,
76-
}
77-
78-
impl Format {
79-
/// File extension of an event stream in this format.
80-
pub fn extension(self) -> &'static str {
81-
match self {
82-
Format::Ndjson => "ndjson",
83-
Format::Msgpack => "msgpack",
84-
Format::Postcard => "postcard",
85-
}
86-
}
87-
88-
/// The `quent_exporter::FileSystemFormat` variant name, for generated code.
89-
pub fn variant(self) -> &'static str {
90-
match self {
91-
Format::Ndjson => "Ndjson",
92-
Format::Msgpack => "Msgpack",
93-
Format::Postcard => "Postcard",
94-
}
95-
}
96-
97-
fn from_extension(ext: &str) -> Option<Self> {
98-
match ext {
99-
"ndjson" => Some(Format::Ndjson),
100-
"msgpack" => Some(Format::Msgpack),
101-
"postcard" => Some(Format::Postcard),
102-
_ => None,
103-
}
104-
}
105-
}
106-
10770
/// A git source pinned to an exact commit, as recorded in the sidecar.
10871
#[derive(Debug, Clone, PartialEq, Eq)]
10972
pub struct GitPin {
@@ -197,8 +160,6 @@ fn validate_package(package: &str) -> Result<()> {
197160
/// serve multiple same-spec contexts.
198161
#[derive(Debug, Clone)]
199162
pub struct ViewerSpec {
200-
/// Event serialization format, detected from the on-disk streams.
201-
pub format: Format,
202163
/// Cargo package of the analyzer crate providing `Viewer` (`QuentViewer`).
203164
pub analyzer_package: String,
204165
/// Quent framework source, pinned to the build commit.
@@ -208,8 +169,8 @@ pub struct ViewerSpec {
208169
}
209170

210171
impl ViewerSpec {
211-
/// Derive a spec from a sidecar and its context directory.
212-
pub fn from_artifact(root: &Path, info: &ArtifactInfo) -> Result<Self> {
172+
/// Derive a spec from a sidecar.
173+
pub fn from_artifact(info: &ArtifactInfo) -> Result<Self> {
213174
let analyzer_package =
214175
info.model
215176
.analyzer_package
@@ -219,7 +180,6 @@ impl ViewerSpec {
219180
})?;
220181
validate_package(&analyzer_package)?;
221182
Ok(Self {
222-
format: detect_format(root)?,
223183
analyzer_package,
224184
quent: GitPin::from_build_info(&info.quent, "quent")?,
225185
analyzer: GitPin::from_build_info(&info.model.source, "analyzer source")?,
@@ -232,29 +192,27 @@ impl ViewerSpec {
232192
self.analyzer_package.replace('-', "_")
233193
}
234194

235-
/// Unambiguous build identity: analyzer package, format, and both git
236-
/// remotes + full commits. Used to group/dedup contexts into viewers.
237-
/// Short label distinguishing this build from other groups (package, format,
238-
/// and short pins) so concurrent viewers with equal context counts are
239-
/// still tellable apart.
195+
/// Short label distinguishing this build from other groups (package and short
196+
/// pins) so concurrent viewers with equal context counts are still tellable
197+
/// apart.
240198
pub fn describe(&self) -> String {
241199
format!(
242-
"{} ({}, quent@{} analyzer@{})",
200+
"{} (quent@{} analyzer@{})",
243201
self.analyzer_package,
244-
self.format.extension(),
245202
short_commit(&self.quent.commit),
246203
short_commit(&self.analyzer.commit),
247204
)
248205
}
249206

207+
/// Unambiguous build identity: analyzer package and both git remotes + full
208+
/// commits. Used to group/dedup contexts into viewers.
250209
pub fn group_key(&self) -> String {
251210
// Key on the Cargo-normalized remotes so equivalent spellings (e.g.
252211
// scp-style vs `ssh://`) — which produce one dependency — share a build
253212
// instead of splitting into separate viewers. Unit separator between
254213
// fields so values can't run together.
255214
[
256215
self.analyzer_package.as_str(),
257-
self.format.extension(),
258216
self.quent.cargo_url().as_str(),
259217
&self.quent.commit,
260218
self.analyzer.cargo_url().as_str(),
@@ -270,10 +228,9 @@ impl ViewerSpec {
270228
let mut hasher = std::collections::hash_map::DefaultHasher::new();
271229
self.group_key().hash(&mut hasher);
272230
format!(
273-
"{}-{}-{}-{:016x}",
231+
"{}-{}-{:016x}",
274232
self.analyzer_package,
275233
short_commit(&self.analyzer.commit),
276-
self.format.extension(),
277234
hasher.finish(),
278235
)
279236
}
@@ -285,36 +242,11 @@ fn short_commit(commit: &str) -> &str {
285242
&commit[..end]
286243
}
287244

288-
/// Detect the artifact format from an `events.<ext>` stream in any per-entity
289-
/// subdirectory.
290-
fn detect_format(root: &Path) -> Result<Format> {
291-
let entries = std::fs::read_dir(root).map_err(|source| OpenError::Sidecar {
292-
path: root.to_path_buf(),
293-
source,
294-
})?;
295-
for entry in entries.flatten() {
296-
if !entry.file_type().map(|t| t.is_dir()).unwrap_or(false) {
297-
continue;
298-
}
299-
if let Ok(files) = std::fs::read_dir(entry.path()) {
300-
for file in files.flatten() {
301-
if let Some(ext) = Path::new(&file.file_name()).extension()
302-
&& let Some(format) = ext.to_str().and_then(Format::from_extension)
303-
{
304-
return Ok(format);
305-
}
306-
}
307-
}
308-
}
309-
Err(OpenError::UnknownFormat {
310-
root: root.to_path_buf(),
311-
})
312-
}
313-
314245
#[cfg(test)]
315246
mod tests {
316247
use super::*;
317248
use quent_build_info::ModelInfo;
249+
use std::path::Path;
318250

319251
fn artifact_with(analyzer_package: Option<&str>, commit: &str) -> ArtifactInfo {
320252
let mut model = ModelInfo::unknown();
@@ -334,14 +266,6 @@ mod tests {
334266
info
335267
}
336268

337-
fn ctx_with_stream(name: &str, file: &str) -> tempfile::TempDir {
338-
let dir = tempfile::tempdir().unwrap();
339-
let entity = dir.path().join(name);
340-
std::fs::create_dir_all(&entity).unwrap();
341-
std::fs::write(entity.join(file), b"").unwrap();
342-
dir
343-
}
344-
345269
fn make_context(dir: &Path) {
346270
std::fs::create_dir_all(dir.join("engine")).unwrap();
347271
std::fs::write(dir.join("engine").join("events.ndjson"), b"").unwrap();
@@ -407,7 +331,6 @@ mod tests {
407331
#[test]
408332
fn group_key_normalizes_equivalent_remotes() {
409333
let scp = ViewerSpec {
410-
format: Format::Ndjson,
411334
analyzer_package: "p".into(),
412335
quent: GitPin {
413336
remote: "git@github.com:org/quent.git".into(),
@@ -446,21 +369,6 @@ mod tests {
446369
assert_eq!(found.len(), 1);
447370
}
448371

449-
#[test]
450-
fn detects_format_from_entity_subdir() {
451-
let ctx = ctx_with_stream("engine", "events.msgpack");
452-
assert_eq!(detect_format(ctx.path()).unwrap(), Format::Msgpack);
453-
}
454-
455-
#[test]
456-
fn unknown_format_when_no_streams() {
457-
let ctx = ctx_with_stream("engine", "notes.txt");
458-
assert!(matches!(
459-
detect_format(ctx.path()),
460-
Err(OpenError::UnknownFormat { .. })
461-
));
462-
}
463-
464372
#[test]
465373
fn validators_accept_good_and_reject_injection() {
466374
assert!(validate_commit("0123456789abcdef0123456789abcdef01234567").is_ok());
@@ -485,10 +393,9 @@ mod tests {
485393

486394
#[test]
487395
fn spec_requires_analyzer_package() {
488-
let ctx = ctx_with_stream("engine", "events.ndjson");
489396
let info = artifact_with(None, "abc");
490397
assert!(matches!(
491-
ViewerSpec::from_artifact(ctx.path(), &info),
398+
ViewerSpec::from_artifact(&info),
492399
Err(OpenError::NoAnalyzer { .. })
493400
));
494401
}
@@ -515,34 +422,25 @@ mod tests {
515422

516423
#[test]
517424
fn spec_derives_crate_ident_and_keys() {
518-
let ctx = ctx_with_stream("engine", "events.ndjson");
519425
let info = artifact_with(Some("quent-simulator-analyzer"), "feedface99887766");
520-
let spec = ViewerSpec::from_artifact(ctx.path(), &info).unwrap();
426+
let spec = ViewerSpec::from_artifact(&info).unwrap();
521427
assert_eq!(spec.analyzer_crate(), "quent_simulator_analyzer");
522-
assert_eq!(spec.format, Format::Ndjson);
523428
assert!(
524429
spec.cache_key()
525-
.starts_with("quent-simulator-analyzer-feedface9988-ndjson-")
430+
.starts_with("quent-simulator-analyzer-feedface9988-")
526431
);
527432
}
528433

529434
#[test]
530435
fn keys_distinguish_full_pins_not_just_short_commit() {
531-
let ctx = ctx_with_stream("engine", "events.ndjson");
532-
// Same package, format, and 12-char commit prefix, but different full
533-
// analyzer commits — must NOT collide.
534-
let a =
535-
ViewerSpec::from_artifact(ctx.path(), &artifact_with(Some("p"), "abcabcabcabc1111"))
536-
.unwrap();
537-
let b =
538-
ViewerSpec::from_artifact(ctx.path(), &artifact_with(Some("p"), "abcabcabcabc2222"))
539-
.unwrap();
436+
// Same package and 12-char commit prefix, but different full analyzer
437+
// commits — must NOT collide.
438+
let a = ViewerSpec::from_artifact(&artifact_with(Some("p"), "abcabcabcabc1111")).unwrap();
439+
let b = ViewerSpec::from_artifact(&artifact_with(Some("p"), "abcabcabcabc2222")).unwrap();
540440
assert_ne!(a.group_key(), b.group_key());
541441
assert_ne!(a.cache_key(), b.cache_key());
542442
// Identical inputs group together and are deterministic.
543-
let a2 =
544-
ViewerSpec::from_artifact(ctx.path(), &artifact_with(Some("p"), "abcabcabcabc1111"))
545-
.unwrap();
443+
let a2 = ViewerSpec::from_artifact(&artifact_with(Some("p"), "abcabcabcabc1111")).unwrap();
546444
assert_eq!(a.group_key(), a2.group_key());
547445
assert_eq!(a.cache_key(), a2.cache_key());
548446
}

0 commit comments

Comments
 (0)