Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,17 @@ jobs:
python-version: ${{ matrix.python-version }}
- name: Test with pytest
run: uv run --with pytest --with pytest-cov pytest --doctest-modules --cov=copilotj --cov-report=xml --cov-report=html --ignore=examples --pyargs copilotj

java-test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v7
- name: Set up JDK 21
uses: actions/setup-java@v5
with:
distribution: zulu
java-version: "21"
cache: maven
- name: Run Java unit tests
working-directory: plugin
run: mvn -B test
24 changes: 13 additions & 11 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,16 @@ Run `just` or `just --list` to see all available commands.

You can also review the `justfile` to understand how each task is defined and configured, or refer to it directly if you prefer not to install [just](https://github.com/casey/just).

| Command | Description |
| ------------------- | ------------------------------ |
| `just dev-server` | Run the bridge server |
| `just dev-plugin` | Run the ImageJ plugin (debug) |
| `just dev-web` | Run the web frontend |
| `just test` | Run Python tests |
| `just test-cov` | Run tests with coverage report |
| `just build-plugin` | Build the plugin JAR |
| `just build-web` | Build the web frontend |
| Command | Description |
| ------------------- | ----------------------------- |
| `just dev-server` | Run the bridge server |
| `just dev-plugin` | Run the ImageJ plugin (debug) |
| `just dev-web` | Run the web frontend |
| `just test` | Run tests |
| `just test-python` | Run Python tests only |
| `just test-plugin` | Run ImageJ plugin tests only |
| `just build-plugin` | Build the plugin JAR |
| `just build-web` | Build the web frontend |

## Running the components

Expand Down Expand Up @@ -278,8 +279,9 @@ and register any custom tools.
## Testing

```bash
just test # Run Python tests
just test-cov # Run tests with coverage report (HTML + XML)
just test # Run tests
just test-python # Run Python tests only
just test-plugin # Run ImageJ plugin tests only
```

## Observability (optional)
Expand Down
1 change: 1 addition & 0 deletions copilotj/multiagent/leader_multiagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,7 @@ async def run(self, task: str, trace_ctx: Langfuse | None = None) -> None:
"Reply with a single Thought + Action, or a Final Answer.",
trace_ctx=trace_ctx,
)
syntax_error_counter = 0 # Reset syntax error counter on success
except ModelSyntaxError as e:
syntax_error_counter += 1
self.log_error(
Expand Down
44 changes: 44 additions & 0 deletions copilotj/plugin/awt/window/ij_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#
# SPDX-License-Identifier: Apache-2.0

import re
from typing import Literal, override

from copilotj.plugin._base import FromTo, Response, Verbosity
Expand Down Expand Up @@ -46,11 +47,42 @@ def _describe(self, *, level: int, verbosity: Verbosity) -> list[str]:
return [self._describe_one_line(level=level, verbosity=verbosity)]


# Values of these metadata keys duplicate the structured fields rendered by IjImage._describe
# (width/height/depth/channels/frames, calibration, units, bit depth). They are skipped on the
# perception/display path only — the stored `info` keeps the full raw metadata. Note that `info`
# also participates in IjImage.equals and is rendered verbatim on the diff path
# (IjImageDifference via FromTo), so volatile metadata can still surface as snapshot diffs; that
# tradeoff is intentional (see the matching note on the Java side, IjImage.buildImageMetadata).
_INFO_REDUNDANT_KEYS = frozenset(
{
"SizeX",
"SizeY",
"SizeZ",
"SizeC",
"SizeT",
"PhysicalSizeX",
"PhysicalSizeY",
"PhysicalSizeZ",
"PhysicalSizeXUnit",
"PhysicalSizeYUnit",
"PhysicalSizeZUnit",
"BitsPerSample",
"BitsPerPixel",
"SignificantBits",
"PixelType",
}
)
# Matches Bio-Formats/TIFF-style "Key: Value" lines; non-matching lines (prose, OME-XML, "key=value"
# headers from FileInfo.description) are left intact.
_INFO_KV_RE = re.compile(r"^([A-Za-z0-9_]+)\s*:\s*(.+)$")


class IjImage(AwtWindowBase[TYPE]):
title: str
image_type: str
size: str
path: str | None
info: str | None = None

bit_depth: int
stack_size: int
Expand Down Expand Up @@ -121,6 +153,17 @@ def _describe(self, *, level: int, verbosity: Verbosity) -> list[str]:
if self.roi:
lines.append(self.roi._describe_one_line(level=level, verbosity=verbosity))

if self.info:
kept = []
for ln in self.info.splitlines():
m = _INFO_KV_RE.match(ln.strip())
if m and m.group(1) in _INFO_REDUNDANT_KEYS:
continue # already shown via the structured fields above
kept.append(ln)
if kept: # don't emit an empty "Metadata:" block when every line was redundant
lines.append("Metadata:")
lines.extend(kept)

return lines


Expand All @@ -129,6 +172,7 @@ class IjImageDifference(AwtWindowDifferenceBase[TYPE]):
image_type: FromTo[str] | None = None
size: FromTo[str] | None = None
path: FromTo[str | None] | None = None
info: FromTo[str | None] | None = None

bit_depth: FromTo[int] | None = None
stack_size: FromTo[int] | None = None
Expand Down
155 changes: 155 additions & 0 deletions copilotj/test/multiagent/test_leader_syntax_reset.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
# SPDX-FileCopyrightText: Copyright contributors to the CopilotJ project.
#
# SPDX-License-Identifier: Apache-2.0

"""Tests for LeaderDriven.run() syntax-error counter reset.

`syntax_error_counter` must track CONSECUTIVE ReAct syntax errors, not
cumulative ones: a model that recovers (produces a valid response) between
errors must have its counter reset, mirroring `tool_retry_counter` at L850.

Control-flow fact these tests rely on (verified by trace + outside-voice
review): every ModelSyntaxError increment in run() leaves `agent_resp = None`,
which routes recovery through the single `send_correction` at ~L782. That site
is the only place a reset is load-bearing.
"""

import asyncio

from copilotj.core import ModelSyntaxError
from copilotj.multiagent.leader_multiagent import LeaderDriven

_MAX_SYNTAX_ERRORS = 3


class _Args:
def model_dump(self) -> dict:
return {}


class _Tool:
def __init__(self, name: str) -> None:
self.name = name


class _ToolCall:
def __init__(self, name: str) -> None:
self.tool = _Tool(name)
self.args = _Args()


class _Resp:
"""Minimal stand-in for ModelResponse exercised by run()."""

def __init__(self, *, content=None, reasoning_content=None, tool_calls=None) -> None:
self.content = content
self.reasoning_content = reasoning_content
self.tool_calls = tool_calls


def _syntax_err() -> ModelSyntaxError:
return ModelSyntaxError("bad ReAct")


def _action() -> _Resp:
"""A valid parsed response that carries a tool call (no final answer)."""
return _Resp(reasoning_content="thought", tool_calls=[_ToolCall("some_tool")])


def _final() -> _Resp:
return _Resp(content="final answer")


class _ScriptedLeader:
"""LeaderAgent stub whose dialog methods pop a scripted queue.

Each queue holds either a _Resp (returned) or a ModelSyntaxError (raised).
"""

def __init__(self) -> None:
self.begin_dialog_q: list = []
self.send_correction_q: list = []
self.continue_dialog_q: list = []
self.call_tool_q: list = []

@staticmethod
def _pop(queue: list):
item = queue.pop(0)
if isinstance(item, Exception):
raise item
return item

async def begin_dialog(self, task, trace_ctx=None):
return self._pop(self.begin_dialog_q)

async def send_correction(self, message, trace_ctx=None):
return self._pop(self.send_correction_q)

async def continue_dialog(self, prior_response, observation, trace_ctx=None):
return self._pop(self.continue_dialog_q)

async def _call_tool(self, tool_call):
return self._pop(self.call_tool_q)


async def _noop_dialog_changed(*args, **kwargs) -> None:
pass


async def _drive(leader: _ScriptedLeader) -> str:
"""Run one LeaderDriven.run() dialog and return its final status.

Bypasses LeaderDriven.__init__ (which needs cfg/apis/model_client); run()
only touches the attributes wired up here.
"""
pattern = LeaderDriven.__new__(LeaderDriven)
pattern.dialog_counter = 1
pattern._summarize_task = None # type: ignore[attr-defined]
pattern.log_info = lambda *a, **k: None # type: ignore[assignment]
pattern.log_error = lambda *a, **k: None # type: ignore[assignment]
pattern.dialog_changed = _noop_dialog_changed # type: ignore[assignment]
pattern.leader_agent = leader # type: ignore[assignment]

captured: dict = {}

async def _record(*args) -> None:
# _background_summarize_and_store(dialog_id, task, dialog_context)
captured["ctx"] = args[-1]

pattern._background_summarize_and_store = _record # type: ignore[assignment]

await pattern.run("do the thing")
await pattern._summarize_task # type: ignore[attr-defined]
return captured["ctx"]["status"]


def test_cumulative_errors_with_recovery_complete():
""">=3 total syntax errors, each recovered from, must NOT abort.

Under the old cumulative counting this sequence aborted at the third error
(status "failed"); with the reset-on-recovery it completes.
"""
leader = _ScriptedLeader()
leader.begin_dialog_q = [_syntax_err()]
# Three recoveries at site (a), each producing a valid tool call:
leader.send_correction_q = [_action(), _action(), _action()]
leader.call_tool_q = ["ok", "ok", "ok"]
# Two errors interleaved, then a final answer on the third continue:
leader.continue_dialog_q = [_syntax_err(), _syntax_err(), _final()]

status = asyncio.run(_drive(leader))

assert status == "completed"


def test_consecutive_errors_still_abort():
"""Three consecutive syntax errors (no recovery) must still abort."""
leader = _ScriptedLeader()
leader.begin_dialog_q = [_syntax_err()]
leader.send_correction_q = [_syntax_err(), _syntax_err()]

status = asyncio.run(_drive(leader))

assert status == "failed"
# Sanity: we exercised the abort threshold, not some other failure path.
assert len(leader.send_correction_q) == 0
2 changes: 0 additions & 2 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@
UV_PYTHON = python.interpreter;
# JDK configuration
JAVA_HOME = pkgs.jdk21.home;
JAVA8_HOME = pkgs.jdk8.home;
}
// lib.optionalAttrs pkgs.stdenv.isLinux {
# Python libraries often load native shared objects using dlopen(3).
Expand Down Expand Up @@ -206,7 +205,6 @@

# JDK configuration
JAVA_HOME = pkgs.jdk21.home;
JAVA8_HOME = pkgs.jdk8.home;
};

shellHook = ''
Expand Down
16 changes: 13 additions & 3 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#
# JDK configuration (set via flake.nix env vars):
# JAVA_HOME -> JDK 21 (builds the single JAR: core -> Java 8 bytecode, MCP -> Java 17)
# JAVA8_HOME -> JDK 8 (only for ad-hoc Java 8 downgrade checks; not required to build)

default:
@just --list
Expand All @@ -23,6 +22,15 @@ build-plugin:
clean-plugin:
cd plugin && mvn clean

# Test: run the Java unit tests (surefire). No Fiji/ImageJ runtime needed.
test-plugin:
cd plugin && mvn test

# Test + coverage: JaCoCo prepare-agent is already bound by pom-scijava-base,
# so `mvn test` produces jacoco.exec; invoke report explicitly for the HTML.
test-cov-plugin:
cd plugin && mvn test jacoco:report

copy-plugin-deps:
cd plugin && mvn dependency:copy-dependencies -DoutputDirectory=target/deps

Expand All @@ -35,13 +43,15 @@ build-web:
run-workflow:
scripts/run-workflow.sh

test:
test: test-python test-plugin

test-python:
uv run --with pytest \
pytest \
--doctest-modules --ignore=examples \
--pyargs copilotj

test-cov:
test-pythn-cov:
uv run --with pytest --with pytest-cov \
pytest \
--doctest-modules --ignore=examples \
Expand Down
14 changes: 13 additions & 1 deletion plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.scijava</groupId>
<artifactId>pom-scijava</artifactId>
<version>38.0.1</version>
<version>40.0.0</version>
<relativePath />
</parent>

Expand Down Expand Up @@ -248,6 +248,18 @@
<artifactId>ij1-patcher</artifactId>
<scope>provided</scope>
</dependency>

<!-- Layer-A unit tests. pom-scijava does not manage the junit-jupiter
aggregator artifact, so the version is pinned via its inherited
junit-jupiter.version property (5.10.2). surefire 3.2.5 auto-activates
the JUnit Platform engine when junit-jupiter-engine is on the test
classpath — no plugin config needed. -->
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>${junit-jupiter.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<repositories>
Expand Down
Loading