You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The PR adds dependency-limited terminal image rendering to storage uploads, introduces consent-gated test and eye-capture uploads, adds periodic capture scheduling and failure events, updates browser consent controls, and repositions the GPU meter overlay.
Changes
Storage image rendering
Layer / File(s)
Summary
Image renderer and dependency wiring Cargo.toml, services/storage/Cargo.toml, services/storage/src/lib.rs, services/storage/src/tty_image.rs, CLAUDE.md
Adds the feature-limited image dependency and a hand-written ANSI half-block renderer, with repository guidance on avoiding GPL/LGPL dependencies.
Image upload integration and validation services/storage/src/routes.rs, services/storage/tests/*
Tracks uploaded bytes, detects image extensions, renders images on the terminal, logs rendering failures, and tests filename detection and upload persistence.
We reviewed changes in 8d69f15...f037c15 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer TIP This summary will be updated as you push new changes.
The reason will be displayed to describe this comment to others. Learn more.
`sample_loop` has a cyclomatic complexity of 20 with "high" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
Diagnostic logs the version, but doesn't fix the underlying stale-cache gap.
The new comment explicitly calls out that this is the version fetched from package.json + injected as the wheel filename, so seeing the right number here after a version bump proves the module reloaded fresh (no stale-cache reuse of the package.json/wheel fetches below, neither of which is cache-busted). That's a good diagnostic, but et_ws_pyeye1.js in this same PR fixed the identical stale-cache risk by adding { cache: "no-cache" } to its package.json/wheel fetches, while et_ws_pydata1.js's equivalent fetches (line 66's wheel fetch, line 70's package.json fetch) were left uncached. Consider applying the same fix here instead of only detecting the symptom after the fact.
♻️ Apply the same no-cache fix used in et_ws_pyeye1.js
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-modules/pydata1/pkg/et_ws_pydata1.js` around lines 65 - 74,
Update the fetch calls in injectWheel and the package.json loading flow to pass
{ cache: "no-cache" }, matching the existing fix in et_ws_pyeye1.js. Apply this
to both wheel retrieval and package metadata retrieval so version and wheel
updates cannot reuse stale cached responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 1078-1085: The licensing rationale is duplicated across the
repository policy and module documentation. Keep the detailed viuer/ansi_colours
precedent in CLAUDE.md lines 1078-1085 unchanged, and remove the duplicated
dependency-license explanation from services/storage/src/tty_image.rs lines 3-8
while retaining only documentation about the renderer’s behavior.
In `@services/storage/src/routes.rs`:
- Around line 119-126: Update the first-line Rust doc summaries at
services/storage/src/routes.rs:119-126 and
services/storage/tests/image_logging.rs:1-2 so each is a self-contained,
complete sentence ending with a period; remove the mid-sentence wording and
cross-references to tty_image and put_file, while leaving the remaining
documentation unchanged.
- Around line 111-113: Update the image-handling branch around show_image_on_tty
to run terminal rendering in a bounded blocking/background task after
persistence, rather than synchronously on the Actix request worker. Preserve the
existing image detection and info! logging, and propagate render failures to
warn! from the spawned task.
In `@services/storage/src/tty_image.rs`:
- Around line 1-13: Update
config/ast-grep/rules/doc-summary-ends-with-period.yaml by adding every modified
Rust file to its sorted files list: services/storage/src/tty_image.rs (anchor),
services/storage/src/lib.rs, services/storage/src/routes.rs,
services/storage/tests/image_logging.rs, and services/storage/tests/put.rs.
Retain any entries already present and do not modify the Rust implementation
files directly.
- Around line 30-31: Update tty_image::render to check whether stdout is a
terminal before calling image::open; return successfully immediately when
std::io::stdout().is_terminal() is false, preserving the existing image
rendering path for terminal output.
- Around line 31-57: Bound thumbnail resource usage in the image-rendering
function: configure decoder limits before image::open to reject excessively
large dimensions or decoded pixels, and clamp the aspect-derived rows value
before sample_height and resize_exact are used. Preserve the existing minimum
one-row behavior while enforcing a finite maximum that also limits the generated
ANSI output.
In `@services/storage/tests/put.rs`:
- Around line 110-112: Update the first doc-summary line for the image PUT test
to explain that rendering fails because line 126 provides malformed PNG bytes,
not because of terminal availability or viuer behavior. Make the summary a
complete sentence ending with a period, while preserving the remaining
documentation.
In `@services/ws-server/static/meters.js`:
- Around line 254-257: Update the overlay initialization near makeDraggable so
the opposing left/top anchors are cleared when pointer dragging begins, before
pixel left/top values are written. Preserve the fixed bottom-right placement
before the first drag, then remove right/bottom when applying the first drag
position to prevent the overlay from stretching.
---
Nitpick comments:
In `@services/ws-modules/pydata1/pkg/et_ws_pydata1.js`:
- Around line 65-74: Update the fetch calls in injectWheel and the package.json
loading flow to pass { cache: "no-cache" }, matching the existing fix in
et_ws_pyeye1.js. Apply this to both wheel retrieval and package metadata
retrieval so version and wheel updates cannot reuse stale cached responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 72622927-0f9a-4a27-a592-f28d96bd0573
📥 Commits
Reviewing files that changed from the base of the PR and between 8d69f15 and f037c15.
Keep the licensing rationale in one location. The detailed viuer/ansi_colours precedent is duplicated between repository policy and module documentation; retain the policy rationale in CLAUDE.md and keep the module docs focused on renderer behavior.
CLAUDE.md#L1078-L1085: retain the repository policy and precedent here.
services/storage/src/tty_image.rs#L3-L8: remove the duplicated dependency-license explanation.
As per coding guidelines, "Document each fact exactly once in its most relevant location; do not duplicate explanations or add documentation cross-references."
📍 Affects 2 files
CLAUDE.md#L1078-L1085 (this comment)
services/storage/src/tty_image.rs#L3-L8
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 1078 - 1085, The licensing rationale is duplicated
across the repository policy and module documentation. Keep the detailed
viuer/ansi_colours precedent in CLAUDE.md lines 1078-1085 unchanged, and remove
the duplicated dependency-license explanation from
services/storage/src/tty_image.rs lines 3-8 while retaining only documentation
about the renderer’s behavior.
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -e
rg -n "show_image_on_tty|is_image_filename|stored image" services/storage/src -S
Repository: edge-toolkit/core
Length of output: 847
🏁 Script executed:
set -e
sed -n '1,220p' services/storage/src/routes.rs
Repository: edge-toolkit/core
Length of output: 6988
🏁 Script executed:
set -e
python3 - <<'PY'from pathlib import Pathp = Path('services/storage/src/routes.rs')for i, line in enumerate(p.read_text().splitlines(), 1): if 90 <= i <= 150: print(f"{i:4}: {line}")PY
Repository: edge-toolkit/core
Length of output: 3296
🏁 Script executed:
set -e
printf'%s\n''--- routes.rs ---'
python3 - <<'PY'from pathlib import Pathp = Path('services/storage/src/routes.rs')text = p.read_text().splitlines()for i, line in enumerate(text, 1): if 1 <= i <= 220: print(f"{i:4}: {line}")PYprintf'\n%s\n''--- show_image_on_tty references ---'
rg -n "fn show_image_on_tty|show_image_on_tty\(" -S services/storage/src
Repository: edge-toolkit/core
Length of output: 8227
🏁 Script executed:
set -e
python3 - <<'PY'from pathlib import Pathp = Path('services/storage/src/routes.rs')for i, line in enumerate(p.read_text().splitlines(), 1): if 103 <= i <= 118: print(f"{i:4}: {line}")PY
Repository: edge-toolkit/core
Length of output: 919
🏁 Script executed:
set -e
wc -l services/storage/src/tty_image.rs
sed -n '1,220p' services/storage/src/tty_image.rs
Repository: edge-toolkit/core
Length of output: 4483
Offload terminal rendering from the request worker.show_image_on_tty is synchronous here and does image decode, resize, string generation, and stdout I/O on the Actix worker. Move it to a bounded blocking/background task after persistence, while keeping the warn! on render failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/storage/src/routes.rs` around lines 111 - 113, Update the
image-handling branch around show_image_on_tty to run terminal rendering in a
bounded blocking/background task after persistence, rather than synchronously on
the Actix request worker. Preserve the existing image detection and info!
logging, and propagate render failures to warn! from the spawned task.
Use self-contained first-line Rust doc summaries. Both new doc blocks begin mid-sentence and point readers at another source file instead of stating a complete local summary.
services/storage/src/routes.rs#L119-L126: make Line 119 a complete period-terminated summary and remove the tty_image cross-reference.
services/storage/tests/image_logging.rs#L1-L2: make Line 1 a complete period-terminated summary and remove the put_file cross-reference.
As per coding guidelines, "fix first-line doc-summary punctuation" and "do not duplicate explanations or add documentation cross-references."
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/storage/src/routes.rs` around lines 119 - 126, Update the first-line
Rust doc summaries at services/storage/src/routes.rs:119-126 and
services/storage/tests/image_logging.rs:1-2 so each is a self-contained,
complete sentence ending with a period; remove the mid-sentence wording and
cross-references to tty_image and put_file, while leaving the remaining
documentation unchanged.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/storage/src/tty_image.rs` around lines 1 - 13, Update
config/ast-grep/rules/doc-summary-ends-with-period.yaml by adding every modified
Rust file to its sorted files list: services/storage/src/tty_image.rs (anchor),
services/storage/src/lib.rs, services/storage/src/routes.rs,
services/storage/tests/image_logging.rs, and services/storage/tests/put.rs.
Retain any entries already present and do not modify the Rust implementation
files directly.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf'## tty_image.rs\n'
sed -n '1,220p' services/storage/src/tty_image.rs
printf'\n## render usages\n'
rg -n "tty_image::render|render\(.*tty_image|render\(" services/storage -g'*.rs'printf'\n## terminal/stdout checks in storage service\n'
rg -n "is_terminal|IsTerminal|stdout\(\)\.is_terminal|stderr\(\)\.is_terminal|tty" services/storage -g'*.rs'
Repository: edge-toolkit/core
Length of output: 6223
🏁 Script executed:
sed -n '90,150p' services/storage/src/routes.rs
printf'\n## tests mentioning tty render\n'
sed -n '90,150p' services/storage/tests/put.rs
Repository: edge-toolkit/core
Length of output: 5527
Gate tty rendering on stdout render() still runs when stdout is redirected, so valid image uploads can emit ANSI escapes into logs and pay the decode/resize cost unnecessarily. Return early when std::io::stdout().is_terminal() is false.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/storage/src/tty_image.rs` around lines 30 - 31, Update
tty_image::render to check whether stdout is a terminal before calling
image::open; return successfully immediately when
std::io::stdout().is_terminal() is false, preserving the existing image
rendering path for terminal output.
#!/bin/bashset -euo pipefail
# Inspect the target file and any related constants/usages.
git ls-files services/storage/src/tty_image.rs
wc -l services/storage/src/tty_image.rs
cat -n services/storage/src/tty_image.rs | sed -n '1,220p'printf'\n-- nearby references --\n'
rg -n "TARGET_COLUMNS|rows|min\(MAX_ROWS\)|resize_exact|image::open|decode" services/storage/src -S
Repository: edge-toolkit/core
Length of output: 6331
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Probe the image crate API usage in the repo to see whether decode limits are already applied elsewhere.
rg -n "limits\(|with_guessed_format|ImageReader|max_dimensions|max_alloc". -S
Repository: edge-toolkit/core
Length of output: 448
Bound the rendered thumbnail size.rows scales directly with the decoded aspect ratio, so a very tall, narrow image can make resize_exact allocate a huge RGBA buffer and then build an equally large ANSI string. Clamp the rendered rows and add decoder limits before image::open so a single upload cannot exhaust memory or CPU.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/storage/src/tty_image.rs` around lines 31 - 57, Bound thumbnail
resource usage in the image-rendering function: configure decoder limits before
image::open to reject excessively large dimensions or decoded pixels, and clamp
the aspect-derived rows value before sample_height and resize_exact are used.
Preserve the existing minimum one-row behavior while enforcing a finite maximum
that also limits the generated ANSI output.
This test fails rendering because Line 126 supplies malformed PNG bytes, not because cargo test lacks a terminal; viuer is no longer used, and stdout writes are intentionally ignored. Rewrite the first doc-summary line as a complete sentence ending in a period. As per coding guidelines, "fix first-line doc-summary punctuation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/storage/tests/put.rs` around lines 110 - 112, Update the first
doc-summary line for the image PUT test to explain that rendering fails because
line 126 provides malformed PNG bytes, not because of terminal availability or
viuer behavior. Make the summary a complete sentence ending with a period, while
preserving the remaining documentation.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Inspect the relevant file and surrounding drag logic.
grep -n -C 4 -E 'makeDraggable|pointerdown|pointermove|pointerup|overlay.style|right|bottom|left|top' services/ws-server/static/meters.js
# Map the file structure to locate the draggable helper definition.
python3 - <<'PY'from pathlib import Pathp = Path('services/ws-server/static/meters.js')for i, line in enumerate(p.read_text().splitlines(), 1): if 'makeDraggable' in line or 'pointerdown' in line or 'pointermove' in line: print(f'{i}: {line}')PY# Try to inspect the published stats-gl artifact referenced in the review comment if reachable from the sandbox.
python3 - <<'PY'import urllib.request, sslurl = 'https://app.unpkg.com/stats-gl%403.8.0/files/dist/main.cjs'ctx = ssl.create_default_context()try: with urllib.request.urlopen(url, context=ctx, timeout=15) as r: data = r.read(4000).decode('utf-8', 'replace') print(data)except Exception as e: print(f'FETCH_ERROR: {e}')PY
Repository: edge-toolkit/core
Length of output: 5801
🏁 Script executed:
#!/bin/bashset -euo pipefail
python3 - <<'PY'import ssl, urllib.request, reurl = 'https://app.unpkg.com/stats-gl%403.8.0/files/dist/main.cjs'ctx = ssl._create_unverified_context()with urllib.request.urlopen(url, context=ctx, timeout=20) as r: text = r.read().decode('utf-8', 'replace')# Show the parts that mention the overlay element's sizing/positioning.for pat in [ r'position\s*[:=]\s*["\']fixed["\']', r'right\s*[:=]\s*["\']0["\']', r'bottom\s*[:=]\s*["\']0["\']', r'width\s*[:=]\s*["\']auto["\']', r'height\s*[:=]\s*["\']auto["\']', r'offsetWidth', r'offsetHeight', r'left\s*[:=]\s*["\']', r'top\s*[:=]\s*["\']',]: m = re.search(pat, text) print(f'{pat}: {"yes" if m else "no"}')# Print a small window around the first "position:fixed" or "right:0" occurrence if present.for needle in ['position:"fixed"', "position:'fixed'", 'right:"0"', "right:'0'", 'bottom:"0"', "bottom:'0'"]: idx = text.find(needle) if idx != -1: start = max(0, idx - 300) end = min(len(text), idx + 900) print('\n--- excerpt ---') print(text[start:end]) breakPY
Repository: edge-toolkit/core
Length of output: 405
🌐 Web query:
CSS absolutely positioned element left right width auto stretch shrink-to-fit specification
💡 Result:
For an absolutely positioned element, the interaction between left, right, and width properties is defined by a specific set of rules in the CSS specification [1][2]. When width is auto, the element does not automatically stretch unless both left and right are also specified [3]. Instead, it typically defaults to a shrink-to-fit behavior [1][4]. The layout behavior depends on which properties are set to auto: 1. Stretching behavior: An absolutely positioned element fills the available horizontal space (stretches) when both left and right are explicitly defined (i.e., not auto) and the width property is left as auto [2][3]. In this state, the browser solves for width based on the distance between the left and right offsets [2][4]. 2. Shrink-to-fit behavior: If width is auto and one of the horizontal offsets (left or right) is also auto, the element adopts a shrink-to-fit width [1][2]. Shrink-to-fit is a sizing algorithm similar to that used for table cells, which calculates the width based on the content's preferred minimum and maximum widths, constrained by the available space [4]. Specifically: - If left and width are auto, and right is specified, the element uses shrink-to-fit and then solves for left [1][5]. - If right and width are auto, and left is specified, the element uses shrink-to-fit and then solves for right [1][5]. 3. All auto: If left, right, and width are all auto, the browser sets margin-left and margin-right to 0, sets the element to its static position, and applies the shrink-to-fit behavior for the width [1][2]. Note that this behavior is distinct from the stretch keyword found in CSS Alignment, which can be applied to align-self or justify-self properties [6]. While specifying left and right offsets forces a stretch-like result for absolutely positioned elements with width: auto, it is a consequence of the layout equation solving for the width variable rather than an alignment-based stretch [2][3].
makeDraggable writes left/top while right/bottom stay set, which over-constrains this auto-width overlay and can stretch it across the viewport on the first drag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-server/static/meters.js` around lines 254 - 257, Update the
overlay initialization near makeDraggable so the opposing left/top anchors are
cleared when pointer dragging begins, before pixel left/top values are written.
Preserve the fixed bottom-right placement before the first drag, then remove
right/bottom when applying the first drag position to prevent the overlay from
stretching.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary by CodeRabbit