Skip to content

Commit 2bd1c7c

Browse files
MisterBrookTclaude
andauthored
fix/feat: P0 bug fixes, type annotations, docstrings, and more tests
* fix/feat: P0 bug fixes, type annotations, docstrings, new tests, CI coverage P0 fixes: - prompts/gen_text2image.py: fix nanochart -> igenbench import - eval_engine.py: guard response with isinstance(dict) before .get() - vis_item.py: correct generation field type Optional[dict] -> Dict[str,Any] - utils/io.py: rename senmantic->semantic, add boundary check with ValueError P1 improvements: - pyproject.toml: add authors, keywords, classifiers, project URLs - pyproject.toml: add pytest-cov to dev deps - CLI functions: add -> None return type annotations - gen_engine.py: add PILImage return type to text2image() - caller_registry.py: add docstring + correct return type annotation - llm_caller.py: add docstrings to all three LLMCaller abstract methods - client.py: add docstrings to call_text/image_generation/understanding - ci.yml: add --cov flag and upload coverage artifact New tests (41 total, up from 17): - tests/test_io.py: 9 tests for extract_from_markdown + split_semantic - tests/test_eval_engine.py: 6 tests for EvalEngine (mocked LLMClient) - tests/test_caller_registry.py: 4 tests for @register_caller decorator - tests/test_cli_score.py: 5 tests for igenbench score command P2: - AGENTS.md: expand deprecated Question -> EvalEntry migration guide * fix: address Gemini review — defensive guards for None/empty LLM responses - client.py: return '' early when provider returns None/empty in call_text_generation and call_image_understanding - io.py: raise ValueError early when prompt is None or empty in split_semantic_and_data_in_t2i_prompt - eval_engine.py: avoid literal 'None' string by checking response is not None before str() conversion - test_io.py: add tests for empty string and None prompt inputs --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d367e79 commit 2bd1c7c

21 files changed

Lines changed: 607 additions & 21 deletions

.github/workflows/ci.yml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,11 @@ jobs:
4646
run: uv sync --dev
4747

4848
- name: Run unit tests
49-
run: uv run pytest tests/ -v --tb=short
49+
run: uv run pytest tests/ -v --tb=short --cov=igenbench --cov-report=term-missing --cov-report=xml
50+
51+
- name: Upload coverage report
52+
if: matrix.python-version == '3.12'
53+
uses: actions/upload-artifact@v4
54+
with:
55+
name: coverage-report
56+
path: coverage.xml

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,5 +146,5 @@ outputs/
146146
- **Never** add API keys to source files. Use environment variables (see `.env.example`).
147147
- `VISItem.from_dict(path)` takes a **file path**, not a dict.
148148
- `EvalEntry.judgments` is a list — multiple (gen_model, eval_model) pairs can coexist on the same question (multi-model comparison).
149-
- The `Question` dataclass in `vis_item.py` is **deprecated**; use `EvalEntry` instead.
149+
- The `Question` dataclass in `vis_item.py` is **deprecated** and will be removed in a future version. Migrate to `EvalEntry`: replace `q``question`, `q_ground``ground`, `q_type``question_type`, and use `VISItem.evaluation: list[EvalEntry]` instead of a separate JSONL questions file.
150150
- When editing CLI commands, register them via `igenbench/cli/main.py` imports (side-effect import pattern).

igenbench/cli/batch_cli.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def cmd_batch_gen(
2727
"--resume/--no-resume",
2828
help="Resume from existing state, skipping already generated images",
2929
),
30-
):
30+
) -> None:
3131
"""Batch generate infographic images for all VISItem JSON files in a directory."""
3232
json_files = sorted(Path(data_dir).glob("*.json"))
3333

@@ -79,7 +79,7 @@ def cmd_batch_eval(
7979
"--resume/--no-resume",
8080
help="Resume from existing state, skipping already evaluated questions",
8181
),
82-
):
82+
) -> None:
8383
"""Batch evaluate generated images for all VISItem JSON files in a directory."""
8484
json_files = sorted(Path(data_dir).glob("*.json"))
8585

igenbench/cli/eval_cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def cmd_run_evaluation(
3333
"--resume",
3434
help="Resume from output directory to skip already processed items",
3535
),
36-
):
36+
) -> None:
3737
"""Run evaluation on a generated image using pre-generated questions.
3838
3939
Evaluates the generated image on all questions in the VISItem.

igenbench/cli/gen_cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def cmd_gen(
2525
"--resume",
2626
help="Resume from output directory to skip already generated images",
2727
),
28-
):
28+
) -> None:
2929
"""Generate image from text prompt using text-to-image model.
3030
3131
Reads the VISItem and generates an image based on the t2i_prompt field.

igenbench/cli/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import igenbench.cli.score_cli # noqa: E402, F401
1212

1313

14-
def main():
14+
def main() -> None:
1515
"""Entry point for the IGenBench CLI."""
1616
app()
1717

igenbench/cli/score_cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def cmd_score(
3232
"--by-type/--no-by-type",
3333
help="Show accuracy breakdown by question type",
3434
),
35-
):
35+
) -> None:
3636
"""Aggregate and display evaluation accuracy scores from the output directory."""
3737
output_path = Path(output_dir)
3838

igenbench/engine/eval_engine.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,18 @@ def judge_entry(
4949
model=eval_model, image_path=image_path, prompt=question_judgment_prompt
5050
)
5151

52+
if isinstance(response, dict):
53+
analysis = response.get("analysis", "")
54+
answer = response.get("answer", "")
55+
else:
56+
analysis = str(response) if response is not None else ""
57+
answer = ""
58+
5259
judgment = Judgment(
5360
eval_model=eval_model,
5461
gen_model=gen_model,
55-
analysis=response.get("analysis", ""),
56-
answer=response.get("answer", ""),
62+
analysis=analysis,
63+
answer=answer,
5764
)
5865

5966
return judgment

igenbench/engine/gen_engine.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from PIL.Image import Image as PILImage
2+
13
from igenbench.engine.base_engine import BaseEngine
24
from igenbench.utils.llm.client import LLMClient
35
from igenbench.vis_item import VISItem
@@ -9,7 +11,7 @@ class GenEngine(BaseEngine):
911
def __init__(self, llm_client: LLMClient, model: str):
1012
super().__init__(llm_client, model)
1113

12-
def text2image(self, item: VISItem):
14+
def text2image(self, item: VISItem) -> PILImage:
1315
"""Generate image from text prompt.
1416
1517
Args:

igenbench/utils/io.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,22 @@ def get_model_name_from_image_path(image_path: Path | str) -> str:
100100
return stem.split("_")[-1]
101101

102102

103-
def split_senmantic_and_data_in_t2i_prompt(t2i_prompt: str) -> tuple[str, str]:
104-
"""
105-
Split the semantic and data parts of the T2I prompt.
103+
def split_semantic_and_data_in_t2i_prompt(t2i_prompt: str) -> tuple[str, str]:
104+
"""Split a T2I prompt into semantic description and data sections.
105+
106+
Expects the prompt to contain the separator "The given data is:".
107+
Returns a (semantic_part, data_part) tuple.
108+
109+
Raises:
110+
ValueError: If the separator is not found in the prompt.
106111
"""
107-
return t2i_prompt.split("The given data is:")
112+
if not t2i_prompt:
113+
raise ValueError("Prompt cannot be empty or None.")
114+
separator = "The given data is:"
115+
if separator not in t2i_prompt:
116+
raise ValueError(
117+
f"Prompt does not contain expected separator '{separator}'. "
118+
"Cannot split semantic and data sections."
119+
)
120+
parts = t2i_prompt.split(separator, maxsplit=1)
121+
return parts[0], parts[1]

0 commit comments

Comments
 (0)