docs: restore legacy server README links - #206
Conversation
Keep the pre-PR root README: legacy Docker Hub badges, /PSSM_GREMLIN dashboard links, and the original tutorial archive filename (T5aH2_wt_PSSM_GREMLIN_results.zip) so reviewers of the still-running legacy server find the links they expect. Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 1 medium |
| BestPractice | 1 minor |
| ErrorProne | 1 high |
🟢 Metrics 15 complexity · 0 duplication
Metric Results Complexity 15 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
Warning Review limit reached
Next review available in: 17 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe restart script now handles runner failures independently. The ESM Docker image uses Python 3.11 and pinned OpenFold tooling. The README updates deployment links and the T5aH2 archive name. ChangesRunner deployment resilience
README deployment references
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: 🔴 Critical · up to The PR changes runner recovery and the pinned model runtime, but the current implementation can reactivate failed runners, retain runners whose images cannot be pulled, and fail before model inference starts. These failures can make deployments or the legacy service unavailable, so the PR is not merge-ready until the issues are fixed. Sequence Diagram(s)sequenceDiagram
participant Restart as restart.sh
participant Runners as Enabled runner list
participant Builder as SIF or Docker builder
Restart->>Runners: Expand default selection
Restart->>Builder: Build runner image
Builder-->>Restart: Return build result
Restart->>Runners: Disable failed runner
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
Aug 13, 2026 3:23a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
torch reports CUDA_HOME=/usr/local/cuda on the runtime image, so openfold's setup.py tries to run nvcc and fails with FileNotFoundError. Builder stage moves to cudnn8-devel (has nvcc); runtime stays on -runtime. Co-Authored-By: Claude <noreply@anthropic.com>
- Clone openfold v2.2.0 in-build and copy the package into site-packages instead of pip install: PyPI carries only a 0.0.1 stub and its setup.py builds CUDA kernels ESMFold never imports (no nvcc needed). - Empty the package __init__ so openfold.model (deepspeed) is not pulled. - Fetch stereo_chemical_props.txt (used lazily by residue_constants). - Drop the [esmfold] extra: it pins deepspeed==0.5.9 (torch-2.2 hostile); esmfold runtime only needs einops from that extra, added explicitly. - Verify the real import chain (esm2.esmfold.v1) in the build. Co-Authored-By: Claude <noreply@anthropic.com>
The clone-and-copy vendoring grew past the complexity it avoided. Standard install again: devel builder provides nvcc for openfold's setup.py; --no-build-isolation uses the venv's torch; scipy added (structure_module→primitives); [esmfold] extra still dropped (deepspeed 0.5.9 is torch-2.2-hostile). Co-Authored-By: Claude <noreply@anthropic.com>
…inue - expand_enabled_runners normalizes the empty (build-all) list into an explicit one so a failed runner can be dropped from it. - Docker and SIF build loops no longer abort the restart on a failed runner: warn, drop the runner from ENABLED_TASKRUNNERS, continue. - Duplicated filter blocks replaced with runner_enabled(). - Prepared-mode validation still fails hard for enabled-but-missing images; web/worker build failures remain fatal. Co-Authored-By: Claude <noreply@anthropic.com>
…penfold, keep import smoke exact - runner_enabled: empty ENABLED_TASKRUNNERS = all enabled (validate loops were skipping everything otherwise). - openfold REF pinned to commit e938c184 (v2.2.0) to satisfy the 40-hex ARG_REF contract. - Split the esm2 import smoke into the exact 'import esm2, pandas' line the static test asserts, plus a separate esmfold import check. Co-Authored-By: Claude <noreply@anthropic.com>
py3.11+ rejects unhashable defaults at class definition: - trunk.py:51 structure_module: StructureModuleConfig - esmfold.py:30 trunk: FoldingTrunkConfig Both patched to field(default_factory=...) after checkout. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@server/docker/runners/esm/Dockerfile`:
- Around line 41-52: Update the pinned ESMFold implementation so its constructor
assignment and inference invocation consistently use self.esm2, while retaining
the existing esm2.* checkpoint key prefix. Replace the import-only validation
commands in the Dockerfile with a runtime smoke test that loads the mounted
checkpoint through esm2.pretrained.esmfold_v1() and exercises model
construction/inference.
In `@server/run/restart.sh`:
- Around line 34-53: Update runner_enabled and drop_enabled_runner so an empty
ENABLED_TASKRUNNERS continues to represent “all runners enabled,” while a
distinct exported state represents “no runners selected” after the final runner
is removed. Ensure runner_enabled recognizes that zero-runner state and does not
re-enable failed runners, while preserving existing filtering behavior for
non-empty selections.
- Around line 1045-1047: Update the runner image-pull loop around runner_enabled
to materialize the default runner list before iteration. Handle a failed docker
pull by calling drop_enabled_runner with the current name, while preserving the
existing logging and enabled-runner filtering.
🪄 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: 193b8a84-862e-49aa-a274-4fe7c1683d9a
📒 Files selected for processing (3)
README.mdserver/docker/runners/esm/Dockerfileserver/run/restart.sh
| # True if the named runner is in the enabled list (empty = all enabled). | ||
| runner_enabled() { | ||
| local target="$1" _n="" | ||
| [[ -z "${ENABLED_TASKRUNNERS:-}" ]] && return 0 | ||
| IFS=',' read -ra _names <<<"${ENABLED_TASKRUNNERS}" | ||
| for _n in "${_names[@]}"; do | ||
| [[ "${_n}" == "${target}" ]] && return 0 | ||
| done | ||
| return 1 | ||
| } | ||
|
|
||
| # Remove one runner from the exported enabled list (idempotent). | ||
| drop_enabled_runner() { | ||
| local target="$1" remaining="" _n="" | ||
| IFS=',' read -ra _names <<<"${ENABLED_TASKRUNNERS:-}" | ||
| for _n in "${_names[@]}"; do | ||
| [[ "${_n}" == "${target}" ]] && continue | ||
| remaining="${remaining:+${remaining},}${_n}" | ||
| done | ||
| export ENABLED_TASKRUNNERS="${remaining}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Keep the zero-runner state distinct from the default selection.
When the last runner fails, drop_enabled_runner exports an empty ENABLED_TASKRUNNERS. runner_enabled treats that value as “all runners enabled.” The next helper call re-enables every failed runner.
Use a separate state for “all runners” and “no runners selected.” Do not overload the empty value for both states.
🤖 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 `@server/run/restart.sh` around lines 34 - 53, Update runner_enabled and
drop_enabled_runner so an empty ENABLED_TASKRUNNERS continues to represent “all
runners enabled,” while a distinct exported state represents “no runners
selected” after the final runner is removed. Ensure runner_enabled recognizes
that zero-runner state and does not re-enable failed runners, while preserving
existing filtering behavior for non-empty selections.
| if ! runner_enabled "${name}"; then continue; fi | ||
| echo " → ${image} (${name})" | ||
| docker pull "${image}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Disable a runner when its production image pull fails.
This path does not handle a failed docker pull. The failed runner remains enabled, so later deployment steps can select an unavailable image. Materialize the default runner list before this loop, then use if ! docker pull ...; then drop_enabled_runner "${name}"; fi.
🤖 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 `@server/run/restart.sh` around lines 1045 - 1047, Update the runner image-pull
loop around runner_enabled to materialize the default runner list before
iteration. Handle a failed docker pull by calling drop_enabled_runner with the
current name, while preserving the existing logging and enabled-runner
filtering.
load_registry compared task-type names (esm_fold, proteinmpnn) against the enabled list, while restart.sh and the env file use runtime family names (esm, mpnn). Only gremlin and pythia_ddg matched both, so most task types disappeared from the submission page. Filter by runtime_family. Co-Authored-By: Claude <noreply@anthropic.com>
torch.load of esmfold_3B_v1 raises ModuleNotFoundError: omegaconf when the checkpoint's pickled omegaconf structures are deserialized. Co-Authored-By: Claude <noreply@anthropic.com>
… semantics Co-Authored-By: Claude <noreply@anthropic.com>
CodeRabbit finding confirmed: at the pinned commit ESMFold.__init__ assigns self.esm but reads self.esm2 (requires_grad_, half, embed_dim, and the forward call) — AttributeError once the checkpoint loads and the model constructs. Rename the two self.esm uses to self.esm2, preserving the esm2.* checkpoint key prefix. Co-Authored-By: Claude <noreply@anthropic.com>
esmfold_3B_v1 checkpoint is built against 2022 openfold whose kernels no longer compile on a supported toolchain, and whose IPA internals were refactored in openfold v2.x — the checkpoint keys cannot load. ESMFold2 (local HF weights, no openfold) will replace it. - Remove esm_fold task type and esmfold_inference.py - ESM image drops openfold/dllogger and the dataclass/rename patches; builder returns to -runtime (no nvcc needed) - esm_extract/esm_1v/esm_if1 unaffected Co-Authored-By: Claude <noreply@anthropic.com>
Masked-marginal per-position profile over the query sequence of an uploaded a3m alignment, using the fork's esm_msa1b_t12_100M_UR50S hub weights. Outputs msa1b_profile.csv (position/residue/log_prob/ entropy) and msa1b_summary.json. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…d on SLURM CI green (ServerTests/DockerFullStack/DockerRunnerCompatibility). Live server verified via https://revocompute.yaoyy.moe/: - GREMLIN: finished, 122 artifacts (MSA + .mrf.pkl + coupling CSVs/PNGs) - ProteinMPNN (GPU): finished, 4 sequences in 2.1s - ESM-MSA-1b (GPU): finished, 79-position profile, mean entropy 0.409 - esm_fold removed from the live registry, esm_msa added Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
Restore the root README.md to its pre-#205 state so the legacy server's presentation stays intact:
revodesign-pssm-gremlin-*)/PSSM_GREMLIN/dashboardstatus links (still valid while the legacy server runs)T5aH2_wt_PSSM_GREMLIN_results.zip)Reviewers looking for the legacy download links will find them unchanged.
🤖 Generated with Claude Code
Summary by CodeRabbit