Conversation
Introduce a protocol-driven comparison of pixel-level tumor bulk models, including Dino-NestedUNet, UNI2 dense heads, SegTME, and the current PLIP/heuristic baselines. Users can ingest their own H&E tiles or GeoJSON, attach GPU-dumped masks, and rank models on private Dice while using public sets only as a sanity check. Co-authored-by: Taobo Hu <hutaobo@users.noreply.github.com>
hutaobo
marked this pull request as ready for review
August 28, 2026 14:36
审查者指南新增一个由协议驱动的 H&E 肿瘤区域基准测试，包含独立的整体和细胞赛道、配对数据集/GeoJSON 数据集导入、可选的预计算基础模型预测结果、内置基线、私有数据优先的报告、CLI 工作流、文档以及端到端测试。 H&E 基准测试运行时序列图sequenceDiagram
actor User
participant CLI as spatho_cli
participant Protocol as he_benchmark_protocol
participant Models as model_adapters
participant Metrics as metrics
participant Report as benchmark_report
User->>CLI: run --protocol
CLI->>Protocol: run_benchmark(protocol)
Protocol->>Protocol: load_protocol(protocol)
Protocol->>Models: build_segmenters(models, prediction_dirs)
loop enabled datasets and cases
Protocol->>Models: available()
Models-->>Protocol: prediction mask or skipped reason
Protocol->>Metrics: case_metrics(pred, gt)
Metrics-->>Protocol: Dice, IoU, precision, recall, HD95
end
Protocol->>Report: write_benchmark_report(results)
Report-->>CLI: leaderboard, case metrics, overlays, agreement
CLI-->>User: JSON run summary
基准测试数据集与预测结果实体关系图erDiagram
DATASET ||--o{ CASE : contains
CASE ||--o| GROUND_TRUTH_MASK : has
CASE ||--o{ PREDICTION_MASK : receives
MODEL ||--o{ PREDICTION_MASK : produces
DATASET {
string dataset_id
string kind
}
CASE {
string case_id
string image
string split
boolean qualitative_only
}
GROUND_TRUTH_MASK {
string mask_path
}
PREDICTION_MASK {
string model_id
string mask_path
}
MODEL {
string model_id
string track
}
H&E 基准测试工作区与评估流程图flowchart LR
A["spatho he-benchmark init"] --> B["protocol.json + catalog.json"]
B --> C["ingest paired images/masks or GeoJSON"]
C --> D["cases.jsonl"]
D --> E["doctor --protocol"]
E --> F["run --protocol"]
P["GPU models"] --> Q["Aligned per-case masks"]
Q --> F
F --> G["Private-first leaderboard"]
F --> H["Per-case metrics"]
F --> I["Overlays + model agreement"]
文件级变更
提示和命令与 Sourcery 交互
自定义你的体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's GuideAdds a protocol-driven H&E tumor-region benchmark with separate bulk and cell tracks, paired/GeoJSON dataset ingestion, optional precomputed foundation-model predictions, built-in baselines, private-first reporting, CLI workflows, documentation, and end-to-end tests. Sequence diagram for the H&E benchmark runsequenceDiagram
actor User
participant CLI as spatho_cli
participant Protocol as he_benchmark_protocol
participant Models as model_adapters
participant Metrics as metrics
participant Report as benchmark_report
User->>CLI: run --protocol
CLI->>Protocol: run_benchmark(protocol)
Protocol->>Protocol: load_protocol(protocol)
Protocol->>Models: build_segmenters(models, prediction_dirs)
loop enabled datasets and cases
Protocol->>Models: available()
Models-->>Protocol: prediction mask or skipped reason
Protocol->>Metrics: case_metrics(pred, gt)
Metrics-->>Protocol: Dice, IoU, precision, recall, HD95
end
Protocol->>Report: write_benchmark_report(results)
Report-->>CLI: leaderboard, case metrics, overlays, agreement
CLI-->>User: JSON run summary
Entity relationship diagram for benchmark datasets and predictionserDiagram
DATASET ||--o{ CASE : contains
CASE ||--o| GROUND_TRUTH_MASK : has
CASE ||--o{ PREDICTION_MASK : receives
MODEL ||--o{ PREDICTION_MASK : produces
DATASET {
string dataset_id
string kind
}
CASE {
string case_id
string image
string split
boolean qualitative_only
}
GROUND_TRUTH_MASK {
string mask_path
}
PREDICTION_MASK {
string model_id
string mask_path
}
MODEL {
string model_id
string track
}
Flow diagram for H&E benchmark workspace and evaluationflowchart LR
A["spatho he-benchmark init"] --> B["protocol.json + catalog.json"]
B --> C["ingest paired images/masks or GeoJSON"]
C --> D["cases.jsonl"]
D --> E["doctor --protocol"]
E --> F["run --protocol"]
P["GPU models"] --> Q["Aligned per-case masks"]
Q --> F
F --> G["Private-first leaderboard"]
F --> H["Per-case metrics"]
F --> I["Overlays + model agreement"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
你好——我发现了 5 个问题
面向 AI Agent 的提示
请处理这次代码审查中的评论：
## 各条评论
### 评论 1
<location path="src/spatho/he_benchmark/protocol.py" line_range="271" />
<code_context>
+
+ leaderboard = _build_leaderboard(case_rows, protocol.models)
+ agreement = _inter_model_agreement(predictions_by_case, protocol.models)
+ report = write_benchmark_report(
+ output_dir=output_dir,
+ protocol=json.loads(protocol.model_dump_json()),
</code_context>
<issue_to_address>
**问题 (bug_risk)：** `run_benchmark` 调用 `_inter_model_agreement(predictions_by_case, protocol.models)` 时缺少必需的 `model_ids` 参数，因此每次基准测试在完成用例评分后都会抛出 `TypeError`，且永远不会写入报告。
**建议修复：** 仅当相应地减少函数签名中的参数时，才将 `protocol.models` 作为第二个参数传入；否则，请根据预期 API 提供缺失的第三个参数。
</issue_to_address>
### 评论 2
<location path="src/spatho/he_benchmark/models.py" line_range="107-121" />
<code_context>
+ def available(self) -> tuple[bool, str]:
</code_context>
<issue_to_address>
**问题 (bug_risk)：** 当 `PLIPZeroShotContourClassifier` 成功导入时，`PlipFullTileSegmenter.available()` 会报告 PLIP 可运行；但 `predict()` 需要一个不存在的 `classify_array` 方法，导致每个 PLIP 用例都抛出 `RuntimeError`，而不是生成掩码或跳过该模型。
**触发条件：** 启用 `plip_fulltile` 且 `pathology_ai_service` 依赖成功导入时。
**建议修复：** 让分割器适配分类器实际的 `classify` 请求/响应 API，或者在兼容的推理适配器存在之前，让 `available()` 返回 false。
</issue_to_address>
### 评论 3
<location path="src/spatho/he_benchmark/protocol.py" line_range="39" />
<code_context>
+
+ name: str = Field(default="he_tumor_region_v1", min_length=1)
+ tile_size_px: int = Field(default=1024, ge=64, le=4096)
+ models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2"])
+ datasets: list[DatasetRef] = Field(default_factory=list)
+ prediction_dirs: dict[str, Path] = Field(default_factory=dict)
</code_context>
<issue_to_address>
**问题 (broader_impact)：** `init_benchmark` 创建的默认协议遗漏了 `plip_fulltile`，尽管模型目录将其标记为必需的 bulk 模型，且基准测试说明称其为必须测试的基线。因此，文档所述的初始化并运行流程不会评估当前产品基线。
**触发条件：** 用户运行 `he-benchmark init`，随后直接运行生成的协议，且未手动编辑模型列表时。
**建议修复：** 将 `plip_fulltile` 加入默认模型列表，或者明确将生成的协议标记为不完整，并要求用户添加该模型。
```suggestion
models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2", "plip_fulltile"])
```
</issue_to_address>
### 评论 4
<location path="src/spatho/he_benchmark/protocol.py" line_range="292-310" />
<code_context>
+ return summary
+
+
+def _build_leaderboard(case_rows: list[dict[str, Any]], model_ids: list[str]) -> list[dict[str, Any]]:
+ board: list[dict[str, Any]] = []
+ for kind in ("private", "public"):
+ for model_id in model_ids:
+ rows = [
+ row
+ for row in case_rows
+ if row["model_id"] == model_id and row.get("kind") == kind and not row.get("qualitative_only")
+ ]
+ if not rows:
+ continue
+ summary = summarize_metrics(rows)
+ spec = MODEL_CATALOG.get(model_id, {})
+ board.append(
+ {
+ "kind": kind,
+ "model_id": model_id,
+ "track": spec.get("track"),
+ "role": spec.get("role"),
+ **summary,
+ }
</code_context>
<issue_to_address>
**问题 (bug_risk)：** `_build_leaderboard` 遍历协议中的每个模型，并且不考虑 `spec["track"]`，将它们全部添加到同一个 private/public 排行榜中。因此，`segtme_uni2` 的肿瘤细胞 Dice 会与 bulk 肿瘤模型一起排名，而不是在单独的细胞表中报告。
**触发条件：** 协议包含 `segtme_uni2`，且至少有一个用例具有真实标注时。
**建议修复：** 按 track 对排行榜行进行分区，并将肿瘤细胞 track 与 `pixel_tumor_bulk` 分开呈现。
</issue_to_address>
### 评论 5
<location path="src/spatho/he_benchmark/protocol.py" line_range="148-151" />
<code_context>
+ n_cases = 0
+ n_masks = 0
+ if exists:
+ cases = read_cases_jsonl(dataset.cases_path)
+ n_cases = len(cases)
+ n_masks = sum(1 for case in cases if case.get("mask"))
+ n_scored += n_masks
+ else:
+ if dataset.enabled:
</code_context>
<issue_to_address>
**问题 (bug_risk)：** 只要 JSON 记录中的 `mask` 字段为真值，`doctor_benchmark` 就会将该用例计为已评分，而不会检查所引用的图像和掩码文件是否存在。因此，doctor 可能会报告协议已准备就绪，但随后 `iter_cases` 会因 `FileNotFoundError` 崩溃。
**触发条件：** `cases.jsonl` 包含过期或无效的图像/掩码路径时。
**建议修复：** 在 doctor 检查期间解析并验证每个引用的图像和掩码路径，并在允许运行前将缺失文件报告为问题。
</issue_to_address>Sourcery 评估
需要人工审查。 有 5 个发现需要优先处理；如果基准测试逻辑或掩码处理存在错误，可能会写入误导性的 Dice/排行榜报告，并导致用户选择错误的肿瘤区域模型。回滚会移除该功能，而修正后的代码可以重新生成范围受限的本地报告和预测结果。
阻塞性发现：src/spatho/he_benchmark/protocol.py:271、src/spatho/he_benchmark/models.py:121、src/spatho/he_benchmark/protocol.py:39、src/spatho/he_benchmark/protocol.py:310、src/spatho/he_benchmark/protocol.py:151
帮助我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈改进审查结果。
Original comment in English
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/spatho/he_benchmark/protocol.py" line_range="271" />
<code_context>
+
+ leaderboard = _build_leaderboard(case_rows, protocol.models)
+ agreement = _inter_model_agreement(predictions_by_case, protocol.models)
+ report = write_benchmark_report(
+ output_dir=output_dir,
+ protocol=json.loads(protocol.model_dump_json()),
</code_context>
<issue_to_address>
**issue (bug_risk):** `run_benchmark` calls `_inter_model_agreement(predictions_by_case, protocol.models)` without the required `model_ids` argument, so every benchmark run raises `TypeError` after scoring cases and never writes its report.
**Suggested fix:** Pass `protocol.models` as the second argument only if the function signature is reduced accordingly, or supply the missing third argument according to the intended API.
</issue_to_address>
### Comment 2
<location path="src/spatho/he_benchmark/models.py" line_range="107-121" />
<code_context>
+ def available(self) -> tuple[bool, str]:
</code_context>
<issue_to_address>
**issue (bug_risk):** `PlipFullTileSegmenter.available()` reports PLIP as runnable when `PLIPZeroShotContourClassifier` imports, but `predict()` requires a nonexistent `classify_array` method and raises `RuntimeError` for every PLIP case instead of producing a mask or skipping the model.
**Triggers:** When `plip_fulltile` is enabled and the `pathology_ai_service` dependency imports successfully.
**Suggested fix:** Adapt the segmenter to the classifier's actual `classify` request/response API, or make `available()` return false until a compatible inference adapter exists.
</issue_to_address>
### Comment 3
<location path="src/spatho/he_benchmark/protocol.py" line_range="39" />
<code_context>
+
+ name: str = Field(default="he_tumor_region_v1", min_length=1)
+ tile_size_px: int = Field(default=1024, ge=64, le=4096)
+ models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2"])
+ datasets: list[DatasetRef] = Field(default_factory=list)
+ prediction_dirs: dict[str, Path] = Field(default_factory=dict)
</code_context>
<issue_to_address>
**issue (broader_impact):** The default protocol created by `init_benchmark` omits `plip_fulltile`, even though the catalog marks it as a required bulk model and the benchmark description says it is a must-test baseline, so the documented init-and-run workflow never evaluates the current product baseline.
**Triggers:** When users run `he-benchmark init` and then run the generated protocol without manually editing its model list.
**Suggested fix:** Include `plip_fulltile` in the default model list, or explicitly mark the generated protocol as incomplete and require users to add it.
```suggestion
models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2", "plip_fulltile"])
```
</issue_to_address>
### Comment 4
<location path="src/spatho/he_benchmark/protocol.py" line_range="292-310" />
<code_context>
+ return summary
+
+
+def _build_leaderboard(case_rows: list[dict[str, Any]], model_ids: list[str]) -> list[dict[str, Any]]:
+ board: list[dict[str, Any]] = []
+ for kind in ("private", "public"):
+ for model_id in model_ids:
+ rows = [
+ row
+ for row in case_rows
+ if row["model_id"] == model_id and row.get("kind") == kind and not row.get("qualitative_only")
+ ]
+ if not rows:
+ continue
+ summary = summarize_metrics(rows)
+ spec = MODEL_CATALOG.get(model_id, {})
+ board.append(
+ {
+ "kind": kind,
+ "model_id": model_id,
+ "track": spec.get("track"),
+ "role": spec.get("role"),
+ **summary,
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** `_build_leaderboard` iterates over every protocol model and appends them to the same private/public leaderboard regardless of `spec["track"]`, so `segtme_uni2` neoplastic-cell Dice is ranked alongside bulk tumor models instead of being reported in a separate cell table.
**Triggers:** Whenever `segtme_uni2` is included in a protocol and at least one case has ground truth.
**Suggested fix:** Partition leaderboard rows by track and render the neoplastic-cell track separately from `pixel_tumor_bulk`.
</issue_to_address>
### Comment 5
<location path="src/spatho/he_benchmark/protocol.py" line_range="148-151" />
<code_context>
+ n_cases = 0
+ n_masks = 0
+ if exists:
+ cases = read_cases_jsonl(dataset.cases_path)
+ n_cases = len(cases)
+ n_masks = sum(1 for case in cases if case.get("mask"))
+ n_scored += n_masks
+ else:
+ if dataset.enabled:
</code_context>
<issue_to_address>
**issue (bug_risk):** `doctor_benchmark` counts a case as scored whenever its JSON record has a truthy `mask` field, without checking that the referenced image and mask files exist, so doctor can report a ready protocol that subsequently crashes in `iter_cases` with `FileNotFoundError`.
**Triggers:** When `cases.jsonl` contains stale or invalid image/mask paths.
**Suggested fix:** Resolve and validate every referenced image and mask path during doctor checks, and report missing files as issues before allowing a run.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 5 findings to address first, and if the benchmark logic or mask handling is wrong, it could write misleading Dice/leaderboard reports and cause users to choose the wrong tumor-region model. Reverting removes the feature, and the bounded local reports and predictions can be regenerated with corrected code.
Blocking findings: src/spatho/he_benchmark/protocol.py:271, src/spatho/he_benchmark/models.py:121, src/spatho/he_benchmark/protocol.py:39, src/spatho/he_benchmark/protocol.py:310, src/spatho/he_benchmark/protocol.py:151
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| leaderboard = _build_leaderboard(case_rows, protocol.models) | ||
| agreement = _inter_model_agreement(predictions_by_case, protocol.models) | ||
| report = write_benchmark_report( |
There was a problem hiding this comment.
问题 (bug_risk)： run_benchmark 调用 _inter_model_agreement(predictions_by_case, protocol.models) 时缺少必需的 model_ids 参数，因此每次基准测试在完成用例评分后都会抛出 TypeError，且永远不会写入报告。
建议修复： 仅当相应地减少函数签名中的参数时，才将 protocol.models 作为第二个参数传入；否则，请根据预期 API 提供缺失的第三个参数。
Original comment in English
issue (bug_risk): run_benchmark calls _inter_model_agreement(predictions_by_case, protocol.models) without the required model_ids argument, so every benchmark run raises TypeError after scoring cases and never writes its report.
Suggested fix: Pass protocol.models as the second argument only if the function signature is reduced accordingly, or supply the missing third argument according to the intended API.
Comment on lines
+107
to
+121
| def available(self) -> tuple[bool, str]: | ||
| try: | ||
| from pathology_ai_service.core import PLIPZeroShotContourClassifier | ||
| except Exception as exc: | ||
| return False, f"PLIP classifier unavailable: {exc}" | ||
| return True, "pathology_ai_service.PLIPZeroShotContourClassifier" | ||
|
|
||
| def predict(self, image: np.ndarray, *, case: dict[str, Any]) -> np.ndarray: | ||
| from pathology_ai_service.core import PLIPZeroShotContourClassifier | ||
|
|
||
| classifier = PLIPZeroShotContourClassifier() | ||
| rgb = _as_rgb(image) | ||
| result = classifier.classify_array(rgb) if hasattr(classifier, "classify_array") else None | ||
| if result is None: | ||
| raise RuntimeError("PLIP classifier does not expose classify_array; skip this model.") |
There was a problem hiding this comment.
问题 (bug_risk)： 当 PLIPZeroShotContourClassifier 成功导入时，PlipFullTileSegmenter.available() 会报告 PLIP 可运行；但 predict() 需要一个不存在的 classify_array 方法，导致每个 PLIP 用例都抛出 RuntimeError，而不是生成掩码或跳过该模型。
触发条件： 启用 plip_fulltile 且 pathology_ai_service 依赖成功导入时。
建议修复： 让分割器适配分类器实际的 classify 请求/响应 API，或者在兼容的推理适配器存在之前，让 available() 返回 false。
Original comment in English
issue (bug_risk): PlipFullTileSegmenter.available() reports PLIP as runnable when PLIPZeroShotContourClassifier imports, but predict() requires a nonexistent classify_array method and raises RuntimeError for every PLIP case instead of producing a mask or skipping the model.
Triggers: When plip_fulltile is enabled and the pathology_ai_service dependency imports successfully.
Suggested fix: Adapt the segmenter to the classifier's actual classify request/response API, or make available() return false until a compatible inference adapter exists.
|
|
||
| name: str = Field(default="he_tumor_region_v1", min_length=1) | ||
| tile_size_px: int = Field(default=1024, ge=64, le=4096) | ||
| models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2"]) |
There was a problem hiding this comment.
问题 (broader_impact)： init_benchmark 创建的默认协议遗漏了 plip_fulltile，尽管模型目录将其标记为必需的 bulk 模型，且基准测试说明称其为必须测试的基线。因此，文档所述的初始化并运行流程不会评估当前产品基线。
触发条件： 用户运行 he-benchmark init，随后直接运行生成的协议，且未手动编辑模型列表时。
建议修复： 将 plip_fulltile 加入默认模型列表，或者明确将生成的协议标记为不完整，并要求用户添加该模型。
Suggested change
| models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2"]) | |
| models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2", "plip_fulltile"]) |
Original comment in English
issue (broader_impact): The default protocol created by init_benchmark omits plip_fulltile, even though the catalog marks it as a required bulk model and the benchmark description says it is a must-test baseline, so the documented init-and-run workflow never evaluates the current product baseline.
Triggers: When users run he-benchmark init and then run the generated protocol without manually editing its model list.
Suggested fix: Include plip_fulltile in the default model list, or explicitly mark the generated protocol as incomplete and require users to add it.
Suggested change
| models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2"]) | |
| models: list[str] = Field(default_factory=lambda: ["stain_threshold", "dino_nested_unet", "uni2_upernet", "segtme_uni2", "plip_fulltile"]) |
Comment on lines
+292
to
+310
| def _build_leaderboard(case_rows: list[dict[str, Any]], model_ids: list[str]) -> list[dict[str, Any]]: | ||
| board: list[dict[str, Any]] = [] | ||
| for kind in ("private", "public"): | ||
| for model_id in model_ids: | ||
| rows = [ | ||
| row | ||
| for row in case_rows | ||
| if row["model_id"] == model_id and row.get("kind") == kind and not row.get("qualitative_only") | ||
| ] | ||
| if not rows: | ||
| continue | ||
| summary = summarize_metrics(rows) | ||
| spec = MODEL_CATALOG.get(model_id, {}) | ||
| board.append( | ||
| { | ||
| "kind": kind, | ||
| "model_id": model_id, | ||
| "track": spec.get("track"), | ||
| "role": spec.get("role"), |
There was a problem hiding this comment.
问题 (bug_risk)： _build_leaderboard 遍历协议中的每个模型，并且不考虑 spec["track"]，将它们全部添加到同一个 private/public 排行榜中。因此，segtme_uni2 的肿瘤细胞 Dice 会与 bulk 肿瘤模型一起排名，而不是在单独的细胞表中报告。
触发条件： 协议包含 segtme_uni2，且至少有一个用例具有真实标注时。
建议修复： 按 track 对排行榜行进行分区，并将肿瘤细胞 track 与 pixel_tumor_bulk 分开呈现。
Original comment in English
issue (bug_risk): _build_leaderboard iterates over every protocol model and appends them to the same private/public leaderboard regardless of spec["track"], so segtme_uni2 neoplastic-cell Dice is ranked alongside bulk tumor models instead of being reported in a separate cell table.
Triggers: Whenever segtme_uni2 is included in a protocol and at least one case has ground truth.
Suggested fix: Partition leaderboard rows by track and render the neoplastic-cell track separately from pixel_tumor_bulk.
Comment on lines
+148
to
+151
| cases = read_cases_jsonl(dataset.cases_path) | ||
| n_cases = len(cases) | ||
| n_masks = sum(1 for case in cases if case.get("mask")) | ||
| n_scored += n_masks |
There was a problem hiding this comment.
问题 (bug_risk)： 只要 JSON 记录中的 mask 字段为真值，doctor_benchmark 就会将该用例计为已评分，而不会检查所引用的图像和掩码文件是否存在。因此，doctor 可能会报告协议已准备就绪，但随后 iter_cases 会因 FileNotFoundError 崩溃。
触发条件： cases.jsonl 包含过期或无效的图像/掩码路径时。
建议修复： 在 doctor 检查期间解析并验证每个引用的图像和掩码路径，并在允许运行前将缺失文件报告为问题。
Original comment in English
issue (bug_risk): doctor_benchmark counts a case as scored whenever its JSON record has a truthy mask field, without checking that the referenced image and mask files exist, so doctor can report a ready protocol that subsequently crashes in iter_cases with FileNotFoundError.
Triggers: When cases.jsonl contains stale or invalid image/mask paths.
Suggested fix: Resolve and validate every referenced image and mask path during doctor checks, and report missing files as issues before allowing a run.
There was a problem hiding this comment.
🟡 Changes recommended
There are a few concrete operational/API issues (GeoJSON rasterization memory blow-up risk, masks_dir validation, and unnecessary in-memory prediction retention) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a protocol-driven, CLI-accessible H&E pixel-level tumor-region benchmarking workflow to compare multiple segmentation approaches (heuristic, PLIP baseline, and external/foundation-model mask dumps) with private-slide ranking prioritized and public sets used for sanity checks.
Changes:
- Introduces the
spatho he-benchmarkCLI (catalog/init/ingest/doctor/run) plus a protocol model and report generation. - Adds dataset ingestion utilities (paired image/mask dirs, single image + GeoJSON rasterization) and pixel-level metrics (Dice/IoU/HD95/etc.).
- Adds documentation and a new pytest suite for the benchmark workflow.
File summaries
| File | Description |
|---|---|
| tests/test_he_benchmark.py | Adds end-to-end and unit tests for catalog, metrics, ingestion, and CLI smoke runs. |
| src/spatho/he_benchmark/report.py | Writes leaderboard/case/agreement artifacts and overlay rendering utilities. |
| src/spatho/he_benchmark/protocol.py | Implements protocol schema, init/doctor/run pipeline, leaderboard + agreement computation. |
| src/spatho/he_benchmark/models.py | Adds segmenter adapters (heuristic, PLIP, external mask dirs, checkpoint placeholder). |
| src/spatho/he_benchmark/metrics.py | Adds binary-mask conversion, overlap metrics, and HD95 computation. |
| src/spatho/he_benchmark/datasets.py | Adds ingestion helpers, array IO, synthetic fixture, and GeoJSON rasterization. |
| src/spatho/he_benchmark/catalog.py | Defines the frozen model/dataset catalog and protocol rules. |
| src/spatho/he_benchmark/init.py | Exposes benchmark public API symbols. |
| src/spatho/cli.py | Wires he-benchmark subcommands into the main CLI. |
| README.md | Documents the new benchmark CLI entry points. |
| docs/index.md | Adds the benchmark page to the docs landing page and toctree. |
| docs/HE_TUMOR_REGION_BENCHMARK.md | Provides detailed usage docs, protocol rules, and prediction mask contract. |
| docs/DEVELOPMENT_GUIDE.md | Lists the new CLI command in the development guide. |
Review details
Suppressed comments (1)
src/spatho/he_benchmark/datasets.py:193
- ingest_paired_directories silently treats a missing --masks directory as “no masks” (qualitative-only) because _index_by_stem returns an empty dict when the directory doesn’t exist. If the caller explicitly provides masks_dir, fail fast so a typo/path issue doesn’t produce an all-qualitative dataset.
images_dir = Path(images_dir).expanduser().resolve()
output_dir = Path(output_dir).expanduser().resolve()
image_index = _index_by_stem(images_dir, IMAGE_SUFFIXES)
mask_index = _index_by_stem(Path(masks_dir).expanduser().resolve(), MASK_SUFFIXES) if masks_dir else {}
if not image_index:
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+111
to
+113
| ys, xs = np.mgrid[0:height, 0:width] | ||
| points = np.stack([xs.ravel(), ys.ravel()], axis=1) | ||
| mask = np.zeros(points.shape[0], dtype=bool) |
Comment on lines
+235
to
+248
| case_id = str(loaded["case_id"]) | ||
| predictions_by_case.setdefault(case_id, {}) | ||
| wrote_overlay = False | ||
| for model_id, segmenter in runnable.items(): | ||
| pred = segmenter.predict(image, case=loaded) | ||
| predictions_by_case[case_id][model_id] = pred | ||
| row: dict[str, Any] = { | ||
| "case_id": case_id, | ||
| "dataset_id": dataset.dataset_id, | ||
| "kind": dataset.kind, | ||
| "model_id": model_id, | ||
| "qualitative_only": bool(gt is None or loaded.get("qualitative_only")), | ||
| "image": loaded.get("image"), | ||
| } |
Comment on lines
+155
to
+158
| "skipped_models_json": str(skipped_json), | ||
| "protocol_snapshot_json": str(protocol_json), | ||
| "n_overlays": str(len(overlays)), | ||
| } |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a protocol-driven H&E pixel-level tumor-region benchmark so we can (1) reproduce the current strongest bulk/cell methods on public tiles and (2) rank those same methods on private H&E.
The comparison is frozen as a catalog, not a notebook:
stain_threshold(weak baseline),plip_fulltile(current product),dino_nested_unet(published tumor-bulk SOTA),uni2_upernet(pathology FM dense head). Optional:uni2_unetr.segtme_uni2(neoplastic class). Do not mix into bulk Dice.private_he. Public CAMELYON16 / TIGER WSIBULK are sanity checks only.Foundation-model weights stay out of this repo. GPU boxes dump aligned masks;
prediction_dirsscores them fairly next to the always-on heuristic.How to use
Private GeoJSON contours can be rasterized with
--image+--geojson. Cases without masks still get overlays and inter-model agreement.Testing
python3 -m pytest tests/test_he_benchmark.py -q— 7 passedstain_thresholdDice 0.916 (recall 1.0; dark artifact is an expected false positive)Synthetic overlay
To show artifacts inline, enable in settings.
Sourcery 摘要
新增一个可复现的 H&E 肿瘤区域基准测试工作流，用于在公开 sanity 数据集和私有切片上比较分割模型。
新功能：
增强功能：
文档：
测试：
Original summary in English
Sourcery 总结
新增一个可复现的 H&E 肿瘤区域基准测试，用于在公开切片块上验证模型，并在私有切片上对模型进行排名。
新功能：
增强功能：
文档：
测试：
Original summary in English
Summary by Sourcery
Add a reproducible H&E tumor-region benchmark for validating models on public tiles and ranking them on private slides.
New Features:
Enhancements:
Documentation:
Tests: