From f9387b32961b508081cc559ed8918ac80a9943f5 Mon Sep 17 00:00:00 2001 From: ugur Date: Mon, 28 Sep 2026 17:10:12 +0100 Subject: [PATCH 01/13] Add CLARA review workflow (issue #3778) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports cell-ontology's `/clara` PR-review workflow to uberon, using the external Cellular-Semantics/clara_workflow package for stage 1 extraction (robot diff -> changes.json) and stage 2/3 literature verification of routed targets. Adjustments vs. the CL original: - `--edit-file src/ontology/uberon-edit.obo` (OBO, not OWL). Verified empirically that `robot diff` detects format from content, not file extension, so this works with the extractor unmodified. One known gap: clara_workflow's `definition_refs()` fallback only recognises OWL functional-syntax `AnnotationAssertion(...)` lines, so it silently contributes no refs for our OBO edit file. This reduces routing coverage for structural axioms on terms whose definition wasn't touched by a given PR, but doesn't break the pipeline. Filed as a gap to fix upstream separately. - ROBOT wrapper now honors `$ROBOT_JAVA_ARGS` (set to -Xmx9G on the extractor step), matching the heap size uberon's own diff.yml/qc.yml use for robot on this ontology. The CL wrapper hardcodes no heap flag at all, which risks OOM on an ontology this much larger than CL. - `CLARA_WORKFLOW_REF` pinned to a commit rather than tracking `main`, per that repo's own adoption guidance. - Added `src/scripts/clara_select_targets.py`, copied unchanged from cell-ontology (it's ontology-agnostic: pure PMID/DOI + change-kind routing logic over `changes.json`). - Added `.github/clara-mcp.json`, a minimal MCP config scoped to just `Asta_semanticscholar` + `artl-mcp` for this workflow, rather than reusing the root dev `.mcp.json` (which also declares `ols4` and `playwright` — not wanted for an unattended CI agent, and playwright needs browser binaries not installed on the runner). Also known but not addressed here: clara_workflow's `agent_instructions.md` uses cell-type-specific wording (`cell_id`, "the cell type") in its verification instructions and output schema, which will show up verbatim in verdicts/reports for uberon terms. Left as-is for this PR; tracked as a follow-up to fix upstream. Requires the `ASTA_API_KEY` secret to be added to this repo before the stage 2/3 verification step can run (not set by this PR). Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/clara-mcp.json | 15 + .github/workflows/clara-review.yml | 356 +++++++++++++++++++++++ src/scripts/clara_select_targets.py | 423 ++++++++++++++++++++++++++++ 3 files changed, 794 insertions(+) create mode 100644 .github/clara-mcp.json create mode 100644 .github/workflows/clara-review.yml create mode 100644 src/scripts/clara_select_targets.py diff --git a/.github/clara-mcp.json b/.github/clara-mcp.json new file mode 100644 index 000000000..7919d5f60 --- /dev/null +++ b/.github/clara-mcp.json @@ -0,0 +1,15 @@ +{ + "mcpServers": { + "Asta_semanticscholar": { + "type": "http", + "url": "https://asta-tools.allen.ai/mcp/v1", + "headers": { "x-api-key": "${ASTA_API_KEY}" }, + "tools": ["*"] + }, + "artl-mcp": { + "type": "stdio", + "command": "uv", + "args": ["tool", "run", "artl-mcp"] + } + } +} diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml new file mode 100644 index 000000000..e9c88b362 --- /dev/null +++ b/.github/workflows/clara-review.yml @@ -0,0 +1,356 @@ +name: CLARA Review + +# Ported from cell-ontology's clara-review.yml (see issue #3778). Two +# uberon-specific adjustments vs. the CL original: +# +# - --edit-file points at the OBO edit file (uberon-edit.obo), not an OWL +# file. `robot diff` detects format from content, not extension, so this +# works with clara_workflow's stage-1 extractor unmodified. NOTE: +# clara_workflow's definition_refs() fallback (used to justify structural +# axioms on terms whose definition wasn't touched by the PR) only +# recognises OWL functional-syntax `AnnotationAssertion(...)` lines and +# silently returns nothing for OBO input — a known upstream gap, tracked +# separately, that degrades routing coverage rather than breaking it. +# - The ROBOT wrapper below honors $ROBOT_JAVA_ARGS (set to -Xmx9G), matching +# the heap size uberon's own diff.yml/qc.yml use for robot on this +# ontology. The CL original's wrapper doesn't parameterize JVM args at +# all, which is fine for CL's much smaller edit file but risks an OOM here. +# +# CLARA_WORKFLOW_REF is pinned to a commit (not `main`) for reproducibility, +# per that repo's own adoption guidance. + +env: + CLARA_WORKFLOW_REPO: Cellular-Semantics/clara_workflow + CLARA_WORKFLOW_REF: 2f5552c8c44058b836dfda7105e89aa695007604 + +on: + issue_comment: + types: [created] + workflow_dispatch: + inputs: + pr: + description: PR number to review + required: true + type: string + +permissions: + contents: read + pull-requests: write + issues: write + id-token: write + +concurrency: + group: clara-review-${{ github.event.issue.number || github.event.inputs.pr || github.run_id }} + cancel-in-progress: true + +jobs: + review: + # issue_comment also fires for plain issues; only honor /clara on PRs. + if: > + github.event_name == 'workflow_dispatch' || + (github.event.issue.pull_request != null && + startsWith(github.event.comment.body, '/clara')) + runs-on: ubuntu-latest + steps: + - name: Resolve PR number + id: pr + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + echo "num=${{ inputs.pr }}" >> "$GITHUB_OUTPUT" + else + echo "num=${{ github.event.issue.number }}" >> "$GITHUB_OUTPUT" + fi + + - name: Acknowledge trigger comment + if: github.event_name == 'issue_comment' + env: + GH_TOKEN: ${{ github.token }} + run: | + gh api --method POST \ + /repos/${{ github.repository }}/issues/comments/${{ github.event.comment.id }}/reactions \ + -f content=eyes + + - name: Look up PR refs + id: refs + env: + GH_TOKEN: ${{ github.token }} + run: | + data=$(gh pr view "${{ steps.pr.outputs.num }}" \ + --repo "${{ github.repository }}" \ + --json baseRefOid,headRefOid,headRefName,baseRefName) + echo "base=$(echo "$data" | jq -r .baseRefOid)" >> "$GITHUB_OUTPUT" + echo "base_name=$(echo "$data" | jq -r .baseRefName)" >> "$GITHUB_OUTPUT" + echo "head=$(echo "$data" | jq -r .headRefOid)" >> "$GITHUB_OUTPUT" + echo "head_name=$(echo "$data" | jq -r .headRefName)" >> "$GITHUB_OUTPUT" + + - name: Checkout PR head with full history + uses: actions/checkout@v4 + with: + fetch-depth: 0 + ref: ${{ steps.refs.outputs.head }} + + - name: Install ROBOT + run: | + mkdir -p "$HOME/.robot" + curl -fsSL -o "$HOME/.robot/robot.jar" \ + https://github.com/ontodev/robot/releases/latest/download/robot.jar + printf '#!/bin/bash\nexec java $ROBOT_JAVA_ARGS -jar "%s/.robot/robot.jar" "$@"\n' "$HOME" \ + > "$HOME/.robot/robot" + chmod +x "$HOME/.robot/robot" + echo "$HOME/.robot" >> "$GITHUB_PATH" + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install uv + uses: astral-sh/setup-uv@v7 + + - name: Check out clara_workflow reference + uses: actions/checkout@v4 + with: + repository: ${{ env.CLARA_WORKFLOW_REPO }} + ref: ${{ env.CLARA_WORKFLOW_REF }} + path: clara_workflow_ref + + - name: Install clara_workflow + run: pip install -e ./clara_workflow_ref + + - name: Run CLARA stage 1 extractor + env: + ROBOT_JAVA_ARGS: -Xmx9G + run: | + python -m clara_workflow.stage1.extract \ + --repo . \ + --left "${{ steps.refs.outputs.base }}" \ + --right "${{ steps.refs.outputs.head }}" \ + --edit-file src/ontology/uberon-edit.obo \ + --output changes.json + + - name: Upload changes.json artifact + uses: actions/upload-artifact@v4 + with: + name: clara-changes-pr-${{ steps.pr.outputs.num }} + path: changes.json + + - name: Select CLARA routing targets + run: | + python src/scripts/clara_select_targets.py \ + --input changes.json \ + --output routing.json + + - name: Upload routing.json artifact + uses: actions/upload-artifact@v4 + with: + name: clara-routing-pr-${{ steps.pr.outputs.num }} + path: routing.json + + - name: Extract routed target summary + id: routing_meta + run: | + python - <<'PY' + import json + from pathlib import Path + + routing = json.loads(Path("routing.json").read_text(encoding="utf-8")) + term_ids = sorted({target["term_id"].replace(":", "_", 1) for target in routing["targets"]}) + + Path("/tmp/routing_outputs.txt").write_text( + f"target_count={len(routing['targets'])}\nterm_count={len(term_ids)}\nterms={' '.join(term_ids)}\n", + encoding="utf-8", + ) + PY + cat /tmp/routing_outputs.txt >> "$GITHUB_OUTPUT" + + - name: Install local MCP server dependencies + if: steps.routing_meta.outputs.target_count != '0' + # The Asta MCP server is remote HTTP, but artl-mcp is configured as a + # local stdio server in clara_workflow's .mcp.json and must be runnable + # on the GitHub runner. + run: | + uv tool install artl-mcp + # Ensure uv tool bin directory is in PATH so Claude's MCP subprocess can find artl-mcp + echo "$(uv tool dir)/bin" >> "$GITHUB_PATH" + + - name: Verify artl-mcp is runnable + if: steps.routing_meta.outputs.target_count != '0' + run: uv tool run artl-mcp --help 2>&1 | head -5 + + - name: Run CLARA stage 2/3 verification for routed targets + id: claude + if: steps.routing_meta.outputs.target_count != '0' + continue-on-error: true + uses: anthropics/claude-code-action@v1 + env: + ASTA_API_KEY: ${{ secrets.ASTA_API_KEY }} + with: + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + github_token: ${{ github.token }} + track_progress: ${{ github.event_name == 'issue_comment' }} + settings: | + {"permissions":{"allow":["mcp__Asta_semanticscholar__*","mcp__artl-mcp__*"]}} + claude_args: | + --mcp-config ${{ github.workspace }}/.github/clara-mcp.json + --disallowedTools "Bash(git add:*),Bash(git commit:*),Bash(git push:*),Bash(git checkout:*),Bash(git switch:*),Bash(git branch:*),Bash(gh pr create:*)" + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ steps.pr.outputs.num }} + BASE SHA: ${{ steps.refs.outputs.base }} + HEAD SHA: ${{ steps.refs.outputs.head }} + + Work in the checked out ontology repository root. + + Read and follow `clara_workflow_ref/clara_workflow/agent_instructions.md`. + + Use `routing.json` in the repository root as the verification input. + Treat `routing.json` as the stable consumer contract for routed CLARA review. + Group targets by `term_id` and process one term-group at a time. + Write one `runs//...` bundle per processed term. + + Requirements: + - Only produce the required `runs//verdicts.json`, `runs//tool_calls.jsonl`, and `runs//report.md` outputs. + - Do not create commits, push branches, or open PRs. + - Ignore any `CLAUDE.md` guidance about committing, pushing, or opening PRs for this run. + - Follow other repository guidance in `CLAUDE.md` when useful, but do not edit ontology source files. + - Do not modify `changes.json` or `routing.json`. + + - name: Upload CLARA runs artifact + if: always() + uses: actions/upload-artifact@v4 + with: + name: clara-runs-pr-${{ steps.pr.outputs.num }} + path: runs + if-no-files-found: warn + + - name: Build CLARA summary comment + env: + PR_NUMBER: ${{ steps.pr.outputs.num }} + CLAUDE_STEP_OUTCOME: ${{ steps.claude.outcome }} + TARGET_COUNT: ${{ steps.routing_meta.outputs.target_count }} + run: | + python - <<'PY' > comment.md + import json + import os + from pathlib import Path + + def load_json(path): + with open(path, encoding="utf-8") as handle: + return json.load(handle) + + def summarize_verdict(path): + data = load_json(path) + assertions = data.get("assertions", []) + core = [a for a in assertions if a.get("category") == "core"] + background = [a for a in assertions if a.get("category") == "background"] + core_fail = [a for a in core if a.get("final_verdict") == "fail"] + core_uncertain = [a for a in core if a.get("final_verdict") == "uncertain"] + if core_fail: + status = "FAIL" + elif core_uncertain: + status = "UNCERTAIN" + else: + status = "PASS" + return { + "cell_id": data["cell_id"], + "name": data["name"], + "status": status, + "core_total": len(core), + "core_fail": len(core_fail), + "core_uncertain": len(core_uncertain), + "background_warn": sum(1 for a in background if a.get("warn_background")), + "failed_texts": [a["text"] for a in core_fail[:3]], + } + + data = load_json("changes.json") + routing = load_json("routing.json") + + route_counts = routing["summary"]["route_counts"] + target_count = int(os.environ["TARGET_COUNT"]) + claude_outcome = os.environ.get("CLAUDE_STEP_OUTCOME", "") + runs_dir = Path("runs") + verdict_files = sorted(runs_dir.glob("*/verdicts.json")) if runs_dir.exists() else [] + verdict_summaries = [summarize_verdict(path) for path in verdict_files] + + lines = [ + f"### CLARA review for PR #{os.environ['PR_NUMBER']}", + "", + f"- Base SHA: `{data['left']['ref']}`", + f"- Head SHA: `{data['right']['ref']}`", + f"- Terms touched: `{len(data['by_term'])}`", + f"- Total extracted changes: `{len(data['changes'])}`", + f"- Reviewable changes: `{len(data['reviewable'])}`", + f"- Decomposable changes: `{len(data['decomposable'])}`", + f"- Routed validation targets: `{routing['summary']['selected_targets']}`", + f"- Routed NTR bundles: `{route_counts['ntr']}`", + f"- Routed revised-text targets: `{route_counts.get('text_revision', 0)}`", + f"- Routed ref-only-addition targets: `{route_counts.get('refs_added', 0)}`", + f"- Routed relationship targets: `{route_counts['relationship']}`", + f"- Routed synonym targets with refs: `{route_counts['synonym']}`", + ] + + if target_count == 0: + lines.extend([ + "", + "_No routed CLARA targets were found, so stage 2/3 verification did not run._", + ]) + elif claude_outcome == "failure": + lines.extend([ + "", + "_Stage 2/3 verification was invoked for routed CLARA targets, but the Claude step did not complete successfully. Check the workflow logs and uploaded artifacts._", + ]) + else: + lines.extend([ + "", + f"_Stage 2/3 verification ran for `{len(verdict_summaries)}` routed term(s)._", + ]) + + if verdict_summaries: + lines.extend([ + "", + "#### Routed verification results", + "", + ]) + for item in verdict_summaries: + lines.append( + f"- `{item['cell_id']}` — {item['name']}: **{item['status']}** " + f"(core: {item['core_total']}, fail: {item['core_fail']}, " + f"uncertain: {item['core_uncertain']}, background warnings: {item['background_warn']})" + ) + for failed in item["failed_texts"]: + lines.append(f" - Failed core assertion: {failed}") + + if routing["summary"]["ignored_changes"]: + # Labels are cosmetic; unknown reasons still surface, so a new + # ignore reason can never go silently unreported. + reason_labels = { + "existing_term_text_not_yet_routed": "Existing-term text changes not yet routed", + "synonym_without_refs": "Synonym changes without refs", + "text_removal_not_reviewable": "Text removals (not justified by refs)", + "refs_added_not_searchable": "Ref-only edits whose new refs are not searchable", + } + lines.extend(["", "#### Currently ignored by routing", ""]) + for reason, count in sorted(routing["summary"]["ignored_reason_counts"].items()): + label = reason_labels.get(reason, reason.replace("_", " ").capitalize()) + lines.append(f"- {label}: `{count}`") + + lines.extend( + [ + "", + "Artifacts uploaded:", + "- `changes.json`", + "- `routing.json`", + "- `runs/`", + ] + ) + + print("\n".join(lines)) + PY + + - name: Post CLARA summary to PR + env: + GH_TOKEN: ${{ github.token }} + run: | + gh pr comment "${{ steps.pr.outputs.num }}" \ + --repo "${{ github.repository }}" \ + --body-file comment.md diff --git a/src/scripts/clara_select_targets.py b/src/scripts/clara_select_targets.py new file mode 100644 index 000000000..1a979028d --- /dev/null +++ b/src/scripts/clara_select_targets.py @@ -0,0 +1,423 @@ +#!/usr/bin/env python3 + +"""Select ticket-relevant CLARA validation targets from stage-1 output. + +Phase 2 is still a routing preview. This script reads `changes.json` from +`clara_workflow.stage1.extract` and emits a normalized `routing.json` payload +that identifies which downstream CLARA checks should run for the current PR. + +This file is the producer-side contract for routed CLARA review. The consumer +is `clara_workflow/clara_workflow/agent_instructions.md`, which expects: + +- processing grouped by `term_id` +- `ntr` targets with canonical prose in `textual_changes` +- `relationship` / `synonym` targets with a single `change` +- `candidate_refs` and `term_level_candidate_refs` already filtered to + searchable literature ids (`PMID:...` / `DOI:...`) + +Notes on compatibility: + +- `textual_changes` is the canonical field for decomposable prose +- `definition_changes` is still emitted as a temporary alias so the current + consumer can tolerate older payload samples during cleanup + +Current routing policy mirrors the ticket scope: + +- New terms (NTRs): route added definitions/comments plus added structural + axioms as one NTR validation bundle. +- Existing terms: route added structural axioms (`subclass`, `relationship`, + `equivalent_class`) as direct atomic checks. +- Existing terms: route text changes from stage-1 `text_deltas`, split by what + actually changed — `text_revision` for new prose, `refs_added` for prose that + is unchanged but has gained a reference. The second exists because ROBOT + reports an annotated axiom as a removed/added pair, so attaching a dbxref to + an untouched definition otherwise looks identical to a rewrite. +- Any term: route added synonym axioms only when the synonym axiom itself has + attached refs. + +Intentionally not routed yet: + +- Reviewable removals +- Synonyms without refs +""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path + + +TEXTUAL_KINDS = frozenset({"text_def", "comment"}) +STRUCTURAL_KINDS = frozenset({"subclass", "relationship", "equivalent_class"}) +SYNONYM_KINDS = frozenset( + {"synonym_exact", "synonym_broad", "synonym_narrow", "synonym_related"} +) + + +def _stable_unique(values: list[str]) -> list[str]: + seen: set[str] = set() + ordered: list[str] = [] + for value in values: + if value not in seen: + seen.add(value) + ordered.append(value) + return ordered + + +def _is_searchable_ref(value: str) -> bool: + """Return whether a ref is usable by the downstream literature tools.""" + upper = value.upper() + return upper.startswith("PMID:") or upper.startswith("DOI:") + + +def _refs_for_changes(changes: list[dict]) -> list[str]: + """Collect unique searchable refs from staged ontology changes. + + This is the upstream filtering point for the routed-target contract. + Downstream agentic stages should treat these lists as already curated and + should not broaden them beyond formatting normalization for tool calls. + """ + refs: list[str] = [] + for change in changes: + refs.extend(ref for ref in change.get("refs", []) if _is_searchable_ref(ref)) + return _stable_unique(refs) + + +def _change_target( + *, + route: str, + validation_mode: str, + ordinal: int, + change: dict, + term_id: str, + term_label: str, + term_is_new: bool, + term_level_candidate_refs: list[str], +) -> dict: + axiom_refs = _refs_for_changes([change]) + # Logical axioms never carry dbxrefs of their own, so an empty axiom-level + # list is the normal case, not a missing-data case: scope them to the + # definition's refs instead of handing the agent an empty list. + if not axiom_refs and change["kind"] in STRUCTURAL_KINDS: + axiom_refs = list(term_level_candidate_refs) + return { + "target_id": f"{route}:{term_id}:{ordinal}", + "route": route, + "validation_mode": validation_mode, + "term_id": term_id, + "term_label": term_label, + "term_is_new": term_is_new, + "change": change, + "candidate_refs": axiom_refs, + "term_level_candidate_refs": term_level_candidate_refs, + } + + +def _text_target( + *, + route: str, + validation_mode: str, + ordinal: int, + change: dict, + term_id: str, + term_label: str, + term_level_candidate_refs: list[str], + candidate_refs: list[str] | None = None, + extra: dict | None = None, +) -> dict: + """Build a text target for an existing term. + + Prose is carried in `textual_changes` (a one-element list) so the consumer + can reuse the same decomposition path it already applies to NTR bundles. + """ + target = { + "target_id": f"{route}:{term_id}:{ordinal}", + "route": route, + "validation_mode": validation_mode, + "term_id": term_id, + "term_label": term_label, + "term_is_new": False, + "textual_changes": [change], + "candidate_refs": ( + _refs_for_changes([change]) if candidate_refs is None else candidate_refs + ), + "term_level_candidate_refs": term_level_candidate_refs, + } + if extra: + target.update(extra) + return target + + +def select_targets(payload: dict) -> dict: + """Transform stage-1 output into routed targets for CLARA verification. + + Output contract: + + - one `ntr` target per new term containing: + - `textual_changes` (canonical prose field) + - `definition_changes` (compatibility alias only) + - `relationship_changes` + - one `relationship` target per existing-term structural assertion + - one `synonym` target per synonym assertion that carries attached refs + + The resulting `routing.json` is consumed term-grouped: every target with the + same `term_id` is expected to be processed together downstream. + """ + targets: list[dict] = [] + ignored: list[dict] = [] + route_counts = { + "ntr": 0, + "text_revision": 0, + "refs_added": 0, + "relationship": 0, + "synonym": 0, + } + # Stage-1 pairs removed/added text axioms; absent on payloads produced + # before that landed, in which case existing-term text stays unrouted. + deltas_by_term: dict[str, list[dict]] = {} + for delta in payload.get("text_deltas", []): + deltas_by_term.setdefault(delta["term_id"], []).append(delta) + have_text_deltas = "text_deltas" in payload + ignored_reason_counts: dict[str, int] = {} + reviewable_terms = 0 + obsoleted_terms_skipped = 0 + + for term_id, entry in sorted(payload["by_term"].items()): + term_label = entry["term_label"] + term_is_new = bool(entry["is_new_term"]) + term_is_obsoleted = bool(entry["is_obsoleted"]) + term_is_reviewable = bool(entry["is_reviewable"]) + if term_is_reviewable: + reviewable_terms += 1 + if term_is_obsoleted: + obsoleted_terms_skipped += 1 + continue + + added_reviewable = [ + change + for change in entry["changes"] + if change["side"] == "added" and change["kind"] in (TEXTUAL_KINDS | STRUCTURAL_KINDS | SYNONYM_KINDS) + ] + # Refs on definitions touched by this PR, falling back to the refs on + # the term's definition as it stands at the head ref. The fallback is + # what makes a logical definition checkable: `robot diff` reports only + # changed axioms, so a PR that adds an EquivalentTo to an untouched term + # carries no definition axiom, yet the text definition it formalises is + # exactly what justifies it. + term_level_candidate_refs = _stable_unique( + _refs_for_changes( + [change for change in added_reviewable if change["kind"] in TEXTUAL_KINDS] + ) + + [ref for ref in entry.get("definition_refs", []) if _is_searchable_ref(ref)] + ) + + if term_is_new: + ntr_textual = [change for change in added_reviewable if change["kind"] in TEXTUAL_KINDS] + ntr_structural = [change for change in added_reviewable if change["kind"] in STRUCTURAL_KINDS] + if ntr_textual or ntr_structural: + targets.append( + { + "target_id": f"ntr:{term_id}", + "route": "ntr", + "validation_mode": "decompose_definition_and_relationships", + "term_id": term_id, + "term_label": term_label, + "term_is_new": True, + # Canonical field consumed by clara_workflow. + "textual_changes": ntr_textual, + # Temporary alias kept during producer/consumer cleanup. + "definition_changes": ntr_textual, + "relationship_changes": ntr_structural, + "candidate_refs": _refs_for_changes(ntr_textual + ntr_structural), + "term_level_candidate_refs": term_level_candidate_refs, + } + ) + route_counts["ntr"] += 1 + + if not term_is_new and have_text_deltas: + text_ordinal = 0 + refs_ordinal = 0 + for delta in deltas_by_term.get(term_id, []): + status = delta["status"] + if status == "removed": + ignored.append( + { + "term_id": term_id, + "term_label": term_label, + "reason": "text_removal_not_reviewable", + "change": delta, + } + ) + continue + + # Prefer the parsed change (it carries the predicate) and fall + # back to the delta itself if stage 1 didn't surface a match. + change = next( + ( + c + for c in added_reviewable + if c["kind"] == delta["kind"] and c["value"] == delta["value"] + ), + None, + ) + if change is None: + continue + change = {**change, "prior_value": delta.get("prior_value")} + + if status == "refs_only": + new_refs = [ + ref for ref in delta.get("refs_added", []) if _is_searchable_ref(ref) + ] + if not new_refs: + # Nothing a literature tool can check; routing it would + # only manufacture an `uncertain` verdict. + ignored.append( + { + "term_id": term_id, + "term_label": term_label, + "reason": "refs_added_not_searchable", + "change": delta, + } + ) + continue + refs_ordinal += 1 + targets.append( + _text_target( + route="refs_added", + validation_mode="validate_new_refs_against_existing_text", + ordinal=refs_ordinal, + change=change, + term_id=term_id, + term_label=term_label, + term_level_candidate_refs=term_level_candidate_refs, + # Only the newly attached refs are under review; the + # pre-existing ones were justified when they landed. + candidate_refs=new_refs, + extra={"refs_added": new_refs}, + ) + ) + route_counts["refs_added"] += 1 + continue + + text_ordinal += 1 + targets.append( + _text_target( + route="text_revision", + validation_mode="decompose_revised_text", + ordinal=text_ordinal, + change=change, + term_id=term_id, + term_label=term_label, + term_level_candidate_refs=term_level_candidate_refs, + ) + ) + route_counts["text_revision"] += 1 + + structural_ordinal = 0 + synonym_ordinal = 0 + for change in added_reviewable: + kind = change["kind"] + if kind in STRUCTURAL_KINDS: + if term_is_new: + continue + structural_ordinal += 1 + targets.append( + _change_target( + route="relationship", + # An equivalence axiom asserts necessary AND sufficient + # conditions, so it is not an atomic relationship check. + validation_mode=( + "validate_equivalent_class_axiom" + if kind == "equivalent_class" + else "validate_atomic_relationship" + ), + ordinal=structural_ordinal, + change=change, + term_id=term_id, + term_label=term_label, + term_is_new=term_is_new, + term_level_candidate_refs=term_level_candidate_refs, + ) + ) + route_counts["relationship"] += 1 + continue + + if kind in SYNONYM_KINDS: + if change.get("refs"): + synonym_ordinal += 1 + targets.append( + _change_target( + route="synonym", + validation_mode="validate_synonym_against_attached_refs", + ordinal=synonym_ordinal, + change=change, + term_id=term_id, + term_label=term_label, + term_is_new=term_is_new, + term_level_candidate_refs=term_level_candidate_refs, + ) + ) + route_counts["synonym"] += 1 + else: + ignored.append( + { + "term_id": term_id, + "term_label": term_label, + "reason": "synonym_without_refs", + "change": change, + } + ) + continue + + if kind in TEXTUAL_KINDS and not term_is_new and not have_text_deltas: + # Pre-text_deltas payload: no way to tell a rewrite from a + # ref-only edit, so leave it unrouted rather than guess. + ignored.append( + { + "term_id": term_id, + "term_label": term_label, + "reason": "existing_term_text_not_yet_routed", + "change": change, + } + ) + + for item in ignored: + reason = item["reason"] + ignored_reason_counts[reason] = ignored_reason_counts.get(reason, 0) + 1 + + return { + "source": { + "left": payload["left"], + "right": payload["right"], + }, + "summary": { + "terms_touched": len(payload["by_term"]), + "reviewable_terms": reviewable_terms, + "obsoleted_terms_skipped": obsoleted_terms_skipped, + "reviewable_changes": len(payload["reviewable"]), + "decomposable_changes": len(payload["decomposable"]), + "selected_targets": len(targets), + "route_counts": route_counts, + "ignored_changes": len(ignored), + "ignored_reason_counts": ignored_reason_counts, + }, + "targets": targets, + "ignored": ignored, + } + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--input", type=Path, required=True, help="Path to stage-1 changes.json") + parser.add_argument("--output", type=Path, required=True, help="Path to write routing.json") + args = parser.parse_args(argv) + + payload = json.loads(args.input.read_text(encoding="utf-8")) + selected = select_targets(payload) + args.output.write_text(json.dumps(selected, indent=2) + "\n", encoding="utf-8") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 429c871f3cacb19a5f1b843e7c16efa1f6be74df Mon Sep 17 00:00:00 2001 From: ugur Date: Tue, 29 Sep 2026 23:07:27 +0100 Subject: [PATCH 02/13] Match clara_workflow's term_id-only output contract Cellular-Semantics/clara_workflow#8 drops the separate underscore-form cell_id field from verdicts.json in favor of the single CURIE-form term_id already used elsewhere in the routing payload. Updates the summary-comment step here to match, so it doesn't KeyError once the CLARA_WORKFLOW_REF pin is bumped to that commit (not done yet). Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index e9c88b362..744040e95 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -252,7 +252,7 @@ jobs: else: status = "PASS" return { - "cell_id": data["cell_id"], + "term_id": data["term_id"], "name": data["name"], "status": status, "core_total": len(core), @@ -313,7 +313,7 @@ jobs: ]) for item in verdict_summaries: lines.append( - f"- `{item['cell_id']}` — {item['name']}: **{item['status']}** " + f"- `{item['term_id']}` — {item['name']}: **{item['status']}** " f"(core: {item['core_total']}, fail: {item['core_fail']}, " f"uncertain: {item['core_uncertain']}, background warnings: {item['background_warn']})" ) From b6b2642fe87f571502bc369a4a00235a63f4b6f1 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 09:40:50 +0100 Subject: [PATCH 03/13] Bump CLARA_WORKFLOW_REF and wire in --catalog Cellular-Semantics/clara_workflow#8 merged (232e9b4): adds catalog support to the stage-1 extractor and drops the cell_id field (already matched in the previous commit here). - CLARA_WORKFLOW_REF bumped to the merge commit. - Added --catalog src/ontology/catalog-v001.xml to the stage-1 extractor invocation. This is the part that actually matters: without it, the extractor falls back to ROBOT's default network import resolution and hits the same 404 (one of uberon's import PURLs is currently broken) that motivated the catalog fix in the first place. The catalog lets ROBOT resolve every owl:imports from the files it already maps to locally-committed content, with no network call. - Updated the header comment, which described the pre-fix limitations. Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index 744040e95..1bccca2a6 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -1,16 +1,17 @@ name: CLARA Review -# Ported from cell-ontology's clara-review.yml (see issue #3778). Two -# uberon-specific adjustments vs. the CL original: +# Ported from cell-ontology's clara-review.yml (see issue #3778). Adjustments +# vs. the CL original: # # - --edit-file points at the OBO edit file (uberon-edit.obo), not an OWL -# file. `robot diff` detects format from content, not extension, so this -# works with clara_workflow's stage-1 extractor unmodified. NOTE: -# clara_workflow's definition_refs() fallback (used to justify structural -# axioms on terms whose definition wasn't touched by the PR) only -# recognises OWL functional-syntax `AnnotationAssertion(...)` lines and -# silently returns nothing for OBO input — a known upstream gap, tracked -# separately, that degrades routing coverage rather than breaking it. +# file, and --catalog passes uberon's own ODK-managed catalog-v001.xml. +# `robot diff`/`robot convert` detect format from content, not extension, +# so OBO input works with clara_workflow's stage-1 extractor unmodified; +# the catalog is what lets ROBOT resolve owl:imports from the locally +# committed files it maps to instead of the network -- required here +# because one of uberon's import PURLs currently 404s, and it's also what +# makes definition_refs() work for OBO input (see +# Cellular-Semantics/clara_workflow#8). # - The ROBOT wrapper below honors $ROBOT_JAVA_ARGS (set to -Xmx9G), matching # the heap size uberon's own diff.yml/qc.yml use for robot on this # ontology. The CL original's wrapper doesn't parameterize JVM args at @@ -21,7 +22,7 @@ name: CLARA Review env: CLARA_WORKFLOW_REPO: Cellular-Semantics/clara_workflow - CLARA_WORKFLOW_REF: 2f5552c8c44058b836dfda7105e89aa695007604 + CLARA_WORKFLOW_REF: 232e9b4796306101eadc159183981adfff7ff185 on: issue_comment: @@ -126,6 +127,7 @@ jobs: --left "${{ steps.refs.outputs.base }}" \ --right "${{ steps.refs.outputs.head }}" \ --edit-file src/ontology/uberon-edit.obo \ + --catalog src/ontology/catalog-v001.xml \ --output changes.json - name: Upload changes.json artifact From 70c16f34f2b941e0102081df34715f281bfdab02 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 09:41:09 +0100 Subject: [PATCH 04/13] TEST (to be reverted): throwaway def edit for CLARA e2e test Revises UBERON:0004177's definition and cites a fabricated DOI (DOI:10.9999/test.uberon.4177, not a real reference) solely to give the CLARA review workflow a real, committed change to diff end-to-end in CI. This commit will be reverted once the test run is reviewed -- not a real content edit. --- src/ontology/uberon-edit.obo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ontology/uberon-edit.obo b/src/ontology/uberon-edit.obo index 35d29772d..7c3ba1b1a 100644 --- a/src/ontology/uberon-edit.obo +++ b/src/ontology/uberon-edit.obo @@ -82735,7 +82735,7 @@ relationship: part_of UBERON:0000990 ! reproductive system [Term] id: UBERON:0004177 name: hemopoietic organ -def: "Organ that is part of the hematopoietic system." [GOC:Obol] +def: "Organ that produces, stores, or regulates the maturation of blood cells, and is part of the hematopoietic system." [GOC:Obol, DOI:10.9999/test.uberon.4177] subset: organ_slim subset: pheno_slim synonym: "haematological system organ" EXACT [OBOL:automatic] From aa745497f3fa1617d3135b0b62754d94dcd762d4 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 09:42:59 +0100 Subject: [PATCH 05/13] Revert "TEST (to be reverted): throwaway def edit for CLARA e2e test" This reverts commit 70c16f34f2b941e0102081df34715f281bfdab02. --- src/ontology/uberon-edit.obo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ontology/uberon-edit.obo b/src/ontology/uberon-edit.obo index 7c3ba1b1a..35d29772d 100644 --- a/src/ontology/uberon-edit.obo +++ b/src/ontology/uberon-edit.obo @@ -82735,7 +82735,7 @@ relationship: part_of UBERON:0000990 ! reproductive system [Term] id: UBERON:0004177 name: hemopoietic organ -def: "Organ that produces, stores, or regulates the maturation of blood cells, and is part of the hematopoietic system." [GOC:Obol, DOI:10.9999/test.uberon.4177] +def: "Organ that is part of the hematopoietic system." [GOC:Obol] subset: organ_slim subset: pheno_slim synonym: "haematological system organ" EXACT [OBOL:automatic] From 385e5b96fa2243700d84aceff8f3f1fdae113d46 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 10:43:49 +0100 Subject: [PATCH 06/13] Address ai4c-reviewer feedback: author gate, concurrency, hardening Per ai4c-reviewer's REQUEST_CHANGES review on this PR: - CRITICAL: /clara had no check on who posted the triggering comment, so anyone (including on a fork PR) could start a run with access to CLAUDE_CODE_OAUTH_TOKEN, ASTA_API_KEY, and a write-scoped GITHUB_TOKEN, and hand the PR's own untrusted content to Claude. Adds a new check-authorization job that: - checks out only the default branch, never PR content, before reading anything - for issue_comment triggers, requires the commenter be listed in .github/ai-controllers.json (the same trusted-controller list ai-agent.yml already uses for this exact purpose); workflow_dispatch is trusted as-is, since GitHub itself already requires write access to trigger it - independently refuses any PR whose head repo differs from its base repo (a fork), regardless of who triggered it -- a trusted commenter doesn't make a fork's CLAUDE.md/ontology text trustworthy once it's checked out and handed to Claude The review job now depends on this and only runs if both checks pass. - IMPORTANT: the concurrency group was computed at workflow level from the PR/issue number alone, before the job-level /clara check ran. Since issue_comment fires for every comment on the PR, an unrelated comment posted mid-run would join the same group and (with cancel-in-progress: true) cancel an actually-running review. Moved concurrency to the review job itself, keyed on needs.check-authorization.outputs.pr_num: check-authorization's own job-level `if:` already skips (never runs, never queues) for anything that isn't a qualifying /clara, so only a real /clara run ever touches this group now. - Suggestions: `inputs.pr` (and its pr_num passthrough) were interpolated directly into `run:` shell blocks; moved to `env:` and added numeric validation on the source input. Fixed `uv tool dir` missing `--bin` (confirmed empirically: `uv tool dir` and `uv tool dir --bin` return different, non-nested paths, so the old `$(uv tool dir)/bin` pointed at a path that doesn't exist -- harmless today only because the actual MCP call uses `uv tool run`, not PATH lookup). Not yet addressed from that review: --left using the base branch tip instead of the merge-base, and pinning ROBOT/uv/artl-mcp versions. Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 102 ++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 15 deletions(-) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index 1bccca2a6..509a4c6b3 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -40,28 +40,98 @@ permissions: issues: write id-token: write -concurrency: - group: clara-review-${{ github.event.issue.number || github.event.inputs.pr || github.run_id }} - cancel-in-progress: true - jobs: - review: - # issue_comment also fires for plain issues; only honor /clara on PRs. + # Gates the review job on two independent things: who triggered it, and + # what content it would review. A trusted commenter doesn't make a fork + # PR's own CLAUDE.md / ontology text trustworthy once it's checked out and + # handed to Claude, so both checks are required, not either/or. + check-authorization: + # issue_comment also fires for plain issues; only consider /clara on PRs. if: > github.event_name == 'workflow_dispatch' || (github.event.issue.pull_request != null && startsWith(github.event.comment.body, '/clara')) runs-on: ubuntu-latest + outputs: + allowed: ${{ steps.check.outputs.allowed }} + pr_num: ${{ steps.pr.outputs.num }} steps: - name: Resolve PR number id: pr + env: + INPUT_PR: ${{ inputs.pr }} run: | if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then - echo "num=${{ inputs.pr }}" >> "$GITHUB_OUTPUT" + if ! [[ "$INPUT_PR" =~ ^[0-9]+$ ]]; then + echo "::error::pr input must be a plain integer, got: $INPUT_PR" + exit 1 + fi + echo "num=$INPUT_PR" >> "$GITHUB_OUTPUT" else echo "num=${{ github.event.issue.number }}" >> "$GITHUB_OUTPUT" fi + - name: Checkout repository (default branch only -- never untrusted PR content) + uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: Check commenter is a trusted controller, and PR is not from a fork + id: check + env: + PR_NUMBER: ${{ steps.pr.outputs.num }} + uses: actions/github-script@v8 + with: + script: | + const fs = require("fs"); + + let allowedUsers = []; + try { + allowedUsers = JSON.parse(fs.readFileSync(".github/ai-controllers.json", "utf8")); + } catch (e) { + core.setFailed(`Could not read .github/ai-controllers.json: ${e}`); + return; + } + + // workflow_dispatch already requires write access, enforced by + // GitHub itself before the workflow even starts; only the + // issue_comment path (anyone who can comment on the PR) needs + // the allowlist. + let commenterAllowed = true; + if (context.eventName === "issue_comment") { + const userLogin = context.payload.comment.user.login; + commenterAllowed = allowedUsers.includes(userLogin); + if (!commenterAllowed) { + console.log(`/clara comment from non-controller: ${userLogin}`); + } + } + + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: Number(process.env.PR_NUMBER), + }); + const isFork = pr.head.repo.full_name !== pr.base.repo.full_name; + if (isFork) { + console.log(`Refusing to review a fork PR: ${pr.head.repo.full_name}`); + } + + core.setOutput("allowed", commenterAllowed && !isFork); + + review: + needs: check-authorization + if: needs.check-authorization.outputs.allowed == 'true' + # Scoped to this job, not the workflow: check-authorization's own job-level + # `if:` already skips (never runs, never queues) for any comment that + # isn't a qualifying /clara -- so only an actual /clara run ever reaches + # this group. A workflow-level group would be computed for every comment + # on the PR regardless of content, and cancel-in-progress would cancel a + # real review over an unrelated "LGTM". + concurrency: + group: clara-review-${{ needs.check-authorization.outputs.pr_num }} + cancel-in-progress: true + runs-on: ubuntu-latest + steps: - name: Acknowledge trigger comment if: github.event_name == 'issue_comment' env: @@ -75,8 +145,9 @@ jobs: id: refs env: GH_TOKEN: ${{ github.token }} + PR_NUM: ${{ needs.check-authorization.outputs.pr_num }} run: | - data=$(gh pr view "${{ steps.pr.outputs.num }}" \ + data=$(gh pr view "$PR_NUM" \ --repo "${{ github.repository }}" \ --json baseRefOid,headRefOid,headRefName,baseRefName) echo "base=$(echo "$data" | jq -r .baseRefOid)" >> "$GITHUB_OUTPUT" @@ -133,7 +204,7 @@ jobs: - name: Upload changes.json artifact uses: actions/upload-artifact@v4 with: - name: clara-changes-pr-${{ steps.pr.outputs.num }} + name: clara-changes-pr-${{ needs.check-authorization.outputs.pr_num }} path: changes.json - name: Select CLARA routing targets @@ -145,7 +216,7 @@ jobs: - name: Upload routing.json artifact uses: actions/upload-artifact@v4 with: - name: clara-routing-pr-${{ steps.pr.outputs.num }} + name: clara-routing-pr-${{ needs.check-authorization.outputs.pr_num }} path: routing.json - name: Extract routed target summary @@ -173,7 +244,7 @@ jobs: run: | uv tool install artl-mcp # Ensure uv tool bin directory is in PATH so Claude's MCP subprocess can find artl-mcp - echo "$(uv tool dir)/bin" >> "$GITHUB_PATH" + echo "$(uv tool dir --bin)" >> "$GITHUB_PATH" - name: Verify artl-mcp is runnable if: steps.routing_meta.outputs.target_count != '0' @@ -197,7 +268,7 @@ jobs: --disallowedTools "Bash(git add:*),Bash(git commit:*),Bash(git push:*),Bash(git checkout:*),Bash(git switch:*),Bash(git branch:*),Bash(gh pr create:*)" prompt: | REPO: ${{ github.repository }} - PR NUMBER: ${{ steps.pr.outputs.num }} + PR NUMBER: ${{ needs.check-authorization.outputs.pr_num }} BASE SHA: ${{ steps.refs.outputs.base }} HEAD SHA: ${{ steps.refs.outputs.head }} @@ -221,13 +292,13 @@ jobs: if: always() uses: actions/upload-artifact@v4 with: - name: clara-runs-pr-${{ steps.pr.outputs.num }} + name: clara-runs-pr-${{ needs.check-authorization.outputs.pr_num }} path: runs if-no-files-found: warn - name: Build CLARA summary comment env: - PR_NUMBER: ${{ steps.pr.outputs.num }} + PR_NUMBER: ${{ needs.check-authorization.outputs.pr_num }} CLAUDE_STEP_OUTCOME: ${{ steps.claude.outcome }} TARGET_COUNT: ${{ steps.routing_meta.outputs.target_count }} run: | @@ -352,7 +423,8 @@ jobs: - name: Post CLARA summary to PR env: GH_TOKEN: ${{ github.token }} + PR_NUM: ${{ needs.check-authorization.outputs.pr_num }} run: | - gh pr comment "${{ steps.pr.outputs.num }}" \ + gh pr comment "$PR_NUM" \ --repo "${{ github.repository }}" \ --body-file comment.md From d1cb998fce7065cd88115f735181a40d59421a11 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 11:17:45 +0100 Subject: [PATCH 07/13] Use merge-base, not base branch tip, for --left Per ai4c-reviewer's REQUEST_CHANGES review: --left used baseRefOid (the current master tip), so a PR branch behind master would diff in every unrelated master change since divergence as spurious reverse-edits. Computes the merge-base via the GitHub compare API's merge_base_commit.sha instead -- verified against this repo that it matches `git merge-base` exactly, both when the branch is caught up (merge-base == tip) and when it's genuinely behind (merge-base is far behind tip). No local checkout is needed for this, so it stays in the same "Look up PR refs" step, before checkout even happens. Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index 509a4c6b3..a95a73472 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -150,9 +150,18 @@ jobs: data=$(gh pr view "$PR_NUM" \ --repo "${{ github.repository }}" \ --json baseRefOid,headRefOid,headRefName,baseRefName) - echo "base=$(echo "$data" | jq -r .baseRefOid)" >> "$GITHUB_OUTPUT" + base_tip=$(echo "$data" | jq -r .baseRefOid) + head_sha=$(echo "$data" | jq -r .headRefOid) + # Use the merge-base, not the base branch's current tip: if this + # branch is behind the base, diffing against the live tip pulls in + # every base-branch change since the branch diverged as spurious + # reverse-edits. merge_base_commit is the same value `git merge-base` + # would give, from the API so no local checkout is needed yet. + merge_base=$(gh api "repos/${{ github.repository }}/compare/${base_tip}...${head_sha}" \ + --jq .merge_base_commit.sha) + echo "base=$merge_base" >> "$GITHUB_OUTPUT" echo "base_name=$(echo "$data" | jq -r .baseRefName)" >> "$GITHUB_OUTPUT" - echo "head=$(echo "$data" | jq -r .headRefOid)" >> "$GITHUB_OUTPUT" + echo "head=$head_sha" >> "$GITHUB_OUTPUT" echo "head_name=$(echo "$data" | jq -r .headRefName)" >> "$GITHUB_OUTPUT" - name: Checkout PR head with full history From d6da5ab0456fdbc457fcde39851d86ad40095244 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 16:03:36 +0100 Subject: [PATCH 08/13] Fix track_progress forcing the wrong claude-code-action mode Found by actually getting a routed target through the pipeline for the first time (against cell-ontology, same workflow) and watching stage 2/3 fail in ~500ms with no real work done. claude-code-action's mode detector (src/modes/detector.ts) forces "tag" mode -- an interactive @mention comment-responder, with its own Todo-list/tracking-comment scaffolding -- whenever track_progress is true on an issue_comment event, before it ever checks whether a custom prompt was supplied. That check only happens later, and only reaches "agent" mode (the batch/file-writing mode this workflow actually needs) if track_progress didn't already force tag mode. track_progress: ${{ github.event_name == 'issue_comment' }} meant every real /clara comment trigger got tag mode instead of agent mode, regardless of our custom prompt. Setting it to a flat `false` lets the detector's "prompt provided on a comment event -> agent mode" rule apply as intended. Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index a95a73472..f174b1b70 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -269,7 +269,16 @@ jobs: with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} github_token: ${{ github.token }} - track_progress: ${{ github.event_name == 'issue_comment' }} + # Must be false: claude-code-action's mode detector forces "tag" + # mode (an interactive @mention comment-responder) whenever + # track_progress is true on an issue_comment event, regardless of + # whether a custom prompt is supplied. That's the wrong mode for + # this batch, file-writing task -- leaving it true made every + # issue_comment-triggered run build @claude-mention scaffolding + # instead of running our actual prompt, and fail immediately. + # false lets the detector's "prompt provided on a comment event" + # rule select agent mode instead, which is what this needs. + track_progress: false settings: | {"permissions":{"allow":["mcp__Asta_semanticscholar__*","mcp__artl-mcp__*"]}} claude_args: | From 6c46affbb6c6a1c65cc0d151a3200f2535b119d1 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 16:14:55 +0100 Subject: [PATCH 09/13] TEMP DEBUG: enable show_full_output on claude-code-action The action hides the real error behind an opaque "result is_error:true" by default. Enabling this to see what's actually failing in stage 2/3, which still fails instantly even after the track_progress fix (agent mode now correctly selected, confirmed via cell-ontology test run, but the claude subprocess still errors in ~500ms with no visible turns). Remove once the real cause is found and fixed. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index f174b1b70..3624d7ba3 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -279,6 +279,10 @@ jobs: # false lets the detector's "prompt provided on a comment event" # rule select agent mode instead, which is what this needs. track_progress: false + # TEMP DEBUG: the action hides the real error behind "result + # is_error:true" unless this is set. Remove once stage 2/3 is + # confirmed working end-to-end. + show_full_output: true settings: | {"permissions":{"allow":["mcp__Asta_semanticscholar__*","mcp__artl-mcp__*"]}} claude_args: | From 70dfeea4fe9af22d579cebe18ad224e9300f4334 Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 16:22:43 +0100 Subject: [PATCH 10/13] Fix stage 2/3: pin --model, action was resolving to a nonexistent one With show_full_output enabled, the real error was finally visible: "error": "model_not_found" "There's an issue with the selected model (claude-sonnet-5-5). It may not exist or you may not have access to it." claude-sonnet-5-5 isn't a real model (the valid id is claude-sonnet-5); something in the action's default-model resolution for this OAuth token was producing a bad value. Explicitly pinning --model sidesteps whatever that resolution logic is doing. This, plus the earlier track_progress fix, are the two real causes of every stage 2/3 failure seen so far -- neither related to MCP config, the security hardening, or anything else touched in this PR. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index 3624d7ba3..5e97a3a67 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -286,6 +286,7 @@ jobs: settings: | {"permissions":{"allow":["mcp__Asta_semanticscholar__*","mcp__artl-mcp__*"]}} claude_args: | + --model claude-sonnet-5 --mcp-config ${{ github.workspace }}/.github/clara-mcp.json --disallowedTools "Bash(git add:*),Bash(git commit:*),Bash(git push:*),Bash(git checkout:*),Bash(git switch:*),Bash(git branch:*),Bash(gh pr create:*)" prompt: | From 1ca3b415e4f5e7e381e12fb9a40648b1146e5e8e Mon Sep 17 00:00:00 2001 From: ugur Date: Wed, 30 Sep 2026 16:33:14 +0100 Subject: [PATCH 11/13] Remove temporary show_full_output debug flag Served its purpose: revealed the model_not_found error (fixed in the previous commit) that a plain is_error:true was hiding. No longer needed now that --model is pinned explicitly. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index 5e97a3a67..444370de6 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -279,10 +279,6 @@ jobs: # false lets the detector's "prompt provided on a comment event" # rule select agent mode instead, which is what this needs. track_progress: false - # TEMP DEBUG: the action hides the real error behind "result - # is_error:true" unless this is set. Remove once stage 2/3 is - # confirmed working end-to-end. - show_full_output: true settings: | {"permissions":{"allow":["mcp__Asta_semanticscholar__*","mcp__artl-mcp__*"]}} claude_args: | From eb020bf9342d1c48f4c428bdd21124b676e8a5ba Mon Sep 17 00:00:00 2001 From: ugur Date: Thu, 1 Oct 2026 12:51:54 +0100 Subject: [PATCH 12/13] Bump CLARA_WORKFLOW_REF to pick up the colon-in-paths fix Cellular-Semantics/clara_workflow#9 merged: fixes runs/{term_id}/ using the CURIE form (colon) in artifact paths, which made actions/upload-artifact fail outright and silently skipped the summary-comment steps on every real run that reached stage 2/3 (confirmed via cell-ontology's PR #3765). Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/workflows/clara-review.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index 444370de6..c9fc5b457 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -22,7 +22,7 @@ name: CLARA Review env: CLARA_WORKFLOW_REPO: Cellular-Semantics/clara_workflow - CLARA_WORKFLOW_REF: 232e9b4796306101eadc159183981adfff7ff185 + CLARA_WORKFLOW_REF: 5c61100b0875f47e2b68de1631d2f531dd52aabe on: issue_comment: From 29889554e39a06e95b66c0222db68db490cb3620 Mon Sep 17 00:00:00 2001 From: ugur Date: Thu, 1 Oct 2026 13:51:13 +0100 Subject: [PATCH 13/13] Port cell-ontology's CLARA fixes: Asta MCP config, track_progress revert Ports three fixes made and tested against a real PR in cell-ontology (obophenotype/cell-ontology#3772, #3773), so uberon doesn't hit the same issues on its first real run: - Bump CLARA_WORKFLOW_REF to 234ac67, which includes Cellular-Semantics/clara_workflow#10: Asta_semanticscholar's `"tools": ["*"]` was invalid (Claude Code expects an array of objects, not strings) and silently dropped the entire server from the session -- not "connected with no tools," never attempted at all. That's why stage 3 snippet_search was never available in any run so far. - Point --mcp-config at clara_workflow_ref/.mcp.json (the external package's own, now-fixed config) instead of this repo's dedicated .github/clara-mcp.json, so MCP tool wiring travels with agent_instructions.md as one versioned unit via CLARA_WORKFLOW_REF, rather than needing to be kept in sync by hand in every consuming repo. Removes the now-unused .github/clara-mcp.json. - Revert track_progress back to its original conditional. The earlier "always false" change here was a misdiagnosis: it forced agent mode to work around what looked like a tag-mode problem, but a real historical cell-ontology run (#3745, 35205839595) proves tag mode completes this task fine, Write included. The actual instant failures seen while debugging were model_not_found (fixed via --model, already in place), not a mode problem -- forcing agent mode just traded tag mode's sufficient default permissions for agent mode's insufficient ones. Signed-off-by: @ai4c-agent Co-Authored-By: Claude Sonnet 5 --- .github/clara-mcp.json | 15 --------------- .github/workflows/clara-review.yml | 28 ++++++++++++++++------------ 2 files changed, 16 insertions(+), 27 deletions(-) delete mode 100644 .github/clara-mcp.json diff --git a/.github/clara-mcp.json b/.github/clara-mcp.json deleted file mode 100644 index 7919d5f60..000000000 --- a/.github/clara-mcp.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "mcpServers": { - "Asta_semanticscholar": { - "type": "http", - "url": "https://asta-tools.allen.ai/mcp/v1", - "headers": { "x-api-key": "${ASTA_API_KEY}" }, - "tools": ["*"] - }, - "artl-mcp": { - "type": "stdio", - "command": "uv", - "args": ["tool", "run", "artl-mcp"] - } - } -} diff --git a/.github/workflows/clara-review.yml b/.github/workflows/clara-review.yml index c9fc5b457..f3a8c58f5 100644 --- a/.github/workflows/clara-review.yml +++ b/.github/workflows/clara-review.yml @@ -22,7 +22,7 @@ name: CLARA Review env: CLARA_WORKFLOW_REPO: Cellular-Semantics/clara_workflow - CLARA_WORKFLOW_REF: 5c61100b0875f47e2b68de1631d2f531dd52aabe + CLARA_WORKFLOW_REF: 234ac67ca82b577c5dedf851a17e57ed2b044478 on: issue_comment: @@ -269,21 +269,25 @@ jobs: with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} github_token: ${{ github.token }} - # Must be false: claude-code-action's mode detector forces "tag" - # mode (an interactive @mention comment-responder) whenever - # track_progress is true on an issue_comment event, regardless of - # whether a custom prompt is supplied. That's the wrong mode for - # this batch, file-writing task -- leaving it true made every - # issue_comment-triggered run build @claude-mention scaffolding - # instead of running our actual prompt, and fail immediately. - # false lets the detector's "prompt provided on a comment event" - # rule select agent mode instead, which is what this needs. - track_progress: false + # true for issue_comment forces claude-code-action's "tag" mode + # (an interactive @mention responder, with its own scaffolding our + # prompt doesn't need) -- but a real historical cell-ontology run + # (obophenotype/cell-ontology#3745, run 35205839595) proves tag + # mode actually completes this task fine, Write included; Claude + # just follows our prompt and ignores the irrelevant scaffolding. + # An earlier "always false" change here was based on a + # misdiagnosis: the actual instant failures seen while debugging + # this were model_not_found (fixed via --model below), not a mode + # problem. + track_progress: ${{ github.event_name == 'issue_comment' }} settings: | {"permissions":{"allow":["mcp__Asta_semanticscholar__*","mcp__artl-mcp__*"]}} + # MCP servers come from clara_workflow's .mcp.json at the pinned ref, + # so tool wiring is versioned with agent_instructions.md, same as + # cell-ontology (obophenotype/cell-ontology#3773). claude_args: | --model claude-sonnet-5 - --mcp-config ${{ github.workspace }}/.github/clara-mcp.json + --mcp-config ${{ github.workspace }}/clara_workflow_ref/.mcp.json --disallowedTools "Bash(git add:*),Bash(git commit:*),Bash(git push:*),Bash(git checkout:*),Bash(git switch:*),Bash(git branch:*),Bash(gh pr create:*)" prompt: | REPO: ${{ github.repository }}