Address PR #177 review feedback

- trace.rs: Use repeated query params (?keys=a&keys=b) instead of
  comma-separated, since commas are legal in S3 keys
- trace.rs: Fetch S3 objects concurrently with join_all instead of
  sequentially
- storage.rs: Use DisplayErrorContext for S3 errors to preserve the
  full error chain
- dev_server.rs: Make port overrideable via PORT env var
- Update browser.html, tests, and README to match new keys format

Author: rcoh

Cargo.lock

@@ -20,8 +20,10 @@ required-features = ["dev-server"]
 
 [dependencies]
 axum = "0.8"
+axum-extra = { version = "0.10", features = ["query"] }
 clap = { version = "4", features = ["derive"] }
 flate2 = "1"
+futures = "0.3"
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
 tokio = { version = "1", features = ["rt-multi-thread", "macros", "signal"] }

dial9-viewer/Cargo.toml

@@ -34,7 +34,7 @@ Open `http://localhost:3000` to browse traces. Enter a search prefix (e.g. `2026
 | Endpoint | Description |
 |----------|-------------|
 | `GET /api/search?q=<prefix>&bucket=<bucket>` | List S3 objects matching the prefix |
-| `GET /api/trace?keys=<k1,k2,...>&bucket=<bucket>` | Fetch, gunzip, and concatenate trace segments |
+| `GET /api/trace?keys=<k1>&keys=<k2>&bucket=<bucket>` | Fetch, gunzip, and concatenate trace segments |
 
 The trace endpoint returns raw binary data (`application/octet-stream`) suitable for loading directly in the trace viewer via `?trace=` URL parameter. Maximum response size is 50 MB.
 

dial9-viewer/README.md

@@ -85,10 +85,14 @@ async fn main() -> anyhow::Result<()> {
     let ui_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("ui");
     let app = dial9_viewer::server::router(state, &ui_dir);
 
-    let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await?;
-    tracing::info!("dial9-viewer dev server listening on http://localhost:3000");
+    let port: u16 = std::env::var("PORT")
+        .ok()
+        .and_then(|p| p.parse().ok())
+        .unwrap_or(3000);
+    let listener = tokio::net::TcpListener::bind(("0.0.0.0", port)).await?;
+    tracing::info!("dial9-viewer dev server listening on http://localhost:{port}");
     tracing::info!("bucket={bucket}, prefix=traces");
-    tracing::info!("try: http://localhost:3000/browser.html");
+    tracing::info!("try: http://localhost:{port}/browser.html");
     tracing::info!("search for: 2026-04-09/");
 
     axum::serve(listener, app)

dial9-viewer/src/bin/dev_server.rs

@@ -1,8 +1,10 @@
 use axum::body::Body;
-use axum::extract::{Query, State};
+use axum::extract::State;
 use axum::http::StatusCode;
 use axum::response::{IntoResponse, Response};
+use axum_extra::extract::Query;
 use flate2::read::GzDecoder;
+use futures::future::join_all;
 use serde::Deserialize;
 use std::io::Read;
 
@@ -13,8 +15,9 @@ const MAX_KEYS: usize = 100;
 
 #[derive(Deserialize)]
 pub struct TraceParams {
-    /// Comma-separated S3 keys
-    pub keys: String,
+    /// S3 keys (repeated query param: ?keys=a&keys=b)
+    #[serde(default)]
+    pub keys: Vec<String>,
     pub bucket: Option<String>,
 }
 
@@ -27,7 +30,12 @@ pub async fn get_trace(
         .or(state.default_bucket.clone())
         .ok_or((StatusCode::BAD_REQUEST, "bucket is required".to_string()))?;
 
-    let keys: Vec<&str> = params.keys.split(',').filter(|k| !k.is_empty()).collect();
+    let keys: Vec<&str> = params
+        .keys
+        .iter()
+        .map(|k| k.as_str())
+        .filter(|k| !k.is_empty())
+        .collect();
     if keys.is_empty() {
         return Err((StatusCode::BAD_REQUEST, "keys is required".to_string()));
     }
@@ -38,15 +46,14 @@ pub async fn get_trace(
         ));
     }
 
-    let mut combined = Vec::new();
-
-    for key in &keys {
-        let data = state
-            .backend
-            .get_object(&bucket, key)
-            .await
-            .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
+    let fetches = keys
+        .iter()
+        .map(|key| state.backend.get_object(&bucket, key));
+    let results = join_all(fetches).await;
 
+    let mut combined = Vec::new();
+    for result in results {
+        let data = result.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
         let raw = maybe_gunzip(&data);
 
         if combined.len() + raw.len() > MAX_RESPONSE_BYTES {

dial9-viewer/src/server/trace.rs

@@ -84,10 +84,10 @@ impl StorageBackend for S3Backend {
                     req = req.continuation_token(token);
                 }
 
-                let resp = req
-                    .send()
-                    .await
-                    .map_err(|e| StorageError::Other(e.to_string()))?;
+                let resp = req.send().await.map_err(|e| {
+                    use aws_sdk_s3::error::DisplayErrorContext;
+                    StorageError::Other(format!("{}", DisplayErrorContext(&e)))
+                })?;
 
                 for obj in resp.contents() {
                     if let Some(key) = obj.key() {
@@ -123,20 +123,23 @@ impl StorageBackend for S3Backend {
         let bucket = bucket.to_string();
         let key = key.to_string();
         Box::pin(async move {
-            use aws_sdk_s3::operation::get_object::GetObjectError;
-
             let resp = self
                 .client
                 .get_object()
                 .bucket(&bucket)
                 .key(&key)
                 .send()
                 .await
-                .map_err(|e| match e.into_service_error() {
-                    GetObjectError::NoSuchKey(_) => {
-                        StorageError::NotFound(format!("{bucket}/{key}"))
+                .map_err(|e| {
+                    use aws_sdk_s3::error::DisplayErrorContext;
+                    use aws_sdk_s3::operation::get_object::GetObjectError;
+
+                    match e.into_service_error() {
+                        GetObjectError::NoSuchKey(_) => {
+                            StorageError::NotFound(format!("{bucket}/{key}"))
+                        }
+                        other => StorageError::Other(format!("{}", DisplayErrorContext(&other))),
                     }
-                    other => StorageError::Other(other.to_string()),
                 })?;
 
             let bytes = resp

dial9-viewer/src/storage.rs

@@ -305,7 +305,9 @@ async fn trace_fetches_and_concatenates() {
     put_object(&s3, "test-bucket", "seg2.bin.gz", &gzip_bytes(trace2)).await;
 
     let resp = client
-        .get(format!("{base}/api/trace?keys=seg1.bin.gz,seg2.bin.gz"))
+        .get(format!(
+            "{base}/api/trace?keys=seg1.bin.gz&keys=seg2.bin.gz"
+        ))
         .send()
         .await
         .unwrap();
@@ -398,12 +400,15 @@ async fn e2e_search_then_view() {
 
     // Step 2: Build the trace URL from search results — like the browser's viewSelected()
     let keys: Vec<&str> = search_resp.iter().map(|o| o.key.as_str()).collect();
-    let keys_param = keys.join(",");
+    let keys_param: String = keys
+        .iter()
+        .map(|k| format!("keys={}", urlencoding::encode(k)))
+        .collect::<Vec<_>>()
+        .join("&");
 
     let trace_resp = client
         .get(format!(
-            "{base}/api/trace?keys={}&bucket=traces-bucket",
-            urlencoding::encode(&keys_param)
+            "{base}/api/trace?{keys_param}&bucket=traces-bucket",
         ))
         .send()
         .await

dial9-viewer/tests/server_test.rs

@@ -310,8 +310,8 @@ <h1>dial9 Trace Browser</h1>
         if (keys.length === 0) return;
 
         const bucket = bucketInput.value.trim();
-        const keysParam = keys.join(',');
-        const traceUrl = `/api/trace?bucket=${encodeURIComponent(bucket)}&keys=${encodeURIComponent(keysParam)}`;
+        const keysParams = keys.map(k => `keys=${encodeURIComponent(k)}`).join('&');
+        const traceUrl = `/api/trace?bucket=${encodeURIComponent(bucket)}&${keysParams}`;
         window.open(`index.html?trace=${encodeURIComponent(traceUrl)}`, '_blank');
     }
     </script>

dial9-viewer/ui/browser.html

@@ -0,0 +1,41 @@
+# dial9-viewer MVP Progress
+
+## Status: In Progress
+
+## Task 1: Move UI files
+- [ ] Move files
+- [ ] Update symlink
+- [ ] Update all references
+- [ ] Verify tests pass
+
+## Task 2: Scaffold crate
+- [ ] Cargo.toml + workspace
+- [ ] main.rs with clap
+- [ ] axum static file serving
+- [ ] placeholder browser.html
+- [ ] serve.py
+- [ ] integration test
+
+## Task 3: StorageBackend + /api/search
+- [ ] StorageBackend trait
+- [ ] S3Backend impl
+- [ ] /api/search endpoint
+- [ ] integration test with s3s-fs
+
+## Task 4: /api/trace endpoint
+- [ ] get_object on trait
+- [ ] gunzip + concat logic
+- [ ] 50MB cap
+- [ ] integration test
+
+## Task 5: browser.html
+- [ ] search box + bucket input
+- [ ] results table with checkboxes
+- [ ] View Selected → opens viewer
+- [ ] fallback when no backend
+
+## Task 6: Cleanup
+- [ ] README.md
+- [ ] clippy clean
+- [ ] fmt clean
+- [ ] nextest all green