Skip to content

fix(design): keep site gallery filled under queue floods - #81

Merged
echobt merged 2 commits into
mainfrom
fix/design-gallery-empty
Aug 7, 2026
Merged

fix(design): keep site gallery filled under queue floods#81
echobt merged 2 commits into
mainfrom
fix/design-gallery-empty

Conversation

@echobt

@echobt echobt commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Root cause: After screenshots-only site filtering, /v1/site/arenas/design/submissions builds from dashboard recent_runs. Prod’s newest 40 runs were all queued (no PNGs), so the gallery filtered to empty even though dozens of awaiting_admin runs have live index.pngs. Leaderboard was empty because the open round has no winners yet while previous-round ratings (21) were only used for deltas.
  • Fix: Prefer post-sanitize stages in dashboard recent_runs (wider 500-row scan), and fall back the marketing design leaderboard to previous-round standings when current ratings are empty.
  • Also serializes agentic run_command env tests that raced under parallel libtest (pre-push flake).
  • No frontend change required; CORP/PNG serving is already healthy (200 image/png, Cross-Origin-Resource-Policy: cross-origin).

Test plan

  • cargo test -p design-http select_recent_runs
  • cargo test -p site-api design_leaderboard_falls_back
  • cargo test -p challenge-agentic --lib tools::tests::run_command
  • pre-push workspace lib tests
  • Deploy design-challenge + gateway (site-api) to prod
  • Verify GET https://chain.joinbase.ai/v1/site/arenas/design/submissions returns total > 0 with screenshotUrl
  • Verify GET .../leaderboard returns previous-round rows when current ratings empty
  • Confirm https://joinbase.ai/challenges/design shows gallery + board

Summary by CodeRabbit

  • Improvements

    • Dashboard recent runs now prioritize stages likely to include screenshots while preserving queue activity.
    • Dashboard results are limited to the most relevant 40 jobs.
    • Design leaderboards now fall back to the previous round when the current round has no ratings.
    • Submissions without screenshots are excluded from site API results.
    • Prism window series now include a terminal point when no step curve is available.
  • Documentation

    • Updated site API documentation to reflect these behaviors.

echobt added 2 commits August 7, 2026 13:26
Dashboard recent_runs prioritized newest queued rows, so the screenshots-only
site filter returned an empty design gallery despite many awaiting_admin PNGs.
Prefer post-sanitize stages in recent_runs, and fall back the marketing
leaderboard to previous-round ratings when the open round has no winners yet.
Parallel libtest raced set_var/remove_var on AGENTIC_ENABLE_RUN_COMMAND and
flaked the pre-push workspace suite.
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request serializes environment-sensitive tests, changes dashboard recent-run selection, adds previous-round leaderboard fallback behavior, and updates site API documentation.

Changes

Test environment isolation

Layer / File(s) Summary
Serialize run-command environment access
crates/challenge-agentic/src/tools.rs
Both run-command tests acquire a shared mutex before accessing AGENTIC_ENABLE_RUN_COMMAND.

Site API behavior

Layer / File(s) Summary
Prioritize dashboard recent runs
crates/design-http/src/stats.rs
Dashboard loading scans up to 500 runs, prioritizes post-sanitize and scored stages, and limits selected jobs to 40. Unit tests verify the ordering.
Fallback to previous-round ratings
crates/site-api/src/handlers.rs, docs/SITE_API.md
The design leaderboard returns previous-round ratings when the current round has none. The integration test and API documentation cover the fallback and related response behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the primary gallery behavior change under queue floods.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/design-gallery-empty

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
crates/design-http/src/stats.rs (1)

123-136: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Add the list_runs descending-order contract.

Both persisted and memory list_runs implementations use the latest rows, but DesignStore::list_runs is unsigned. Add a short doc/contract next to the trait method so future store implementations do not break dashboard ordering.

🤖 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 `@crates/design-http/src/stats.rs` around lines 123 - 136, Document the
descending-order requirement on the DesignStore::list_runs trait method, stating
that implementations must return the latest runs first so dashboard ordering
remains consistent across persisted and in-memory stores. Do not change
select_recent_runs or the list_runs implementation logic.
🤖 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 `@crates/challenge-agentic/src/tools.rs`:
- Around line 707-715: Update both tests protected by RUN_COMMAND_ENV_LOCK,
run_command_executes_in_sandboxed_cwd and
run_command_disabled_without_container_env, to capture the existing
AGENTIC_ENABLE_RUN_COMMAND value and restore it via a scoped guard before the
mutex guard is released, including during panic unwinding. Preserve each test’s
current environment mutations while ensuring the original present or absent
value is reinstated afterward.

---

Nitpick comments:
In `@crates/design-http/src/stats.rs`:
- Around line 123-136: Document the descending-order requirement on the
DesignStore::list_runs trait method, stating that implementations must return
the latest runs first so dashboard ordering remains consistent across persisted
and in-memory stores. Do not change select_recent_runs or the list_runs
implementation logic.
🪄 Autofix

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: fbdfaaf9-8f4d-4583-b0ac-79e9768ac71a

📥 Commits

Reviewing files that changed from the base of the PR and between f8065fa and fd159ae.

📒 Files selected for processing (4)
  • crates/challenge-agentic/src/tools.rs
  • crates/design-http/src/stats.rs
  • crates/site-api/src/handlers.rs
  • docs/SITE_API.md

Comment on lines +707 to +715
/// `AGENTIC_ENABLE_RUN_COMMAND` is process-global; serialize the two tests
/// that mutate it so parallel libtest cannot race enable vs disable.
static RUN_COMMAND_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());

#[test]
fn run_command_executes_in_sandboxed_cwd() {
let _guard = RUN_COMMAND_ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Restore AGENTIC_ENABLE_RUN_COMMAND after each test.

The mutex serializes the two tests, but it does not restore the previous process-global value. run_command_executes_in_sandboxed_cwd removes the variable at Line 729, and run_command_disabled_without_container_env removes it at Line 746. The review container sets this variable in crates/review-docker/src/lib.rs:79-95. A later test can therefore observe the wrong ToolContext::from_request mode. Capture the previous value and restore it with a scoped guard before releasing RUN_COMMAND_ENV_LOCK, including during panic unwinding.

Also applies to: 743-746

🤖 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 `@crates/challenge-agentic/src/tools.rs` around lines 707 - 715, Update both
tests protected by RUN_COMMAND_ENV_LOCK, run_command_executes_in_sandboxed_cwd
and run_command_disabled_without_container_env, to capture the existing
AGENTIC_ENABLE_RUN_COMMAND value and restore it via a scoped guard before the
mutex guard is released, including during panic unwinding. Preserve each test’s
current environment mutations while ensuring the original present or absent
value is reinstated afterward.

@echobt
echobt merged commit 4608b97 into main Aug 7, 2026
3 checks passed
@echobt
echobt deleted the fix/design-gallery-empty branch August 7, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant