feat(viewer): add interactive champion diff replay - #48
Conversation
📝 WalkthroughWalkthroughAdds a read-only ChangesExperiment viewer
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CLI
participant ViewerApp
participant SnapshotStore
participant WorkspaceReader
participant HarborBridge
participant Browser
Operator->>CLI: run evolve view
CLI->>ViewerApp: start with workspace and port range
ViewerApp->>SnapshotStore: refresh workspace snapshot
SnapshotStore->>WorkspaceReader: read archive and documents
WorkspaceReader-->>SnapshotStore: return workspace sources
SnapshotStore->>HarborBridge: refresh referenced Harbor jobs
HarborBridge-->>SnapshotStore: return trial links
SnapshotStore-->>ViewerApp: expose snapshot and inspection APIs
Browser->>ViewerApp: fetch snapshot and detail routes
ViewerApp-->>Browser: return read-only data and previews
Browser->>ViewerApp: poll for snapshot revisions
ViewerApp-->>Browser: return updated snapshot or warning
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (11)
src/evolve/viewer/static/viewer-ui.js (2)
8-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
generationKeydiscards a third path segment.
text.split('-', 2)returns at most two parts. For an id such as1-2-3the value becomes["1", "2"]and the trailing-3is lost. Two ids that differ only after the second segment then compare as equal on the numeric keys, and only thelocaleCompareon the full text separates them. That still gives a stable order, so this is a naming and clarity concern rather than a defect. Add a short comment that states the supported id shape.🤖 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 `@src/evolve/viewer/static/viewer-ui.js` around lines 8 - 13, Add a short comment immediately above generationKey documenting that supported IDs use at most two hyphen-separated segments; leave the existing parsing and comparison logic unchanged.
50-55: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
snapshotRevisionserializes the whole snapshot on every poll.The function returns the full JSON string, and
app.jsstores it instate.revisionand compares strings every 3 seconds. For an experiment with many generations this allocates and retains a large string on each tick. A hash of the serialized value would keep the comparison cheap and the retained state small.🤖 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 `@src/evolve/viewer/static/viewer-ui.js` around lines 50 - 55, Update snapshotRevision to hash the normalized snapshot serialization instead of returning the full JSON string, while preserving removal of experiment.updated_at and deterministic revision comparisons used by app.js. Return a compact, stable hash value suitable for storing in state.revision.src/evolve/viewer/models.py (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare
pydanticas a direct dependency.This module imports
pydanticdirectly, butpyproject.tomllists onlyfastapi,harbor,python-dotenv,PyYAML,typer, anduvicorn. The package resolves today only because FastAPI pulls it in. Add an explicitpydantic>=2requirement so a FastAPI change cannot break the viewer contracts.🤖 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 `@src/evolve/viewer/models.py` at line 8, Declare pydantic>=2 as a direct project dependency in pyproject.toml, alongside the existing runtime dependencies used by the viewer models module. Keep the direct BaseModel and Field imports in models.py unchanged.src/evolve/viewer/app.py (1)
247-255: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
_select_bindable_portleaves a race beforeuvicorn.runbinds.The function binds a probe socket, closes it, and returns the port. Another process can take the port in the gap, and
uvicorn.runthen fails with an unhandledOSError. For a local tool this is acceptable, but the error message is unclear. Catch the bind failure inrun_viewerand retry the next candidate port, or report a clear message.🤖 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 `@src/evolve/viewer/app.py` around lines 247 - 255, Update run_viewer to handle OSError from uvicorn.run after _select_bindable_port returns a candidate, retrying with the next available port when possible; if no candidate succeeds, raise or report a clear viewer startup error that includes the bind failure details.src/evolve/viewer/static/app.js (2)
108-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplaying the champion step with synthetic clicks is fragile.
The loop clicks
[data-champion-next]once per saved step. The restore therefore depends on the click handler, on the button existing, and on the button not being disabled. It also runs one fullrenderSelectionper step, so restoring step 20 renders 20 times.Expose a step setter on the replay controller and call it once with the saved index.
🤖 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 `@src/evolve/viewer/static/app.js` around lines 108 - 110, Replace the synthetic-click loop in the replay restoration logic with a step setter exposed by the replay controller, invoking it once with saved.championStep. Ensure the setter updates the controller’s current step and performs the necessary single renderSelection call without depending on the [data-champion-next] button or click-handler state.
816-817: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPolling continues while the tab is hidden.
setInterval(refresh, 3000)runs for the lifetime of the page. Each tick refreshes the workspace on the server, which re-reads the archive, re-federates Harbor jobs, and rebuilds the snapshot. A background tab keeps that work going for hours.Pause the timer on
visibilitychangeand refresh once when the page becomes visible again.♻️ Proposed refactor
await refresh(); -state.timer = window.setInterval(refresh, 3000); +const startPolling = () => { + if (state.timer == null) state.timer = window.setInterval(refresh, 3000); +}; +const stopPolling = () => { + if (state.timer != null) window.clearInterval(state.timer); + state.timer = null; +}; +document.addEventListener('visibilitychange', () => { + if (document.hidden) stopPolling(); + else { void refresh(); startPolling(); } +}); +startPolling();🤖 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 `@src/evolve/viewer/static/app.js` around lines 816 - 817, Update the polling setup around refresh and state.timer to stop scheduling refreshes while document.visibilityState is hidden. Add a visibilitychange handler that clears the timer when hidden and restarts the 3-second interval plus performs one immediate refresh when the page becomes visible, while preserving the initial refresh behavior.src/evolve/viewer/harbor_bridge.py (1)
40-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRe-entering the bridge after
__exit__is allowed, but a second__enter__while active is silently ignored.
__enter__returns the same root when_tempdiris set.app.pycalls__enter__outside the lifespan and__exit__inside it. If a caller passes an already-entered bridge, the single__exit__still destroys the root that the caller owns. Consider reference counting or documenting thatcreate_viewer_apptakes ownership of the supplied bridge.🤖 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 `@src/evolve/viewer/harbor_bridge.py` around lines 40 - 49, Update HarborBridge.__enter__ and __exit__ to track active ownership depth, so repeated __enter__ calls are balanced by matching __exit__ calls and cleanup occurs only after the final exit. Preserve the existing root across re-entry and ensure create_viewer_app’s lifecycle cannot destroy a root still owned by its caller.src/evolve/cli.py (1)
397-401: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate the workspace directory before launching the viewer.
viewpasses an arbitraryPathintoWorkspaceReader, so a typo reachesevolve.yaml/archive.jsonlchecks. Use Typer’s path validator so the CLI rejects non-directory or missing workspace paths before server startup.♻️ Proposed refactor
- workspace: Path = typer.Argument(Path(".")), + workspace: Path = typer.Argument(Path("."), exists=True, file_okay=False, dir_okay=True),🤖 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 `@src/evolve/cli.py` around lines 397 - 401, Update the workspace parameter in the view command to use Typer’s directory path validation, requiring the path to exist and be a directory before WorkspaceReader or server startup runs. Preserve the existing default workspace value and host/port behavior.src/evolve/viewer/static/vendor/LICENSE.highlight.js (1)
1-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude this license file from Biome parsing.
The filename ends with
.js, so Biome parses the license text as JavaScript and emits many parse errors. Add the vendor directory to the Biomeignorelist, or rename the file toLICENSE.highlight-js.txtand update the reference insrc/evolve/viewer/static/vendor/THIRD_PARTY_NOTICES.mdline 17. Keep the license text itself unchanged.🤖 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 `@src/evolve/viewer/static/vendor/LICENSE.highlight.js` around lines 1 - 29, Exclude the vendor license from Biome parsing by adding the vendor directory to Biome’s ignore configuration, or rename LICENSE.highlight.js to LICENSE.highlight-js.txt and update its reference in THIRD_PARTY_NOTICES.md. Preserve the license text unchanged.Source: Linters/SAST tools
tests/frontend/viewer-ui.test.mjs (1)
69-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting behavior instead of exact SVG geometry.
x1="78.8"andheight="136"bind the test to the current chart layout. A padding or viewBox change breaks the test without any behavior regression. Assert the selected point, the guide line, and the tooltip presence, and leave coordinate values out.🤖 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 `@tests/frontend/viewer-ui.test.mjs` around lines 69 - 72, Update the assertions in the viewer UI test to verify behavior rather than exact SVG geometry: retain checks for the selected trend point, guide line, and tooltip presence, but remove coordinate- and dimension-specific expectations such as x1/x2 values and rect height.tests/test_viewer_archive.py (1)
21-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
RECEIPT_CERTIFIED_FIELDinstead of repeating the literal.If the constant value changes in
src/evolve/archive.py, this test still passes but stops exercising receipt projection. Import the constant to keep the test bound to the production field name.♻️ Proposed change
-from evolve.archive import merge_events, merged_rows +from evolve.archive import RECEIPT_CERTIFIED_FIELD, merge_events, merged_rows- events = [{"genid": "1", "parent": "0", "_evolve_receipt_certified": True}] + events = [{"genid": "1", "parent": "0", RECEIPT_CERTIFIED_FIELD: True}]🤖 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 `@tests/test_viewer_archive.py` around lines 21 - 28, Update test_merge_events_keeps_input_immutable to import and use RECEIPT_CERTIFIED_FIELD from the production archive module instead of the literal "_evolve_receipt_certified" when constructing events, keeping the test bound to the production field name.
🤖 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 `@docs/guides/experiment-viewer.md`:
- Around line 13-14: Update the experiment-root description in the guide so only
evolve.yaml and archive.jsonl are presented as required inputs; describe runs/
as the usual generated layout or optional directory, consistent with
_validate_workspace() and its conditional discovery behavior.
- Around line 75-78: Update the viewer documentation to acknowledge that the
Harbor job index normally uses hard links but may fall back to copying files
into another temporary directory when links are unavailable. Preserve the
existing cleanup and source-file behavior description without claiming the index
never copies contents.
In `@src/evolve/viewer/app.py`:
- Around line 118-143: Update both git diff invocations in generation_diff—the
name-only discovery call and the final expanded diff call—to pass an appropriate
timeout argument to git. Keep the existing diff arguments and HTTPException
handling unchanged, ensuring either subprocess cannot block the viewer request
indefinitely.
- Around line 222-224: Add a small public root accessor to HarborBridge and
update the app setup around create_harbor_app to use it instead of
_require_root. Replace positional route slicing with a route-path filter that
excludes /openapi.json, Harbor documentation paths, and the catch-all
viewer-shell route while retaining all other Harbor routes.
- Around line 66-74: Update enforce_read_only to explicitly identify and block
unlisted Harbor GET routes, or route all Harbor endpoints through a separate
read-only guard. Ensure only documented read-only Harbor routes pass through
while preserving the existing method and action-path restrictions.
In `@src/evolve/viewer/harbor_bridge.py`:
- Line 171: Run Ruff formatting across the repository with `uv run ruff format
.`; this must reformat the long `parts` list comprehension and the `provider,
model` tuple assignment in `src/evolve/viewer/harbor_bridge.py` (lines 171 and
288), plus the wrapped `lambda` passed to `monkeypatch.setattr` in
`tests/test_viewer_cli.py` (lines 26-31).
- Around line 71-81: Update the refresh loop over desired.values() to always
rebuild each job’s temporary .{name}.next directory and atomically replace
destination, removing the destination.exists() guard. Preserve the existing
_remove_path, shutil.copytree, and os.replace staging flow so newly added trial
files are included on every refresh; keep cleanup of entries not in expected
unchanged.
In `@src/evolve/viewer/reader.py`:
- Around line 54-55: Rename the archive helper _eval_receipts to
read_eval_receipts, update its internal caller and export it from archive.py. In
the viewer reader flow, replace the raw receipts_path.read_text().splitlines()
parsing with read_eval_receipts so _has_evaluation_provenance and downstream
certification use the shared stripping and blank-line filtering behavior.
In `@src/evolve/viewer/static/app.js`:
- Around line 376-382: Update diffFileTabs so every tab button includes
aria-controls="artifact-preview", and update the `#artifact-preview` container to
role="tabpanel" with aria-labelledby referencing the active tab. Keep the
existing keyboard navigation and tab selection behavior unchanged, including the
alternate markup around the other referenced tab-rendering block.
- Around line 384-393: Update loadArtifact and the surrounding artifact-cache
flow to associate cached entries with the current snapshot revision,
invalidating or replacing entries when the revision changes so renderArtifact
displays refreshed content. Also bound state.artifactCache to a fixed maximum
size, evicting older entries when the limit is exceeded while preserving cache
reuse within the same revision.
In `@src/evolve/viewer/static/styles.css`:
- Around line 19-23: Insert an empty line in the :root block between the
--shadow custom property and the font-family declaration to satisfy Stylelint’s
declaration-empty-line-before rule.
- Around line 212-219: Update the .trend-point focus styling to preserve a
visible, high-contrast keyboard focus outline instead of removing it with
outline: none. Keep the existing .trend-dot focus fill and shadow behavior,
while using an appropriate outline style for focusable trend points.
In `@src/evolve/viewer/static/viewer-ui.js`:
- Around line 72-94: Update scoreAxis to derive min/max from finite numeric
scores without clamping values to [0, 1]; retain the existing [0, 1] domain only
when no valid scores exist. Update the y() mapping near the chart rendering
logic to use the derived domain directly and remove its [0, 1] clamping so raw
rewards and percentages retain their relative positions.
- Line 142: Replace the Array.prototype.toSorted call in the surrounding
generation-list flow with a copied-array sort using the existing
compareGenerationIds comparator, preserving the current ordering while
supporting older runtimes.
In `@tests/test_viewer_app.py`:
- Around line 187-244: Refactor
test_frontend_has_required_navigation_and_refresh_contract to retain only
user-visible HTML labels, branding, asset parity, and other stable contract
assertions. Remove exact app.js/styles.css implementation-text checks, including
the “3000” substring and expressions such as panel.classList.toggle,
event-listener wiring, option defaults, and global-result conditions. Add
equivalent behavioral coverage in tests/frontend/viewer-ui.test.mjs using its
existing rendering utilities, including validation of the refresh interval
behavior rather than searching source text.
- Around line 58-87: The git-backed test setup in
test_generation_diff_adds_bounded_parent_context is not hermetic because git may
use repository-specific signing or an environment-dependent default branch.
Update the shared git helper used by this test to inherit configuration
disabling commit signing and setting a deterministic initial branch, or
explicitly declare the required git configuration in the test setup before the
first git invocation.
In `@tests/test_viewer_harbor_bridge.py`:
- Around line 33-38: Run uv run ruff format . and apply the resulting formatting
changes to tests/test_viewer_harbor_bridge.py, including the wrapped
bridge.refresh call chains and all listed ranges, without changing behavior.
---
Nitpick comments:
In `@src/evolve/cli.py`:
- Around line 397-401: Update the workspace parameter in the view command to use
Typer’s directory path validation, requiring the path to exist and be a
directory before WorkspaceReader or server startup runs. Preserve the existing
default workspace value and host/port behavior.
In `@src/evolve/viewer/app.py`:
- Around line 247-255: Update run_viewer to handle OSError from uvicorn.run
after _select_bindable_port returns a candidate, retrying with the next
available port when possible; if no candidate succeeds, raise or report a clear
viewer startup error that includes the bind failure details.
In `@src/evolve/viewer/harbor_bridge.py`:
- Around line 40-49: Update HarborBridge.__enter__ and __exit__ to track active
ownership depth, so repeated __enter__ calls are balanced by matching __exit__
calls and cleanup occurs only after the final exit. Preserve the existing root
across re-entry and ensure create_viewer_app’s lifecycle cannot destroy a root
still owned by its caller.
In `@src/evolve/viewer/models.py`:
- Line 8: Declare pydantic>=2 as a direct project dependency in pyproject.toml,
alongside the existing runtime dependencies used by the viewer models module.
Keep the direct BaseModel and Field imports in models.py unchanged.
In `@src/evolve/viewer/static/app.js`:
- Around line 108-110: Replace the synthetic-click loop in the replay
restoration logic with a step setter exposed by the replay controller, invoking
it once with saved.championStep. Ensure the setter updates the controller’s
current step and performs the necessary single renderSelection call without
depending on the [data-champion-next] button or click-handler state.
- Around line 816-817: Update the polling setup around refresh and state.timer
to stop scheduling refreshes while document.visibilityState is hidden. Add a
visibilitychange handler that clears the timer when hidden and restarts the
3-second interval plus performs one immediate refresh when the page becomes
visible, while preserving the initial refresh behavior.
In `@src/evolve/viewer/static/vendor/LICENSE.highlight.js`:
- Around line 1-29: Exclude the vendor license from Biome parsing by adding the
vendor directory to Biome’s ignore configuration, or rename LICENSE.highlight.js
to LICENSE.highlight-js.txt and update its reference in THIRD_PARTY_NOTICES.md.
Preserve the license text unchanged.
In `@src/evolve/viewer/static/viewer-ui.js`:
- Around line 8-13: Add a short comment immediately above generationKey
documenting that supported IDs use at most two hyphen-separated segments; leave
the existing parsing and comparison logic unchanged.
- Around line 50-55: Update snapshotRevision to hash the normalized snapshot
serialization instead of returning the full JSON string, while preserving
removal of experiment.updated_at and deterministic revision comparisons used by
app.js. Return a compact, stable hash value suitable for storing in
state.revision.
In `@tests/frontend/viewer-ui.test.mjs`:
- Around line 69-72: Update the assertions in the viewer UI test to verify
behavior rather than exact SVG geometry: retain checks for the selected trend
point, guide line, and tooltip presence, but remove coordinate- and
dimension-specific expectations such as x1/x2 values and rect height.
In `@tests/test_viewer_archive.py`:
- Around line 21-28: Update test_merge_events_keeps_input_immutable to import
and use RECEIPT_CERTIFIED_FIELD from the production archive module instead of
the literal "_evolve_receipt_certified" when constructing events, keeping the
test bound to the production field name.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9d499cd-7046-4c5b-b881-f11b8d9a6d71
⛔ Files ignored due to path filters (6)
src/evolve/viewer/static/evolve-mark.svgis excluded by!**/*.svgsrc/evolve/viewer/static/vendor/diff2html.min.cssis excluded by!**/*.min.csssrc/evolve/viewer/static/vendor/diff2html.min.jsis excluded by!**/*.min.jssrc/evolve/viewer/static/vendor/highlight-github.min.cssis excluded by!**/*.min.csssrc/evolve/viewer/static/vendor/highlight.min.jsis excluded by!**/*.min.jsuv.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
ARCHITECTURE.mdREADME.mddocs/guides/experiment-viewer.mdmkdocs.ymlpyproject.tomlsrc/evolve/archive.pysrc/evolve/cli.pysrc/evolve/viewer/__init__.pysrc/evolve/viewer/app.pysrc/evolve/viewer/harbor_bridge.pysrc/evolve/viewer/models.pysrc/evolve/viewer/reader.pysrc/evolve/viewer/snapshot.pysrc/evolve/viewer/static/app.jssrc/evolve/viewer/static/index.htmlsrc/evolve/viewer/static/styles.csssrc/evolve/viewer/static/vendor/LICENSE.diff2htmlsrc/evolve/viewer/static/vendor/LICENSE.highlight.jssrc/evolve/viewer/static/vendor/THIRD_PARTY_NOTICES.mdsrc/evolve/viewer/static/viewer-ui.jstests/frontend/viewer-ui.test.mjstests/test_viewer_app.pytests/test_viewer_archive.pytests/test_viewer_cli.pytests/test_viewer_harbor_bridge.pytests/test_viewer_reader.pytests/test_viewer_snapshot.py
| Pass the experiment root—the directory containing `evolve.yaml`, | ||
| `archive.jsonl`, and `runs/`: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not present runs/ as a required directory.
In src/evolve/viewer/reader.py:44-173, _validate_workspace() requires evolve.yaml and archive.jsonl; runs/ discovery is conditional. Describe runs/ as the usual generated layout, not as a required input.
Suggested wording
-Pass the experiment root—the directory containing `evolve.yaml`,
-`archive.jsonl`, and `runs/`:
+Pass the experiment root—the directory containing `evolve.yaml` and
+`archive.jsonl`. Generated workspaces normally also contain `runs/`:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Pass the experiment root—the directory containing `evolve.yaml`, | |
| `archive.jsonl`, and `runs/`: | |
| Pass the experiment root—the directory containing `evolve.yaml` and | |
| `archive.jsonl`. Generated workspaces normally also contain `runs/`: |
🤖 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 `@docs/guides/experiment-viewer.md` around lines 13 - 14, Update the
experiment-root description in the guide so only evolve.yaml and archive.jsonl
are presented as required inputs; describe runs/ as the usual generated layout
or optional directory, consistent with _validate_workspace() and its conditional
discovery behavior.
| The viewer builds a disposable hard-link index for referenced Harbor jobs. | ||
| This lets Harbor enforce its normal path-containment checks without copying the | ||
| job contents. The index is removed when the viewer exits; source experiment | ||
| files are not modified. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the hard-link fallback.
In src/evolve/viewer/harbor_bridge.py:32-99, the bridge uses copy_function=_link_or_copy and can fall back to another temporary directory. Hard links are not guaranteed. The claim that the index works “without copying” is false when the fallback copies files.
Suggested wording
-The viewer builds a disposable hard-link index for referenced Harbor jobs.
-This lets Harbor enforce its normal path-containment checks without copying the
-job contents. The index is removed when the viewer exits; source experiment
-files are not modified.
+The viewer builds a disposable index for referenced Harbor jobs. It uses hard
+links when possible and copies files when hard links are unavailable. This lets
+Harbor enforce its normal path-containment checks. The index is removed when
+the viewer exits; source experiment files are not modified.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The viewer builds a disposable hard-link index for referenced Harbor jobs. | |
| This lets Harbor enforce its normal path-containment checks without copying the | |
| job contents. The index is removed when the viewer exits; source experiment | |
| files are not modified. | |
| The viewer builds a disposable index for referenced Harbor jobs. It uses hard | |
| links when possible and copies files when hard links are unavailable. This lets | |
| Harbor enforce its normal path-containment checks. The index is removed when | |
| the viewer exits; source experiment files are not modified. |
🤖 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 `@docs/guides/experiment-viewer.md` around lines 75 - 78, Update the viewer
documentation to acknowledge that the Harbor job index normally uses hard links
but may fall back to copying files into another temporary directory when links
are unavailable. Preserve the existing cleanup and source-file behavior
description without claiming the index never copies contents.
| @app.middleware("http") | ||
| async def enforce_read_only(request: Request, call_next): | ||
| path = request.url.path | ||
| if request.method not in {"GET", "HEAD", "OPTIONS"} or _is_action_path(path): | ||
| return Response(status_code=405, headers={"Allow": "GET, HEAD, OPTIONS"}) | ||
| response = await call_next(request) | ||
| if path.startswith(("/evolve-assets/", "/generations", "/trials", "/artifacts")) or path == "/": | ||
| response.headers["Cache-Control"] = "no-store" | ||
| return response |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List Harbor viewer route paths and methods to compare against the allowed set.
set -euo pipefail
python - <<'PY'
import importlib.util, sys
if importlib.util.find_spec("harbor") is None:
print("harbor is not installed in this sandbox; enumerate harbor.viewer.server routes manually")
sys.exit(0)
import tempfile
from harbor.viewer.server import create_app
with tempfile.TemporaryDirectory() as root:
app = create_app(root)
for route in app.router.routes:
print(sorted(getattr(route, "methods", []) or []), getattr(route, "path", route))
PYRepository: simple-agent-lab/EvolveX
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate app.py and relevant imports =="
fd -a 'app\.py$' . | sed 's#^\./##' | while read -r f; do
if grep -q "_is_action_path\|enforce_read_only" "$f"; then
echo "--- $f"
nl -ba "$f" | sed -n '1,140p'
fi
done
echo
echo "== search for Harbor viewer deps/version references =="
rg -n "harbor|github\.com|pypi|pip|requirements|pyproject|0\.18" -S . --glob '!*.lock' --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -200Repository: simple-agent-lab/EvolveX
Length of output: 291
🌐 Web query:
Harbor viewer server 0.18.0 routes _is_action_path GET endpoints create_app
💡 Result:
The identifier is_action_path is not a standard component of the Harbor viewer server API or its routing mechanisms in the context of the harbor-framework (an evaluation framework for agentic workflows) [1][2]. Based on the architecture of the Harbor viewer server (typically implemented as a FastAPI application in src/harbor/viewer/server.py), the server uses standard decorators like @app.get to define API endpoints [3]. The viewer's routing and application initialization are handled by a create_app factory function, which configures REST endpoints such as GET /api/jobs, GET /api/jobs/{job_id}/trials, and GET /api/trials/{trial_id}/trajectory [1][3]. The term is_action_path appears to be unrelated to the Harbor viewer server's codebase or routing logic [4][5][6]. It is possible that this term arises from a different framework or a misunderstanding of internal utility functions. For context, the Harbor viewer's frontend is a React application that defines routes via an app/routes.ts file, and its backend provides data via the aforementioned REST API, neither of which involves an is_action_path route configuration or endpoint [7][6].
Citations:
- 1: https://mintlify.wiki/harbor-framework/harbor/cli/view
- 2: Add trajectory viewer harbor-framework/harbor#143
- 3: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/server.py
- 4: harbor-framework/harbor@c1f6e4f
- 5: harbor-framework/harbor@fcc0205
- 6: harbor-framework/harbor@abb0773
- 7: https://github.com/harbor-framework/harbor/blob/main/apps/viewer/CLAUDE.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect viewer app.py around middleware =="
awk '{printf "%6d\t%s\n", NR, $0}' /home/jailuser/git/src/evolve/viewer/app.py | sed -n '1,130p'
echo
echo "== inspect viewer routes in installed package source, if available =="
python3 - <<'PY'
import importlib.util, inspect, site, pathlib, json, urllib.request
print("Python paths:")
for p in site.getsitepackages() + ([site.getusersitepackages()] if site.getusersitepackages() else []):
print(p)
print()
spec = importlib.util.find_spec("harbor")
print("harbor spec:", spec)
if spec and spec.origin:
root = pathlib.Path(spec.origin).parent
print("harbor root files (viewer/server candidates):")
candidates = []
for path in root.rglob("*.py"):
if "viewer" in path.parts or path.name == "server.py":
candidates.append(path)
for path in sorted(candidates)[:100]:
print(path)
user_paths = [
pathlib.Path(spec.origin).parents[2] / "apps" / "viewer" / "server.py",
pathlib.Path(spec.origin).parents[2] / "viewer" / "server.py",
]
for path in user_paths:
if path.exists():
print("\n---", path)
lines = path.read_text().splitlines()
for i, line in enumerate(lines[:260], 1):
if i <= 260 and ("`@app`." in line or "router." in line or "/api" in line or "/auth" in line or "def " in line.split("#")[0]):
print(f"{i:4d}\t{line}")
PY
echo
echo "== package metadata if available =="
python3 - <<'PY'
try:
import importlib.metadata as im
print("harbor version:", im.version("harbor") or "unknown")
print("files:", [str(p) for p in im.files("harbor")[:50]])
except Exception as e:
print("metadata lookup failed:", repr(e))
PYRepository: simple-agent-lab/EvolveX
Length of output: 6713
🌐 Web query:
site:github.com/bird-bench/livesqlbench harbor.viewer.server GET endpoints
💡 Result:
The harbor.viewer.server module is part of the LiveSQLBench project and provides a web-based interface for browsing jobs and trajectories [1][2]. It exposes several GET endpoints to retrieve information about jobs and configurations, as defined in its implementation [3]. The identified GET endpoints include: - /api/config: Retrieves the viewer configuration (e.g., jobs directory) [3]. - /api/jobs/{job_name}/summary: Retrieves a summary of a specific job [3]. - /api/compare: Retrieves comparison grid data for multiple jobs [3]. Additionally, the server provides endpoints with query parameters for listing and filtering data: - A paginated endpoint (path inferred from the implementation) for listing jobs or trials, which supports filtering by query (q), agent, provider, and sorting options (e.g., sort_by, sort_order) [3]. The server is built using FastAPI, and its core functionality relies on a JobScanner that reads job data, configurations, and results directly from the file system [3][4].
Citations:
- 1: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/__init__.py
- 2: https://github.com/bird-bench/livesqlbench/tree/main/LiveSQLBench-CLI/src/harbor/viewer
- 3: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/server.py
- 4: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/scanner.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== _is_action_path implementation/usages =="
grep -n "_is_action_path\|_ACTION_PATH" /home/jailuser/git/src/evolve/viewer/app.py
echo
echo "== app.py tail until likely end =="
awk '{printf "%6d\t%s\n", NR, $0}' /home/jailuser/git/src/evolve/viewer/app.py | sed -n '130,260p'
echo
echo "== read LiveSQLBench Harbor viewer source from GitHub without installing harbor =="
python3 - <<'PY'
from urllib.request import urlopen
urls = [
"https://raw.githubusercontent.com/bird-bench/livesqlbench/main/LiveSQLBench-CLI/src/harbor/viewer/server.py",
"https://raw.githubusercontent.com/bird-bench/livesqlbench/main/LiveSQLBench-CLI/src/harbor/viewer/__init__.py",
"https://raw.githubusercontent.com/bird-bench/livesqlbench/main/LiveSQLBench-CLI/pyproject.toml",
]
for url in urls:
print(f"\n--- {url}")
with urlopen(url, timeout=20) as r:
text = r.read().decode("utf-8")
for i, line in enumerate(text.splitlines(), 1):
if i <= 280 and ("`@app`" in line or "router." in line or "create_app" in line or "/api" in line or "`@app.include_router`" in line or "/auth" in line or "def " in line or "def _is_action_path" in line):
print(f"{i:4d}\t{line}")
PYRepository: simple-agent-lab/EvolveX
Length of output: 9138
🌐 Web query:
"harbor.viewer.server" "create_app" "@app.get" "@app.post" jobs upload
💡 Result:
In the context of the Harbor framework, the terms identified relate to the internal implementation of the interactive job viewer and CLI-based data management commands [1][2]. 1. harbor.viewer.server create_app: This is a factory function used to instantiate a FastAPI application designed to serve the Harbor job/task viewer [3][4][5]. It typically takes the path to a jobs directory as an argument and configures the necessary API routes for browsing jobs, trials, and agent trajectories [3][1][6]. 2. @app.get / @app.post: These are standard FastAPI decorators used within the viewer server to define API endpoints [3][7]. The viewer utilizes these to expose data such as job configurations, summaries, and specific trajectory details to the React-based frontend [3][1][6]. 3. jobs upload: This refers to the CLI command used to upload local job results to the Harbor Hub platform [2]. Running harbor upload streams local trial data, allowing users to create shareable links for their evaluation results [2]. In summary, create_app and the associated @app routes form the backend infrastructure for the local interactive viewer, while jobs upload is a distinct CLI operation for cloud-based storage and sharing of evaluation outcomes [1][2].
Citations:
- 1: https://mintlify.wiki/harbor-framework/harbor/cli/view
- 2: https://www.harborframework.com/docs/sharing/jobs
- 3: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/server.py
- 4: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/__init__.py
- 5: harbor-framework/harbor@fcc0205
- 6: https://github.com/harbor-framework/harbor/blob/main/apps/viewer/CLAUDE.md
- 7: https://github.com/pymc-labs/decision-lab/blob/1d2fd2f3/dlab/viewer/server.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== _is_action_path implementation =="
awk '{printf "%6d\t%s\n", NR, $0}' /home/jailuser/git/src/evolve/viewer/app.py | sed -n '258,270p'
echo
echo "== search GitHub liveSQLBench branch without TLS verification =="
python3 - <<'PY'
import ssl, urllib.request, re
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
urls = {
"server.py": "https://raw.githubusercontent.com/bird-bench/livesqlbench/main/LiveSQLBench-CLI/src/harbor/viewer/server.py",
"__init__.py": "https://raw.githubusercontent.com/bird-bench/livesqlbench/main/LiveSQLBench-CLI/src/harbor/viewer/__init__.py",
"pyproject.toml": "https://raw.githubusercontent.com/bird-bench/livesqlbench/main/LiveSQLBench-CLI/pyproject.toml",
}
for name, url in urls.items():
print(f"\n--- {name}")
with urllib.request.urlopen(url, context=ctx, timeout=30) as r:
text = r.read().decode("utf-8")
lines = text.splitlines()
for i, line in enumerate(lines, 1):
if i <= 320 and re.search(r'(`@app`\.|include_router|create_app|def _|/api|/auth|upload|POST|PUT|DELETE|Patch|Patch)', line):
print(f"{i:4d}\t{line}")
PYRepository: simple-agent-lab/EvolveX
Length of output: 1335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== install harbor package without running repo code =="
python3 - <<'PY'
import subprocess, sys, json, os, tempfile
env = os.environ.copy()
env["PIP_NO_CACHE_DIR"] = "1"
env["PIP_DISABLE_PIP_VERSION_CHECK"] = "1"
# Avoid running potentially existing repo scripts; only inspect package from a temp location via pip wheel metadata/source.
print("== available package candidates ==")
try:
import importlib.metadata as im
print("harbor installed:", im.distribution("harbor").metadata.get("Name","?"), im.distribution("harbor").metadata.get("Version","?"))
except Exception as exc:
print("harbor not installed:", repr(exc))
PY
echo
echo "== fetch harbor 0.18.0 package metadata/source listing =="
python3 - <<'PY'
import subprocess, tarfile, io, urllib.request, ssl, tempfile
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
url = "https://files.pythonhosted.org/packages/5f/ba/e15f6e2469434109a22110c6c28b09126cb1d187692775655222eb8405/harbor-0.18.0.tar.gz"
print("fetching", url)
data = urllib.request.urlopen(url, context=ctx, timeout=45).read()
with tempfile.NamedTemporaryFile(suffix=".tar.gz") as f:
f.write(data)
f.flush()
with tarfile.open(f.name, "r:gz") as tf:
names = [n for n in tf.getnames() if "viewer" in n.lower() or n in {"pyproject.toml", "setup.py"}]
for name in sorted(names)[:80]:
print(name)
print("\n--- viewer/server.py relevant lines ---")
src = tf.extractfile("harbor-viewer-0.18.0/src/harbor/viewer/server.py").read().decode("utf-8", "replace").splitlines()
for i, line in enumerate(src, 1):
if i <= 360 and ("`@app`." in line or "app.include_router" in line or "/api" in line or "/auth" in line or "def " in line or "Patch" in line or "POST" in line or "PUT" in line or "DELETE" in line or "upload" in line):
print(f"{i:4d}\t{line}")
print("\n--- harbor/viewer source file line counts ---")
for n in tf.getnames():
if n.startswith("harbor-viewer-0.18.0/src/harbor/viewer/") and n.endswith(".py"):
s = len(tf.extractfile(n).read().decode("utf-8", "replace").splitlines())
print(f"{s:4d}\t{n}")
PYRepository: simple-agent-lab/EvolveX
Length of output: 1285
🌐 Web query:
pythonhosted.org harbor-viewer 0.18.0.tar.gz
💡 Result:
There is no Python package named harbor-viewer. The term refers to a component within the Harbor framework, which is a tool for evaluating and optimizing AI agents [1][2][3]. Harbor is a Python-based framework [3][4] distributed on PyPI as the package harbor [5]. Within this framework, there is a submodule or feature called harbor.viewer [6], which provides a web-based user interface for browsing and inspecting agent trajectories, evaluation jobs, and trial results [1][2][7]. The file name harbor-viewer 0.18.0.tar.gz does not correspond to a standalone package distribution. It is likely a misinterpretation of: 1. The harbor version 0.18.0 release, which is the official PyPI package for the Harbor framework [4]. 2. Internal directory structures or build artifacts related to the viewer component, such as the source code located at src/harbor/viewer/ in the main repository [2][7][6]. If you are looking for the Harbor framework itself, you can install it via the official PyPI package using uv tool install harbor or pip install harbor [8]. The viewer interface is then accessed via the harbor view command [1][7].
Citations:
- 1: https://mintlify.wiki/harbor-framework/harbor/cli/view
- 2: https://github.com/harbor-framework/harbor/blob/main/apps/viewer/CLAUDE.md
- 3: https://context7.com/harbor-framework/harbor
- 4: https://pypi.org/project/harbor/0.18.0/
- 5: https://pypi.org/project/harbor/
- 6: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/__init__.py
- 7: harbor-framework/harbor@abb0773
- 8: https://www.harborframework.com/docs/getting-started
🌐 Web query:
harbor 0.18.0 source viewer server.py /api/run /api/auth upload
💡 Result:
In the context of the Harbor framework, specifically version 0.18.0, the terms mentioned relate to task execution, artifact management, and API orchestration rather than a single source-viewer server file [1][2]. 1. Harbor 0.18.0 and API Upload: In Harbor 0.18.0, the command-line interface (CLI) includes an upload functionality used to transfer job directories to the Harbor hub (e.g., harbor upload jobs/rubric-review --share-org harbor -y) [1][2]. This version of Harbor does not support an --upload flag within the exec command itself, necessitating a separate upload call following task execution [1][2]. 2. server.py and API endpoints: The filenames and paths cited (e.g., server.py, /api/run, /api/auth) are common conventions for Python-based server implementations used within various projects that interact with or are built using the Harbor framework [3][4][5][6]. For example: - server.py is frequently used to define FastMCP or standard HTTP servers that expose tools or endpoints [3][7][6]. - /api/auth is a standard path for authentication (often returning JWTs) in many web services and MCP servers [5][6][8]. - /api/run is often used for orchestration or task execution endpoints [5]. These elements are not part of a singular "source viewer" component in Harbor 0.18.0; rather, they appear in different contexts within the Harbor ecosystem, such as MCP server tutorials or automated benchmarking environments where developers create custom server.py files to manage tasks, auth, and artifact uploads [3][1][2].
Citations:
- 1: harbor-framework/terminal-bench@98b3f5b
- 2: https://github.com/harbor-framework/frontier-bench/blob/2d260bc7fb344dd13b1b920265e372b6dbc6a38d/.github/workflows/review.yml
- 3: https://www.harborframework.com/docs/tutorials/mcp-server-task
- 4: harbor-framework/harbor@3c8bd7f
- 5: https://github.com/voly-codes/voly/blob/main/openwiki/backend/entrypoints.md
- 6: https://github.com/WOOWTECH/woow_hermes_mcp_server
- 7: https://github.com/iflow-mcp/ibaic-cs-mcp/blob/main/server.py
- 8: https://github.com/ibaiC/CS-MCP/blob/main/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exact repo references to pinned Harbor version =="
git ls-files | xargs rg -n 'harbor|habor|viewer|0\.18|pyproject|requirements|lock|Pipfile|environment' -S || true
echo
echo "== pyproject dependency declarations =="
for f in pyproject.toml pyproject.lock poetry.lock uv.lock requirements.txt requirements*.txt Pipfile Pipfile.lock; do
[ -f "$f" ] && { echo "--- $f"; rg -n '^harbor|harbor-viewer|version|0\.18|@ ' "$f" -S || true; }
done
echo
echo "== liveSQLBench latest viewer source lines 150-360 via HTTPS with verify SSL errors accepted =="
python3 - <<'PY'
import urllib.request, ssl, re
ctx = ssl.create_default_context()
ctx.check_hostname = False
ctx.verify_mode = ssl.PEM_CERT_REQUIRED
ctx.verify_mode = ssl.CERT_NONE
url = "https://raw.githubusercontent.com/bird-bench/livesqlbench/main/LiveSQLBench-CLI/src/harbor/viewer/server.py"
print(url)
with urllib.request.urlopen(url, context=ctx, timeout=45) as r:
text = r.read().decode("utf-8")
lines = text.splitlines()
for i, line in enumerate(lines, 1):
if i < 150 or i > 360:
continue
if re.search(r'(`@app`\.|include_router|create_app|def _|/api|/auth|upload|POST|PUT|DELETE|Patch|Patch|methods=)', line):
print(f"{i:4d}\t{line}")
PYRepository: simple-agent-lab/EvolveX
Length of output: 50381
Document Harbor GET side effects or block every unlisted Harbor route.
The middleware only excludes _is_action_path; unlisted Harbor GET routes can still pass through despite the read-only guarantee. Add those routes explicitly or keep Harbor routes behind a separate lock unless they are read-only.
🤖 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 `@src/evolve/viewer/app.py` around lines 66 - 74, Update enforce_read_only to
explicitly identify and block unlisted Harbor GET routes, or route all Harbor
endpoints through a separate read-only guard. Ensure only documented read-only
Harbor routes pass through while preserving the existing method and action-path
restrictions.
| names = git( | ||
| workspace, | ||
| "diff", | ||
| "--name-only", | ||
| f"gen/{comparison_base}", | ||
| f"gen/{genid}", | ||
| "--", | ||
| "target", | ||
| check=False, | ||
| ) | ||
| paths = [path for path in names.stdout.splitlines() if path == "target" or path.startswith("target/")] | ||
| if not paths: | ||
| raise HTTPException(status_code=404, detail="expanded generation diff is unavailable") | ||
| result = git( | ||
| workspace, | ||
| "diff", | ||
| "--no-ext-diff", | ||
| f"--unified={context}", | ||
| f"gen/{comparison_base}", | ||
| f"gen/{genid}", | ||
| "--", | ||
| *paths, | ||
| check=False, | ||
| ) | ||
| if result.returncode != 0 or not result.stdout: | ||
| raise HTTPException(status_code=404, detail="expanded generation diff is unavailable") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the git helper for timeout and environment hardening.
set -euo pipefail
fd -t f 'git.py' src/evolve --exec cat -nRepository: simple-agent-lab/EvolveX
Length of output: 4597
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f 'app.py' src/evolve/viewer --exec sh -c 'echo "===== $1 ====="; wc -l "$1"; sed -n "1,220p" "$1"' sh {}
echo "===== calls to git helper ====="
rg -n "from evolve\.git import git|from evolve\.git import|def generation_diff|def .*(diff|generation)" src/evolve/viewer/app.py src/evolve -g '*.py' | head -80Repository: simple-agent-lab/EvolveX
Length of output: 10876
Add a timeout to the diff git calls.
evolve.git.git uses subprocess.run() without timeout, and generation_diff runs up to two git diff calls in a request thread. Add a timeout to these calls so a large repository, slow filesystem, or stuck credential prompt does not hold the viewer request thread indefinitely.
🤖 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 `@src/evolve/viewer/app.py` around lines 118 - 143, Update both git diff
invocations in generation_diff—the name-only discovery call and the final
expanded diff call—to pass an appropriate timeout argument to git. Keep the
existing diff arguments and HTTPException handling unchanged, ensuring either
subprocess cannot block the viewer request indefinitely.
| harbor_static = Path(next(iter(harbor_viewer.__path__))) / "static" | ||
| harbor_app = create_harbor_app(active_bridge._require_root(), static_dir=harbor_static) | ||
| app.router.routes.extend(harbor_app.router.routes[4:]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the Harbor app factory to confirm which leading routes the slice intends to drop.
set -euo pipefail
fd -t f 'server.py' -p 'harbor' || true
python - <<'PY'
import importlib.util, sys
if importlib.util.find_spec("harbor") is None:
print("harbor is not installed in this sandbox; inspect harbor.viewer.server.create_app manually")
sys.exit(0)
from harbor.viewer.server import create_app
import inspect
print(inspect.getsource(create_app))
PYRepository: simple-agent-lab/EvolveX
Length of output: 346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)(app\.py|server\.py)$|viewer|harbor' | head -200
echo "== app.py outline / relevant section =="
sed -n '1,280p' src/evolve/viewer/app.py 2>/dev/null || true
echo "== references to create_harbor_app / harbor_viewer =="
rg -n "create_harbor_app|harbor_viewer|docs_url|redoc_url|swagger_ui_oauth2_redirect_url|openapi" -S .Repository: simple-agent-lab/EvolveX
Length of output: 13799
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== harbor bridge relevant sections =="
sed -n '1,260p' src/evolve/viewer/harbor_bridge.py 2>/dev/null || true
echo "== pyproject/requirements harbor/fastapi/version constraints =="
for f in pyproject.toml requirements.txt setup.cfg setup.py uv.lock poetry.lock requirements-dev.txt; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n "harbor|fastapi|uvicorn|pydantic" "$f" -i || true
fi
done
echo "== public APIs on HarborBridge in repo =="
rg -n "class HarborBridge|def [A-Za-z_][A-Za-z0-9_]*\\(" src/evolve/viewer/harbor_bridge.py src/evolve/integrations/harbor -SRepository: simple-agent-lab/EvolveX
Length of output: 40930
🌐 Web query:
github python harbor viewer server create_app docs_url redoc_url FastAPI routes include in schema
💡 Result:
In FastAPI, you can control the documentation URLs and the visibility of your routes in the generated OpenAPI schema using specific parameters within the FastAPI application instance or when defining routes [1][2][3]. Docs and ReDoc URLs To configure or disable the documentation interfaces, use the docs_url and redoc_url parameters when initializing your FastAPI application [1][4]. - docs_url: The path for the Swagger UI documentation (default is /docs) [1][2]. - redoc_url: The path for the ReDoc documentation (default is /redoc) [1][2]. - Disabling: Set either parameter to None to disable the respective documentation interface [4][5]. Note that if openapi_url is set to None, both documentation UIs will be automatically disabled [1][6]. Example: from fastapi import FastAPI app = FastAPI(docs_url="/documentation", redoc_url=None) Including Routes in Schema The include_in_schema parameter controls whether a route or router appears in the generated OpenAPI documentation [1][3]. - Path Operations: You can set include_in_schema=False in a decorator to hide a specific endpoint from the generated schema [7][3]. @app.get("/hidden-route", include_in_schema=False) async def hidden_route: return {"message": "This is not in the docs"} - Routers: When using APIRouter, you can pass include_in_schema=False to the router constructor or the include_router method to hide all routes within that router from the schema [3]. router = APIRouter(include_in_schema=False) app.include_router(router) This parameter is a boolean that defaults to True [1][3]. When set to False, the path operation is still functional but will not be included in the OpenAPI JSON or the interactive documentation UIs [7][3].
Citations:
- 1: https://fastapi.tiangolo.com/reference/fastapi/
- 2: https://github.com/fastapi/fastapi/blob/master/fastapi%2Fapplications.py
- 3: https://fastapi.tiangolo.com/reference/apirouter/
- 4: https://fastapi.tiangolo.com/tutorial/metadata/
- 5: https://fastapi.tiangolo.com/tutorial/metadata/?h=docs_url
- 6: https://mintlify.wiki/fastapi/fastapi/advanced/conditional-openapi
- 7: https://fastapi.tiangolo.com/advanced/path-operation-advanced-configuration/
🌐 Web query:
python harbor viewer server.py create_app harbor 0.18.0 FastAPI routes
💡 Result:
In the Harbor framework (specifically version 0.18.0 and related ecosystem projects), the create_app function within server.py is a standard pattern used to initialize a FastAPI application [1]. This function typically configures the application instance, sets up middleware (such as CORS for local development), mounts static directories, and registers API routes for browsing jobs, trials, and artifacts [1][2]. Key aspects of the create_app function and FastAPI implementation in Harbor include: 1. Application Initialization: The function returns a FastAPI instance, often configured with a title (e.g., Harbor Viewer) and version [1]. 2. Middleware: To support local development and browser-based viewers, CORS middleware is commonly configured to allow all origins, methods, and headers [1]. 3. Route Configuration: - Configuration APIs: Endpoints like /api/config allow the frontend to retrieve viewer settings [1]. - Job/Trial Management: Routes are defined to scan directories, aggregate job summaries, and retrieve specific trial data or artifacts [1][2]. - Health Checks: The server often includes a /health endpoint used by the environment runner to verify that the server has successfully started [3]. - Static File Serving: If provided, the app mounts directories for static assets, fonts, and favicon.ico, and typically includes a catch-all route (/{path:path}) to serve a Single Page Application (SPA) index.html file [1]. 4. Version Compatibility: Harbor 0.18.0 specifically requires FastAPI >= 0.128.0 and uvicorn >= 0.38.0 [4]. In many Harbor-based projects, server.py acts as the backend verifier or viewer interface, running alongside agent-evaluated tasks [5][6]. When tasks are deployed, they often include a server.py that provides an API for the agent to interact with or for the platform to verify trial results [6][7]. Top results: [1], [4], [3]
Citations:
- 1: https://github.com/bird-bench/livesqlbench/blob/main/LiveSQLBench-CLI/src/harbor/viewer/server.py
- 2: harbor-framework/harbor@e6a884f
- 3: [singularity] server start health check hardcoded 60s is too short for fresh bootstrap harbor-framework/harbor#1713
- 4: https://pypi.org/project/harbor/0.18.0/
- 5: https://github.com/harbor-framework/frontier-bench/tree/main/tasks/ctr-optimization/environment/api
- 6: https://github.com/harbor-framework/frontier-bench/tree/main/tasks/vba-userform-port/solution
- 7: [WIP] Adapter: contractbench harbor-framework/harbor#1949
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository lock files for source location =="
rg -n 'harbor_framework|github.com/harbor-framework/harbor|files.pythonhosted.*harbor' uv.lock pyproject.toml -S || true
echo "== tests around harbor viewer routes =="
sed -n '1,180p' tests/test_viewer_harbor_bridge.py 2>/dev/null || true
echo "== any local app tests around harbor routes =="
rg -n "harbor|routes\\[|create_harbor_app|_require_root|docs_url|redoc_url" tests src tests/test_viewer*.py -S || trueRepository: simple-agent-lab/EvolveX
Length of output: 50381
Use a route-path filter instead of slicing Harbor routes.
HarborBridge no longer exposes _require_root, and harbor_app.router.routes[:4] depends on FastAPI’s default documentation routes being ordered in a fixed set. If that order changes, this can drop real Harbor API routes or the viewer schema from the merged router. Add a small public root accessor and extend routes by excluding /openapi.json, the Harbor docs paths, and any catch-all that duplicates the viewer shell.
🤖 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 `@src/evolve/viewer/app.py` around lines 222 - 224, Add a small public root
accessor to HarborBridge and update the app setup around create_harbor_app to
use it instead of _require_root. Replace positional route slicing with a
route-path filter that excludes /openapi.json, Harbor documentation paths, and
the catch-all viewer-shell route while retaining all other Harbor routes.
| export function scoreAxis(scores) { | ||
| const values = scores.map(Number).filter(Number.isFinite).map((value) => Math.max(0, Math.min(1, value))); | ||
| if (!values.length) return {min: 0, max: 1, ticks: [1, 0.5, 0]}; | ||
| const low = Math.min(...values); | ||
| const high = Math.max(...values); | ||
| const padding = Math.max(0.03, (high - low) * 0.2); | ||
| const rawMin = Math.max(0, low - padding); | ||
| const rawMax = Math.min(1, high + padding); | ||
| const targetStep = Math.max(0.01, (rawMax - rawMin) / 5); | ||
| const steps = [0.01, 0.02, 0.025, 0.05, 0.1, 0.2, 0.25, 0.5, 1]; | ||
| const step = steps.find((candidate) => candidate >= targetStep) || 1; | ||
| let min = Math.max(0, Math.floor(rawMin / step) * step); | ||
| let max = Math.min(1, Math.ceil(rawMax / step) * step); | ||
| if (max <= min) { | ||
| min = Math.max(0, min - step); | ||
| max = Math.min(1, max + step); | ||
| } | ||
| min = Number(min.toFixed(4)); | ||
| max = Number(max.toFixed(4)); | ||
| const ticks = []; | ||
| for (let value = max; value >= min - step / 2; value -= step) ticks.push(Number(value.toFixed(4))); | ||
| return {min, max, ticks}; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
scoreAxis clamps every score into [0, 1].
Line 73 applies Math.max(0, Math.min(1, value)), and y() at Line 165 clamps again. The axis therefore assumes normalized scores. If a recipe reports a raw reward or a percentage, every out-of-range point pins to the top or bottom edge and the chart misrepresents the run. No warning tells the user.
Derive the domain from the data and keep the [0, 1] assumption only as the empty-data default.
Also applies to: 164-166
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 5: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
.replaceAll('>', '>')
.replaceAll('"', '"')
.replaceAll("'", ''')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 4: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
.replaceAll('>', '>')
.replaceAll('"', '"')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 3: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
.replaceAll('>', '>')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 2: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 1: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
🤖 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 `@src/evolve/viewer/static/viewer-ui.js` around lines 72 - 94, Update scoreAxis
to derive min/max from finite numeric scores without clamping values to [0, 1];
retain the existing [0, 1] domain only when no valid scores exist. Update the
y() mapping near the chart rendering logic to use the derived domain directly
and remove its [0, 1] clamping so raw rewards and percentages retain their
relative positions.
| export function scoreTrend(generations, selectedId = null) { | ||
| const points = generations | ||
| .filter((item) => item.score != null) | ||
| .toSorted((a, b) => compareGenerationIds(a.genid, b.genid)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Array.prototype.toSorted browser and Node.js support baseline
💡 Result:
Baseline (i.e., effectively “supported everywhere modern”): Array.prototype.toSorted is available in browsers starting with Chrome/Edge 110+, Firefox 115+, Safari 16+ (and corresponding mobile/WebView versions). Node.js support is present from Node.js 20.0.0+. [1][2][3] Notes: - toSorted is the non-mutating (copying) version of sort, returning a new array without modifying the original. [1][4] - If you need to support older environments (Node < 20 or older browsers), use a copy+sort equivalent like [...arr].sort(compareFn) as the common fallback pattern. [5]
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toSorted
- 2: https://caniuse.com/mdn-javascript_builtins_array_tosorted
- 3: https://replacements.fyi/array.prototype.tosorted
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array
- 5: https://jsguides.dev/reference/array-methods/to-sorted/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(package.json|package-lock.json|yarn.lock|pnpm-lock.yaml|browserslist|README\.md|viewer-ui\.js|viewer-ui\.test\.mjs)$' || true
echo
echo "== package metadata =="
if [ -f package.json ]; then
cat package.json | sed -n '1,220p'
fi
echo
echo "== viewer-ui usage/import locations =="
rg -n "viewer-ui|viewer-ui\.test|viewer-ui\.mjs|Array\.prototype\.toSorted|toSorted|engines|browserslist|targets|Node|node|serve|browser" -S . \
-g '!node_modules' -g '!dist' -g '!build' || true
echo
echo "== targeted file snippets =="
if [ -f src/evolve/viewer/static/viewer-ui.js ]; then
wc -l src/evolve/viewer/static/viewer-ui.js
sed -n '120,155p' src/evolve/viewer/static/viewer-ui.js
fi
if [ -f tests/frontend/viewer-ui.test.mjs ]; then
wc -l tests/frontend/viewer-ui.test.mjs
sed -n '1,80p' tests/frontend/viewer-ui.test.mjs
fiRepository: simple-agent-lab/EvolveX
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lockfile engine/platform hints =="
for f in package-lock.json yarn.lock pnpm-lock.yaml; do
if [ -f "$f" ]; then
echo "-- $f --"
rg -n '"node|engines|Node' "$f" -A2 -B2 || true
fi
done
echo
echo "== static module headers =="
if [ -f src/evolve/viewer/static/viewer-ui.js ]; then
sed -n '1,60p' src/evolve/viewer/static/viewer-ui.js
elif [ -f tests/frontend/viewer-ui.test.mjs ]; then
sed -n '1,40p' tests/frontend/viewer-u.test.mjs || true
fiRepository: simple-agent-lab/EvolveX
Length of output: 2342
Avoid Array.prototype.toSorted unless the supported runtime baseline includes it.
This project does not declare browser or Node runtime requirements, so this call can fail under older Node/browser versions. Use [...array].sort((a, b) => compareGenerationIds(a.genid, b.genid)) if unsupported runtimes need to be safe.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 5: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
.replaceAll('>', '>')
.replaceAll('"', '"')
.replaceAll("'", ''')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 4: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
.replaceAll('>', '>')
.replaceAll('"', '"')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 3: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
.replaceAll('>', '>')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 2: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
.replaceAll('<', '<')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 1: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(value ?? '')
.replaceAll('&', '&')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
🤖 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 `@src/evolve/viewer/static/viewer-ui.js` at line 142, Replace the
Array.prototype.toSorted call in the surrounding generation-list flow with a
copied-array sort using the existing compareGenerationIds comparator, preserving
the current ordering while supporting older runtimes.
| def test_generation_diff_adds_bounded_parent_context(viewer_workspace: Path) -> None: | ||
| git(viewer_workspace, "init") | ||
| git(viewer_workspace, "config", "user.name", "Viewer Test") | ||
| git(viewer_workspace, "config", "user.email", "viewer@example.com") | ||
| target = viewer_workspace / "target/example.py" | ||
| target.parent.mkdir() | ||
| target.write_text("".join(f"line {index}\n" for index in range(1, 16))) | ||
| git(viewer_workspace, "add", "target/example.py") | ||
| git(viewer_workspace, "commit", "-m", "baseline") | ||
| git(viewer_workspace, "tag", "gen/0") | ||
| target.write_text("".join("changed\n" if index == 8 else f"line {index}\n" for index in range(1, 16))) | ||
| git(viewer_workspace, "add", "target/example.py") | ||
| git(viewer_workspace, "commit", "-m", "generation 1") | ||
| git(viewer_workspace, "tag", "gen/1") | ||
| (viewer_workspace / "runs/gen-1/meta_agent/changed.json").write_text('["target/example.py"]') | ||
|
|
||
| with TestClient(create_viewer_app(viewer_workspace)) as client: | ||
| response = client.get("/api/evolve/generations/1/diff", params={"context": 5}) | ||
| cumulative = client.get("/api/evolve/generations/1/diff", params={"context": 5, "base": "0"}) | ||
| invalid = client.get("/api/evolve/generations/1/diff", params={"context": 5, "base": "missing"}) | ||
|
|
||
| assert response.status_code == 200 | ||
| assert "-line 8" in response.text | ||
| assert "+changed" in response.text | ||
| assert " line 13" in response.text | ||
| assert " line 14" not in response.text | ||
| assert response.headers["cache-control"] == "no-store" | ||
| assert cumulative.status_code == 200 | ||
| assert cumulative.headers["x-evolve-diff-base"] == "0" | ||
| assert invalid.status_code == 400 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the repository already isolates git configuration in tests or conftest.
set -euo pipefail
fd -t f 'conftest.py' tests --exec cat -n
rg -n 'gpgsign|GIT_CONFIG|defaultBranch|HOME' tests src/evolve/git.pyRepository: simple-agent-lab/EvolveX
Length of output: 22977
🏁 Script executed:
#!/bin/bash
# Inspect the git helper and git-backed tests for isolation patterns or hook/signing effects.
set -euo pipefail
cat -n tests/conftest.py | sed -n '263,276p'
rg -n "def test_generation_diff|commit\.gpgsign|init\.defaultBranch|hook|commit-msg|pre-commit|git(" tests tests/conftest.py | sed -n '1,220p'
python3 - <<'PY'
from pathlib import Path
p = Path("tests/conftest.py")
text = p.read_text()
print("git() passes env:")
idx = text.index("def git(workspace: Path, *args: str) -> str:")
print(text[idx:text.index("def allow_local_runtime", idx)])
print("subprocess calls in git helper:", text[idx:text.index("def allow_local_runtime", idx)].count("subprocess.run"))
print("subprocess calls provide env kwarg in git helper:", "env=" in text[idx:text.index("def allow_local_runtime", idx)])
PYRepository: simple-agent-lab/EvolveX
Length of output: 818
🏁 Script executed:
#!/bin/bash
# Inspect git-backed tests and git helper behavior without regex pitfalls.
set -euo pipefail
sed -n '263,271p' tests/conftest.py
rg -n 'def test_generation_diff|commit\.gpgsign|init\.defaultBranch|hook|commit-msg|pre-commit' tests tests/conftest.py || true
rg -n 'def git\(|subprocess\.run|env=' tests/conftest.pyRepository: simple-agent-lab/EvolveX
Length of output: 2019
Make git() hermetic before relying on git-backed assertions.
tests/conftest.py:263 runs git without env=..., and there is no global init.defaultBranch or commit.gpgsign override. Add an inherited config that disables signing and defaults to a known branch, or state the git dependency requirement in tests/test_viewer_app.py.
🤖 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 `@tests/test_viewer_app.py` around lines 58 - 87, The git-backed test setup in
test_generation_diff_adds_bounded_parent_context is not hermetic because git may
use repository-specific signing or an environment-dependent default branch.
Update the shared git helper used by this test to inherit configuration
disabling commit signing and setting a deterministic initial branch, or
explicitly declare the required git configuration in the test setup before the
first git invocation.
| def test_frontend_has_required_navigation_and_refresh_contract() -> None: | ||
| repository = Path(__file__).parents[1] | ||
| static = repository / "src/evolve/viewer/static" | ||
| html = (static / "index.html").read_text() | ||
| javascript = (static / "app.js").read_text() | ||
| styles = (static / "styles.css").read_text() | ||
|
|
||
| assert all(label in html for label in ("Overview", "Generations", "Trials")) | ||
| assert "<strong>EvolveX</strong>" in html | ||
| assert 'src="/evolve-assets/evolve-mark.svg"' in html | ||
| assert (static / "evolve-mark.svg").read_bytes() == (repository / "docs/evolve-mark.svg").read_bytes() | ||
| assert "3000" in javascript | ||
| assert "/api/evolve/snapshot" in javascript | ||
| assert "Full Harbor inspection" in javascript | ||
| assert all(label in javascript for label in ("← Overview", "← Generations", "← Generation")) | ||
| assert all( | ||
| label in javascript | ||
| for label in ("Previous performance page", "Next performance page", "GEPA train score change") | ||
| ) | ||
| assert "Global final result" in javascript | ||
| assert "Global champion from canonical evaluation" in javascript | ||
| assert "Champion agent ·" in javascript | ||
| assert "Champion diff" in javascript | ||
| assert "Champion files" in javascript | ||
| assert "View diff" in javascript | ||
| assert "View formatted diff" not in javascript | ||
| assert "Champion replay" in javascript | ||
| assert "Next · Generation" in javascript | ||
| assert "data-champion-next" in javascript | ||
| assert "next.addEventListener('click'" in javascript | ||
| assert "did not change from" in javascript | ||
| assert "hasTrainScore && !globalResult" in javascript | ||
| assert "championDiffCard" in javascript | ||
| assert "performance-pages" in javascript | ||
| assert "panel.classList.toggle('is-active', active)" in javascript | ||
| assert all( | ||
| label in javascript | ||
| for label in ("Generation comparison", "Original", "Modified", "Modified files", "Split", "Unified") | ||
| ) | ||
| assert "options.outputFormat || 'side-by-side'" in javascript | ||
| assert "options.drawFileList ?? true" in javascript | ||
| assert ".d2h-code-side-linenumber" in styles | ||
| assert "display: table-cell" in styles | ||
| assert all( | ||
| label in javascript | ||
| for label in ( | ||
| "Select", | ||
| "Rollout", | ||
| "Trace Analyzer", | ||
| "Meta Agent", | ||
| "Validate", | ||
| "Novelty", | ||
| "Canonical Evaluation", | ||
| "Gate", | ||
| "Record", | ||
| "Reflect", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
These assertions test source text, not behavior.
The test reads app.js and styles.css and checks about forty exact substrings. Several assert implementation details rather than user-visible contracts, for example "panel.classList.toggle('is-active', active)", "next.addEventListener('click'", "options.drawFileList ?? true", and "hasTrainScore && !globalResult". Any rename, reformat, or equivalent rewrite breaks the suite without a real regression. A future Prettier or minification step breaks it too.
assert "3000" in javascript at Line 198 is the weakest check. It passes for any unrelated occurrence of 3000, and it fails as soon as the poll interval moves into a named constant.
Keep the assertions that describe user-visible output, such as navigation labels, the brand mark, and the asset parity check at Line 197. Move the behavior checks into tests/frontend/viewer-ui.test.mjs, which already exercises the rendering utilities directly.
🤖 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 `@tests/test_viewer_app.py` around lines 187 - 244, Refactor
test_frontend_has_required_navigation_and_refresh_contract to retain only
user-visible HTML labels, branding, asset parity, and other stable contract
assertions. Remove exact app.js/styles.css implementation-text checks, including
the “3000” substring and expressions such as panel.classList.toggle,
event-listener wiring, option defaults, and global-result conditions. Add
equivalent behavioral coverage in tests/frontend/viewer-ui.test.mjs using its
existing rendering utilities, including validation of the refresh interval
behavior rather than searching source text.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
README.md (1)
5-7: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPreserve a semantic level-one heading.
The branding replacement removes the previous level-one heading. The next shown headings use
##, so heading navigation does not expose the project title. Wrap the wordmark in an<h1>or add a visually hidden<h1>RSIHub</h1>.Proposed README structure
-<p align="center"> +<h1 align="center"> <img src="docs/rsihub-wordmark.svg" width="184" alt="RSIHub"> -</p> +</h1>🤖 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 `@README.md` around lines 5 - 7, Preserve a semantic level-one README heading for the project title by wrapping the existing RSIHub wordmark in an h1 or adding a visually hidden h1 containing “RSIHub”; keep the current visual branding and subsequent heading hierarchy unchanged.tests/test_viewer_cli.py (1)
43-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRecord the Uvicorn call in this test.
The
uvicorn.runstub discards every argument. This test can pass ifrun_viewerprints8765but passes a different application, host, or port to the production call atsrc/evolve/viewer/app.pyLine 228-232. Capture the arguments and assert them.Proposed test improvement
- monkeypatch.setattr("evolve.viewer.app.create_viewer_app", lambda _workspace: object()) - monkeypatch.setattr("evolve.viewer.app.uvicorn.run", lambda *_args, **_kwargs: None) + app = object() + calls = {} + + def fake_create_viewer_app(workspace): + calls["workspace"] = workspace + return app + + def fake_run(application, **kwargs): + calls["application"] = application + calls.update(kwargs) + + monkeypatch.setattr("evolve.viewer.app.create_viewer_app", fake_create_viewer_app) + monkeypatch.setattr("evolve.viewer.app.uvicorn.run", fake_run) ... + assert calls == { + "workspace": tmp_path, + "application": app, + "host": "127.0.0.1", + "port": 8765, + }🤖 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 `@tests/test_viewer_cli.py` around lines 43 - 49, Update the test around run_viewer to record the arguments passed through the monkeypatched evolve.viewer.app.uvicorn.run, then assert the captured application, host, and port match the expected viewer app, 127.0.0.1, and 8765. Keep the existing URL output assertion.
🤖 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.
Nitpick comments:
In `@README.md`:
- Around line 5-7: Preserve a semantic level-one README heading for the project
title by wrapping the existing RSIHub wordmark in an h1 or adding a visually
hidden h1 containing “RSIHub”; keep the current visual branding and subsequent
heading hierarchy unchanged.
In `@tests/test_viewer_cli.py`:
- Around line 43-49: Update the test around run_viewer to record the arguments
passed through the monkeypatched evolve.viewer.app.uvicorn.run, then assert the
captured application, host, and port match the expected viewer app, 127.0.0.1,
and 8765. Keep the existing URL output assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ca1b45a3-119a-41db-983b-fff9f55892a8
⛔ Files ignored due to path filters (4)
docs/rsihub-mark.svgis excluded by!**/*.svgdocs/rsihub-wordmark.svgis excluded by!**/*.svgsrc/evolve/viewer/static/rsihub-mark.svgis excluded by!**/*.svguv.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
ARCHITECTURE.mdREADME.mddocs/development/documentation.mddocs/guides/experiment-viewer.mdmkdocs.ymlpyproject.tomlsrc/evolve/cli.pysrc/evolve/viewer/app.pysrc/evolve/viewer/reader.pysrc/evolve/viewer/static/app.jssrc/evolve/viewer/static/index.htmlsrc/evolve/viewer/static/vendor/THIRD_PARTY_NOTICES.mdtests/test_public_repository.pytests/test_viewer_app.pytests/test_viewer_cli.py
🚧 Files skipped from review as they are similar to previous changes (11)
- mkdocs.yml
- src/evolve/cli.py
- src/evolve/viewer/static/index.html
- src/evolve/viewer/static/vendor/THIRD_PARTY_NOTICES.md
- docs/guides/experiment-viewer.md
- pyproject.toml
- ARCHITECTURE.md
- src/evolve/viewer/reader.py
- src/evolve/viewer/static/app.js
- src/evolve/viewer/app.py
- tests/test_viewer_app.py
2 participants
Summary
Validation
node --check src/evolve/viewer/static/app.jsnode --test tests/frontend/viewer-ui.test.mjs.venv/bin/ruff check src/evolve/viewer/app.py tests/test_viewer_app.py.venv/bin/ruff format --check src/evolve/viewer/app.py tests/test_viewer_app.py.venv/bin/pytest -q tests/test_viewer_app.py tests/test_viewer_snapshot.pygit diff --checkValidated against completed GEPA and A-Evolve workspaces, including a champion lineage with consecutive changes to the same files across G0→G1→G2→G3.
Summary by CodeRabbit
evolve viewexperiment viewer.