Skip to content

Sign msa-search nim - #46

Open
ohadmo wants to merge 2 commits into
mainfrom
omosafi/msa-search-nim
Open

ohadmo wants to merge 2 commits into
mainfrom
omosafi/msa-search-nim

Conversation

@ohadmo

@ohadmo ohadmo commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

@greptileai

@ohadmo

ohadmo commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds a hosted protein search client and evaluation updates.

The PR appears safe to merge, though the skill card is misleading and the earlier client issues remain unresolved.

Findings

  1. P2 Hosted template support is overstated ▶
  2. P2 Partial output survives write failure ▶
  3. P2 Malformed alignments can pass validation ▶

Summary

The PR adds a bounded hosted MSA client and tests to both skill copies, updates usage and evaluation guidance, and adds publication artifacts to the distributed skill.

  • The new skill card should distinguish local-only template retrieval from hosted MSA operations.
  • The two earlier client findings remain outstanding.

Reviews (2) · Last reviewed commit: "Attach NVSkills validation signatures"

Comment on lines +117 to +122
args.output_dir.mkdir(parents=True, exist_ok=False)
(args.output_dir / "response.json").write_text(json.dumps(result, indent=2) + "\n", encoding="utf-8")
for database in dict.fromkeys(args.databases):
alignment = result["alignments"][database]["a3m"]["alignment"]
path = args.output_dir / f"{database}.a3m"
path.write_text(alignment, encoding="utf-8")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Partial output survives write failure If writing response.json succeeds but a later A3M write fails, the client exits with an error while leaving incomplete files behind. The next run cannot reuse the directory because --output-dir must not already exist. Consider publishing the directory only after all writes succeed, or removing it on failure, and testing a failure during writing. The distributed copy has the same behavior.

Comment on lines +95 to +100
a3m = formats.get("a3m") if isinstance(formats, dict) else None
alignment = a3m.get("alignment") if isinstance(a3m, dict) else None
if not isinstance(alignment, str) or not alignment.lstrip().startswith(">"):
raise SearchError(f"Hosted MSA returned no A3M alignment for {database}.")
if not any(line.strip() and not line.startswith((">", "#")) for line in alignment.splitlines()):
raise SearchError(f"Hosted MSA returned an empty A3M alignment for {database}.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Malformed alignments can pass validation The client accepts an alignment if it starts with > and has any non-header line; it does not check the returned format or whether the records form usable A3M. A malformed response can therefore be saved as a successful .a3m file and fail later in a structure-prediction tool. Check the format and basic record structure before reporting success. The distributed copy has the same check.

Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
### License/Terms of Use: <br>
Apache-2.0 AND CC-BY-4.0 <br>
## Use Case: <br>
Developers and engineers use this skill to generate multiple sequence alignments for protein sequences via GPU-accelerated MMSeqs2, supporting hosted NVIDIA API or local Docker NIM deployment for homolog search, paired MSA for complexes, and structural template retrieval. <br>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Hosted template support is overstated The card presents structural template retrieval alongside both hosted and local deployment. The skill’s API reference says the hosted template endpoint is unavailable and returned HTTP 404, so readers may try a workflow that cannot work. Clarify that template retrieval requires a local NIM.

Suggested change
Developers and engineers use this skill to generate multiple sequence alignments for protein sequences via GPU-accelerated MMSeqs2, supporting hosted NVIDIA API or local Docker NIM deployment for homolog search, paired MSA for complexes, and structural template retrieval. <br>
Developers and engineers use this skill to generate multiple sequence alignments for protein sequences via GPU-accelerated MMSeqs2. Hosted NVIDIA API and local Docker NIM support homolog search and paired MSA for complexes; structural template retrieval requires a local Docker NIM. <br>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants