feat: add lightweight GraphRAG benchmark - #77
Closed
Postroggy wants to merge 18 commits into
Closed
Conversation
|
@codecov-ai-reviewer review |
Walkthrough本 PR 新增 hugegraph-llm 的独立 benchmark 子系统，覆盖答案、抽取、检索三类指标，补齐基线保存/对比、Markdown/JSON 报告、CLI 运行入口、外部数据集转换、pipeline 适配和大量测试与文档。 ChangesGraphRAG Benchmark 评测能力
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Runner
participant MetricRegistry
participant LLMJudge
participant BaselineStore
participant MarkdownReporter
User->>CLI: run --mode ...
CLI->>Runner: run(data_path, metrics)
Runner->>MetricRegistry: create(metric_name)
Runner->>LLMJudge: judge(...) %% 可选
LLMJudge-->>Runner: score/reason
Runner-->>CLI: BenchmarkResult
CLI->>BaselineStore: save(result) %% 可选
CLI->>MarkdownReporter: report(result, comparison)
MarkdownReporter-->>User: Markdown / JSON
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Postroggy
force-pushed
the
feat/graphrag-benchmark-issue7
branch
from
July 1, 2026 18:25
22a4877 to
fbb0823
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive GraphRAG Benchmark evaluation suite to the hugegraph-llm project, adding various metrics for graph extraction, document retrieval, and answer generation, along with orchestration runners, CLI commands, and extensive tests. The code review identified several critical bugs and improvement opportunities, including a matching bug in symmetric relation conflict detection due to underscore normalization, incorrect repository root fallback paths in shell scripts, a too-permissive Unix timestamp check in temporal validity, and a missing question-type re-computation when filtering samples in the CLI. Additionally, optimizations were suggested for memory reuse in ROUGE-L calculation and cleaner string representation of reference lists in context precision.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Postroggy
force-pushed
the
feat/graphrag-benchmark-issue7
branch
from
July 2, 2026 02:24
fbb0823 to
d178b1e
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Postroggy
force-pushed
the
feat/graphrag-benchmark-issue7
branch
from
July 2, 2026 02:50
d178b1e to
d789fbc
Compare
Postroggy
force-pushed
the
feat/graphrag-benchmark-issue7
branch
from
July 2, 2026 03:23
d789fbc to
801db09
Compare
imbajin
requested changes
Jul 3, 2026
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
❗️ 总体结论：建议先 Request changes，暂不建议直接合并。综合评分：6.0 / 10。
方向是对的：把 GraphRAG benchmark 做成 hugegraph_llm.benchmark 下相对独立、离线优先、可用于 PR 回归对比的工具，这个目标很适合 HugeGraph-LLM 的工程场景。但当前 PR 的核心问题不是“指标数量不够”，而是 benchmark 数据契约和 metric 输入边界没有收敛。在 benchmark 领域，最重要的是分数可信、可解释、可复现；如果契约混用，report 越完整越容易误导用户。
当前设计里有三类概念被混在一起：
flowchart LR
A[retrieved_docs / gold_docs<br/>doc id 列表] --> B[离线 ranking metrics<br/>recall@k / hit@k / mrr]
A -. 当前也传给 .-> C[LLM Judge metrics<br/>context_precision / evidence_recall]
D[retrieved_contexts<br/>文本片段] --> C
E[gold_evidence / gold_answer<br/>证据与答案文本] --> C
建议把首版目标收窄到一个更稳的最小闭环：
数据格式清晰 -> metric 输入明确 -> 分数方向明确 -> compare 不误判 -> report 可解释
合并前建议至少完成：
- ❗️ 修复
graph_structure中顶点 ID 与边端点 ID 不一致导致的拓扑指标错误。 - ❗️ 明确 retrieval 数据契约：doc id、context text、gold evidence、gold answer 需要分字段，不要让 LLM metric 吃 doc id。
- ❗️ 给 metric 增加
higher_is_better/ direction 元信息，修复 baseline compare 对 error/rate 指标的反向判断。 ⚠️ CLI 文档与实现对齐：baseline save/list-metrics要么实现，要么从文档删除。⚠️ invalid metric、mode/data 不匹配、offline 下显式请求 LLM metric 都需要 fail-fast 或显式skipped_metrics。⚠️ 补关键回归测试，而不只是 toy fixture 的“能跑”。
已有评论线程中 resolved/outdated 的问题我这里不重复。下面 inline comments 主要聚焦仍然影响设计、执行正确性、用户易用性和测试有效性的点。
| in_v = str(_edge_in(e)) | ||
| edge_label = str(e.get("label", "")) | ||
| if out_v and in_v: | ||
| g.add_edge(out_v, in_v, label=edge_label) |
There was a problem hiding this comment.
❗️ 这里的节点 ID 和边端点 ID 使用了两套规则，会系统性算错图拓扑指标。
上面添加节点时，如果 vertex 有 label，会使用 label:name：
vertex: {label: person, name: Alice} -> node_id = person:Alice
但这里添加边时直接使用 _edge_out/_edge_in 返回的裸端点：
edge: {source: Alice, target: Bob} -> Alice -> Bob
NetworkX 会自动把 Alice / Bob 当成新节点，导致一个正常的 2 点 1 边图变成：
{ person:Alice, person:Bob, Alice, Bob }
这样 num_nodes、density、num_components、largest_component_ratio 都会被污染。建议抽一个统一的 endpoint canonicalization helper，例如先构建 name -> canonical_node_id 映射，边端点统一映射到同一套 node id。
| sample_id=sample_id, | ||
| question=sample.get("question", ""), | ||
| context=sample.get("retrieved_docs", []), | ||
| ground_truth=sample.get("gold_answer", sample.get("gold_docs", [])), |
There was a problem hiding this comment.
❗️ 这里把 retrieved_docs / gold_docs 同时传给所有 retrieval metrics，导致 doc-id 指标和 LLM/context 指标的输入契约混在一起。
recall@k / hit@k / mrr 吃 doc id 没问题；但 context_precision、context_relevancy、evidence_recall_llm 需要的是 context text / evidence text。当前如果 sample 只有 doc_paris 这类 id，LLM metric 会把 doc id 当上下文或证据来评估，得到的是“能跑”的分数，不是可信分数。
建议数据结构拆开：
{
"gold_doc_ids": ["doc_a"],
"retrieved_doc_ids": ["doc_a", "doc_b"],
"retrieved_contexts": [{"id": "doc_a", "text": "..."}],
"gold_evidence": ["..."],
"gold_answer": "..."
}runner 按 metric 类型选择输入；缺少文本时应显式 skip LLM metric，而不是默默把 doc id 传进去。
| if _is_llm_judge_metric(metric): | ||
| effective_delta = max(delta, cls.DEFAULT_LLM_JUDGE_DELTA) | ||
|
|
||
| if diff < -effective_delta: |
There was a problem hiding this comment.
❗️ baseline compare 需要知道每个 metric 的方向，否则会把低越好的指标反向解释。
当前统一用 candidate - baseline：
正数 => improvement
负数 => regression
这对 f1/recall/precision 成立，但对 illegal_edge_rate、orphan_edge_rate、duplicate_entity_rate、duplicate_edge_rate、conflict_rate、num_conflicts 这类 error/rate 指标是反的。例如：
illegal_edge_rate: 0.10 -> 0.30
当前逻辑：+0.20 => improvement
实际语义：更差，应该是 regression
建议给 metric 加一个极简元信息即可，不需要上复杂框架：
higher_is_better: bool = Truecompare 时按方向归一化 delta，再判断 regression / improvement。
| metrics so they can be selected explicitly, e.g. ``--metrics coverage``. | ||
| """ | ||
| allowed = set(_MODE_ALLOWED_METRICS.get(mode_key, [])) | ||
| return [m for m in metrics if m in allowed] |
There was a problem hiding this comment.
如果用户写错 metric 名，或把 extraction metric 传给 retrieval mode，当前会被直接 drop；极端情况下 metrics 为空，runner 仍可能输出一个看似成功的 report。benchmark 工具更应该 fail-fast，避免用户在 PR 里贴出空结果或不完整结果。
建议：
requested_metrics - allowed_metrics != empty -> exit 2 + 打印非法项和可选项
filtered_metrics == empty -> exit 2
这样更符合工程回归工具的预期。
| vl_name = normalize_answer(str(v.get("label", "")), language) | ||
| if vl_name not in vertex_labels_schema: | ||
| continue | ||
| primary_keys = vertex_labels_schema[vl_name].get("primary_keys", []) |
There was a problem hiding this comment.
required_property_fill 只检查 primary_keys，但样例 schema 使用的是 properties，没有 primary_keys。这会让“required property completeness”在当前样例和很多常见 schema 输入下偏乐观。
建议明确 schema contract：
required 字段来自哪里？
- primary_keys？
- nullable_keys 的反集？
- benchmark 自定义 required_properties？
不要把 properties 写在样例里，但实际不参与完整性校验；否则用户会以为缺少 name/age 会被扣分，实际不会。
|
|
||
| return round(sum(scores) / len(scores)) | ||
|
|
||
|
|
There was a problem hiding this comment.
🧹 双评估后这里直接 round() 会把 0.5 / 1.5 这类中间分压成整数，丢掉 dual-rating 的细粒度信息。
如果目标是降低 LLM 方差，建议保留平均值再归一化：
return sum(scores) / len(scores)最终 mean_score / 2.0 输出即可。
| answer_key = f"{mode}_answer" | ||
| prediction = sample.get(answer_key, "") | ||
|
|
||
| for metric_name, metric in metric_instances.items(): |
There was a problem hiding this comment.
例如某个 sample 没有 graph_vector_answer，当前会按 "" 继续计算，最后看起来像 graph+vector 表现很差，但真实原因可能只是数据缺字段。建议缺失字段时显式记录 sample error / skipped mode，或者在数据加载阶段校验必需字段。
| @@ -0,0 +1,22 @@ | |||
| { | |||
There was a problem hiding this comment.
❗️ 文件级：当前 retrieval sample 只有 doc id，没有 retrieved_contexts、gold_evidence、gold_answer。这与 PR 描述里的 context/evidence LLM-Judge 指标不匹配。
建议把样例拆成两类，避免用户误解：
retrieval_docid_sample.json
- gold_doc_ids
- retrieved_doc_ids
- 只跑 recall@k / hit@k / mrr
retrieval_context_sample.json
- question
- retrieved_contexts[].text
- gold_evidence[]
- gold_answer
- 可跑 context_precision / context_relevancy / evidence_recall_llm
这样既保持离线路径简洁，也不会让 LLM metric 在 doc id 上“假跑通”。
| @@ -0,0 +1,115 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
建议直接加：
def test_graphstructure_labeled_vertices_source_target_edges_no_extra_nodes():
prediction = {
"vertices": [
{"label": "person", "properties": {"name": "Alice"}},
{"label": "person", "properties": {"name": "Bob"}},
],
"edges": [{"label": "knows", "source": "Alice", "target": "Bob"}],
}
result = GraphStructure().calculate(prediction)
assert result["num_nodes"] == 2.0
assert result["num_edges"] == 1.0当前测试里有边的 case 都是无 label 顶点，无法发现 person:Alice 与 Alice 被当作两个节点的问题。
| @@ -0,0 +1,554 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
baseline save、list-metrics，但当前 parser 只注册了 run 和 compare。这会造成用户按 README/PR 描述执行命令时直接失败。
建议二选一：
方案 A：实现命令
hugegraph-benchmark baseline save ...
hugegraph-benchmark list-metrics
方案 B：收敛文档
统一改成 run --save-baseline
删除 list-metrics 相关描述
首版为了保持轻量，我更倾向先做方案 B；如果保留 list-metrics，也建议输出 requires_llm、higher_is_better、required_fields，帮助用户理解指标契约。
…egistry - Remove fallback LLM client in benchmark CLI; use internal OpenAI-compatible client with fixed temperature=0.0 and seed=42 for LLM-Judge metrics. - Record model/temperature/seed in BenchmarkResult.metadata and persist via BaselineStore.save. - Move metric direction metadata into metric classes; query via MetricRegistry.is_higher_is_better() in baseline comparator. - Update sample data, dataset converters, and runners for retrieval/extraction fields and answer runner. - Add/expand unit tests for CLI judge params and baseline metadata. Local infrastructure adjustments (kept local, not for upstream): - Add Jina reranker option and robust schema JSON parsing. - Add retry/rate-limit/timeout handling in OpenAI embedding and LLM clients.
…-ID validation - Implement normalize_graph_extract() to convert property_graph / triples extraction output into benchmark candidate_vertices / candidate_edges format. - Expose normalize_graph_extract from benchmark.utils. - Improve RetrievalRunner error messages when ranking metrics are requested but gold_doc_ids / retrieved_doc_ids are missing; fail-fast with guidance to use context / LLM-Judge metrics instead. - Add unit tests for graph extraction normalization and retrieval runner doc-ID contract validation.
…upstream) Stash local-only artifacts so they are not lost while benchmark module source stays on the PR branch. These changes are intended to stay local and must NOT be pushed upstream: - docs/benchmark/ experiment notes - scripts/benchmark/ one-off generation/preparation scripts - BENCHMARK_DATASETS.md / GRAPHRAG_BENCHMARK.md - temporary changes to graph_extract flow / operators / ai_state / tests used to run experiments
- Add normalize_schema() to accept JSON-string schema from pipeline output and return the dict expected by ExtractionRunner. - Add normalize_extraction_output() as the pipeline -> benchmark adapter for extraction: normalizes schema, converts vertices/edges to candidate fields, and preserves trace fields (raw_responses, parse_results) when collect_trace=True is enabled locally. - Expose both helpers from benchmark.utils. - Add unit tests for schema parsing and full extraction output normalization.
- Add build_retrieval_sample_from_state() to extract retrieved_contexts from WkFlowState.vector_result / graph_result for benchmark context and LLM-Judge metrics. - Support raw / vector_only / graph_only / graph_vector modes matching the four RAG flows. - Expose the adapter from benchmark.utils. - Add unit tests covering all modes, JSON-string input, and field mapping.
- Add a 'Failed Samples' section to the single-run Markdown report using runner-collected errors from result.metadata. - Add a 'Degraded Samples' section for single-run reports, listing samples whose metrics are at the worst bound (0 for higher-is-better, 1 for lower-is-better) or are None. - Use MetricRegistry direction metadata so lower-is-better metrics are not falsely flagged. - Add unit tests for failure/degradation reporting and error truncation.
- Restrict single-run degraded sample detection to primary quality metrics per mode, avoiding noise from auxiliary counters (e.g. num_temporal_attrs, clustering_coefficient, load_to_db_success). - Extraction: entity_f1 / triple_f1 / property_f1. - Retrieval: recall@* / hit_*@* / mrr / context precision / relevancy / evidence recall. - Ablation/answer: token_f1 / exact_match / rouge_l / answer_correctness / faithfulness / coverage. - Use 0.5 as degradation threshold and respect metric direction. - Update tests to cover extraction/retrieval/unknown mode behavior.
- Clarify that single-run reports flag low-performing samples using a 0.5 threshold on primary quality metrics, not true regression. - Rename section to 'Low-performing Samples' and add an explanatory note pointing users to the Version: ImageMagick 7.1.2-26 Q16-HDRI aarch64 702175ac4:20260621 https://imagemagick.org Copyright: (C) 1999 ImageMagick Studio LLC License: https://imagemagick.org/license/ Features: Cipher DPC HDRI Modules Delegates (built-in): bzlib freetype heic jng jpeg lcms ltdl lzma png tiff webp xml zlib zstd Compiler: clang (21.0.0) Usage: compare [options ...] image reconstruct difference Image Settings: -adjoin join images into a single multi-image file -alpha option on, activate, off, deactivate, set, opaque, copy transparent, extract, background, or shape -authenticate password decipher image with this password -background color background color -colorspace type alternate image colorspace -compose operator set image composite operator -compress type type of pixel compression when writing the image -decipher filename convert cipher pixels to plain pixels -define format:option define one or more image format options -density geometry horizontal and vertical density of the image -depth value image depth -dissimilarity-threshold value maximum distortion for (sub)image match -encipher filename convert plain pixels to cipher pixels -extract geometry extract area from image -format "string" output formatted image characteristics -fuzz distance colors within this distance are considered equal -gravity type horizontal and vertical text placement -highlight-color color emphasize pixel differences with this color -identify identify the format and characteristics of the image -interlace type type of image interlacing scheme -limit type value pixel cache resource limit -lowlight-color color de-emphasize pixel differences with this color -metric type measure differences between images with this metric -monitor monitor progress -negate replace every pixel with its complementary color -passphrase filename get the passphrase from this file -precision value maximum number of significant digits to print -profile filename add, delete, or apply an image profile -quality value JPEG/MIFF/PNG compression level -quiet suppress all warning messages -quantize colorspace reduce colors in this colorspace -read-mask filename associate a read mask with the image -regard-warnings pay attention to warning messages -respect-parentheses settings remain in effect until parenthesis boundary -sampling-factor geometry horizontal and vertical sampling factor -seed value seed a new sequence of pseudo-random numbers -set attribute value set an image attribute -quality value JPEG/MIFF/PNG compression level -repage geometry size and location of an image canvas -similarity-threshold value minimum distortion for (sub)image match -size geometry width and height of image -subimage-search search for subimage -synchronize synchronize image to storage device -taint declare the image as modified -transparent-color color transparent color -type type image type -verbose print detailed information about the image -version print version information -virtual-pixel method virtual pixel access method -write-mask filename associate a write mask with the image Image Operators: -auto-orient automagically orient (rotate) image -brightness-contrast geometry improve brightness / contrast of the image -distort method args distort images according to given method and args -level value adjust the level of image contrast -resize geometry resize the image -rotate degrees apply Paeth rotation to the image -sigmoidal-contrast geometry increase the contrast without saturating highlights or -trim trim image edges -write filename write images to this file Image Channel Operators: -separate separate an image channel into a grayscale image Image Sequence Operators: -crop geometry cut out a rectangular region of the image Image Stack Operators: -delete indexes delete the image from the image sequence Miscellaneous Options: -channel mask set the image channel mask -debug events display copious debugging information -help print program options -list type print a list of supported option arguments -log format format of debugging information By default, the image format of 'file' is determined by its magic number. To specify a particular image format, precede the filename with an image format name and a colon (i.e. ps:image) or specify the image type as the filename suffix (i.e. image.ps). Specify 'file' as '-' for standard input or output. command for baseline-based regression. - Update internal helpers and tests accordingly.
…ic direction indicators - MarkdownReporter no longer reports 'Low-performing Samples' in single-run mode because degradation requires a baseline. - Add ↑/↓ direction symbols to Overall Metrics, Metrics by Question Type, and Regressed Samples tables using MetricRegistry.is_higher_is_better(). - Update tests to cover direction arrows and removal of low-performing logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hugegraph-llm/src/tests/benchmark/test_markdown_reporter.py (1)
1-134: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win该文件未通过
ruff format --check,CI 已在 3.10/3.11/3.12 三个构建中报错。请在合入前本地运行
uv run ruff format .并重新提交,避免阻塞流水线。As per coding guidelines, "For Python code changes, run root
uv run ruff format --check .anduv run ruff check .before handoff."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/tests/benchmark/test_markdown_reporter.py` around lines 1 - 134, The test file is failing Ruff formatting, so reformat the module and verify it passes style checks before handoff. Run formatting on the repository, then recheck the test module around the MarkdownReporter tests to ensure imports, long assertions, and multiline argument blocks match the formatter’s output and the file no longer changes under format check.Sources: Coding guidelines, Pipeline failures
♻️ Duplicate comments (1)
hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/schema_validity.py (1)
130-153: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
required_property_fill的 schema 契约仍不明确，与历史评审意见重复。当顶点标签 schema 未定义
primary_keys（而是用properties定义字段）时，primary_keys为空列表，代码直接记为“完整性满分”，即便实际应有的属性（如name/age）缺失也不会被扣分。此问题此前已被指出，建议明确 required 字段的来源（primary_keys、nullable_keys的反集，还是 benchmark 自定义的required_properties），并在样例 schema 与文档中保持一致。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/schema_validity.py` around lines 130 - 153, `required_property_fill` currently treats labels with empty `primary_keys` as automatically complete, which hides missing required fields when the schema defines properties but not keys. Update the logic in `schema_validity.py` to derive required fields from an explicit contract in `vertex_labels_schema` (for example `primary_keys`, a benchmark-defined `required_properties`, or another clearly documented source) instead of defaulting to full credit, and make the metric check those fields consistently in the `required_property_fill` calculation. Ensure the schema sample and any related docs/tests use the same required-field convention so the behavior is unambiguous.
🟡 Minor comments (22)
hugegraph-llm/src/hugegraph_llm/benchmark/baseline/compare.py-104-146 (1)
104-146: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win指标单侧缺失时用 0.0 兜底可能掩盖回归。
当某个 lower-is-better 指标在 candidate 中缺失(如指标被移除或计算异常导致未产出),
cand_val默认 0.0,diff = -(0-base) = base > 0,会被误判为 improvement,而不是提示"指标缺失"。建议区分"缺失"与"值为 0"两种情况,例如对单侧缺失的指标单独记录警告而非直接参与方向计算。💡 参考修复思路
- for metric in sample_metrics: - base_val = base_sample.metrics.get(metric, 0.0) - cand_val = cand_sample.metrics.get(metric, 0.0) - diff = _semantic_delta(metric, base_val, cand_val) + for metric in sample_metrics: + if metric not in base_sample.metrics or metric not in cand_sample.metrics: + # metric only present on one side: skip direction-based comparison + # and optionally record as a distinct "metric_missing" signal. + continue + base_val = base_sample.metrics[metric] + cand_val = cand_sample.metrics[metric] + diff = _semantic_delta(metric, base_val, cand_val)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/baseline/compare.py` around lines 104 - 146, In `compare_benchmark_results` (the loop building `result.overall_diff` and per-sample `regressions`/`improvements`), missing metrics are being treated as `0.0`, which can turn a removed or failed metric into a fake improvement. Update the logic so `_semantic_delta` is only used when both sides actually contain the metric; when `baseline.overall`, `candidate.overall`, `base_sample.metrics`, or `cand_sample.metrics` lacks a key, record it separately as a missing-metric warning/status instead of comparing against zero. Keep the existing direction-aware diff only for real values, and use the same handling for both overall and sample-level comparison paths.hugegraph-llm/src/hugegraph_llm/benchmark/models/result.py-85-101 (1)
85-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
skipped_metrics在多次compute_overall()调用间会重复累积。
self.overall每次调用都会重置（Line 87），但self.metadata["skipped_metrics"]只会extend，从不清空。cli.py在--smoke/--samples场景下会对同一个BenchmarkResult多次调用compute_overall()（先由 runner 计算一次，再在样本过滤后重算），导致同一 metric 名称在skipped_metrics中重复出现，甚至可能残留过滤前才成立、过滤后已不适用的过期条目，误导报告读者。🐛 建议修复
def compute_overall(self) -> None: """Compute overall metrics by averaging per-sample metrics.""" self.overall = {} + self.metadata["skipped_metrics"] = [] if not self.samples: return all_keys: set = set() for s in self.samples: all_keys.update(s.metrics.keys()) skipped: List[str] = [] for key in all_keys: values = [s.metrics[key] for s in self.samples if key in s.metrics and s.metrics[key] is not None] if values: self.overall[key] = round(sum(values) / len(values), 4) else: skipped.append(key) if skipped: - self.metadata.setdefault("skipped_metrics", []).extend(skipped) + self.metadata["skipped_metrics"] = skipped🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/models/result.py` around lines 85 - 101, `compute_overall` in `BenchmarkResult` currently resets `overall` but keeps appending to `metadata["skipped_metrics"]`, so repeated calls accumulate duplicates and stale entries. Update `compute_overall` to clear or recompute `skipped_metrics` on each invocation before extending it, and ensure the logic in `BenchmarkResult.compute_overall` only reflects the current `samples` state when `cli.py` triggers multiple recomputations.hugegraph-llm/scripts/benchmark/run_external_benchmarks.sh-43-65 (1)
43-65: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win单个数据集失败会导致后续所有数据集被跳过。
set -euo pipefail下，run_retrieval/run_extraction内部调用"${BENCHMARK[@]}"一旦非零退出，脚本立即终止，line 71-76、81 中尚未执行的数据集将不会被跑到，这与"跑遍所有已准备好的外部数据集"的烟雾测试目的相悖。💡 建议：捕获单次运行的失败并继续执行剩余数据集
+FAILED=0 + run_retrieval() { local name="$1" local lang="$2" local file="$DATA_DIR/${name}_retrieval.json" if [[ ! -f "$file" ]]; then echo "SKIP: $file not found" return fi echo "==> Running retrieval benchmark: $name" - "${BENCHMARK[@]}" --mode retrieval --data "$file" --language "$lang" --offline + if ! "${BENCHMARK[@]}" --mode retrieval --data "$file" --language "$lang" --offline; then + echo "FAILED: $name" + FAILED=1 + fi echo "" }同样处理
run_extraction，并在脚本末尾exit $FAILED。Also applies to: 70-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/scripts/benchmark/run_external_benchmarks.sh` around lines 43 - 65, The benchmark helpers run under set -euo pipefail, so a non-zero exit from BENCHMARK inside run_retrieval or run_extraction aborts the whole script and skips remaining datasets. Update run_retrieval and run_extraction to catch each "${BENCHMARK[@]}" failure, record it in a shared FAILED flag, and continue looping through the rest of the datasets; then have the script exit with that accumulated status at the end. Use the run_retrieval and run_extraction functions and the final summary/exit path as the main places to adjust.hugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/coverage.py-103-170 (1)
103-170: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win补充 Coverage 单测
hugegraph-llm/src/tests/benchmark/test_answer_metrics.py和test_llm_judge_metrics.py里还没有Coverage的用例。建议补上正常路径，以及llm缺失、空 reference、fact extraction 失败等分支的测试，避免这个新指标后续回归。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/coverage.py` around lines 103 - 170, Coverage metric tests are missing, so add coverage cases to the existing answer-metrics and judge-metrics test suites by targeting the Coverage.calculate method and its helpers _extract_facts and _check_coverage. Cover the normal happy path, missing llm returning None fields, empty reference yielding perfect coverage, and fact extraction failure returning None, and assert the returned keys coverage, coverage_ref_facts, and coverage_covered match the expected branch behavior.Source: Path instructions
hugegraph-llm/src/hugegraph_llm/models/llms/openai.py-47-47 (1)
47-47: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
OPENAI_TIMEOUT格式错误会导致启动崩溃。若环境变量
OPENAI_TIMEOUT被设置为非数字字符串，float(...)会抛出ValueError，在客户端构造阶段直接崩溃而非优雅降级。🛡️ 建议修复
- timeout = float(os.getenv("OPENAI_TIMEOUT", "0")) or None + try: + timeout = float(os.getenv("OPENAI_TIMEOUT", "0")) or None + except ValueError: + log.warning("Invalid OPENAI_TIMEOUT value; falling back to no explicit timeout") + timeout = None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/models/llms/openai.py` at line 47, The OPENAI_TIMEOUT parsing in the OpenAI client setup can crash startup when the env var is not numeric. Update the timeout handling in the openai.py initialization path to safely parse OPENAI_TIMEOUT with error handling, and fall back to a default/None value instead of letting ValueError escape. Use the existing timeout assignment near the OpenAI client construction as the fix point.hugegraph-llm/src/hugegraph_llm/operators/llm_op/schema_build.py-76-115 (1)
76-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win补充
SchemaBuilder._extract_schema的单测：当前没有针对hugegraph_llm/operators/llm_op/schema_build.py的直接测试，建议补上未闭合围栏、前缀/尾部噪声和数组根节点的用例，避免这段新增解析逻辑回归。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/operators/llm_op/schema_build.py` around lines 76 - 115, Add direct unit tests for SchemaBuilder._extract_schema to cover the new parsing paths: a truncated or unclosed fenced JSON block, responses with explanatory text before and/or after the JSON, and a valid array-root JSON payload. Use the _extract_schema static method from SchemaBuilder in schema_build.py so the tests stay focused on this logic and guard against regressions in the added normalization and trimming behavior.Source: Path instructions
hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/conflict_detection.py-53-58 (1)
53-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win实体身份应包含 label
_detect_property_conflicts现在只按归一化后的name分组，label没有参与；而上游顶点数据本身保留了label。同名异类实体（如Person:Alice和Company:Alice）会被合并，进而产生虚假的属性冲突。建议把实体键改成(label, name)或等价的复合键。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/conflict_detection.py` around lines 53 - 58, The conflict grouping currently uses only the normalized vertex name, so different entity types with the same name can be merged incorrectly. Update the identity key used by _detect_property_conflicts to include the vertex label together with the name, and adjust any helper such as _get_vertex_name or a new key builder so vertices are grouped by a composite (label, name) identity instead of name alone.hugegraph-llm/src/hugegraph_llm/benchmark/runners/ablation_runner.py-32-37 (1)
32-37: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
_validate_sample_contract未校验sample_id，导致缺失时抛出裸 KeyError 而非清晰的 ValueError。
required_fields（第34行）未包含sample_id，但第101行sample["sample_id"]直接索引访问。若样本缺失sample_id，将绕过契约校验、在process_sample内抛出KeyError，而不是本函数设计的、更具诊断价值的ValueError提示，弱化了 fail-fast 契约校验的一致性。🐛 建议修复
def _validate_sample_contract(sample: Dict[str, Any]) -> None: sample_id = sample.get("sample_id", "unknown") - required_fields = ["gold_answer", *[f"{mode}_answer" for mode in _ANSWER_MODES]] + required_fields = ["sample_id", "gold_answer", *[f"{mode}_answer" for mode in _ANSWER_MODES]] missing = [field for field in required_fields if field not in sample] if missing: raise ValueError(f"Ablation sample {sample_id!r} missing required field(s): {', '.join(missing)}")Also applies to: 100-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/runners/ablation_runner.py` around lines 32 - 37, _validate_sample_contract currently skips checking sample_id, so process_sample can fail later with a bare KeyError instead of a clear ValueError. Add sample_id to the required_fields validation in _validate_sample_contract and keep the existing missing-field error path so any sample without sample_id is rejected up front with the same contract-checking behavior as gold_answer and the answer fields.hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/triple_f1.py-46-47 (1)
46-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win"双空"约定与其他指标不一致。
当
prediction和reference均为空列表时,此处返回triple_precision/recall/f1 = 0.0。而同一 PR 中的token_f1.py(_compute_token_f1_single,双空返回 1.0)和rouge_l.py(第 112-113 行,双空返回 1.0)都将"预测与参考皆为空"视为平凡匹配(1.0)。若某条样本本身没有可抽取的三元组(gold 为空)且模型也正确地未产生三元组,当前实现会把这种"正确的空结果"记为 0 分,拉低总体指标,与项目其他指标的语义不一致。
🐛 建议修复
if not prediction and not reference: - return {"triple_precision": 0.0, "triple_recall": 0.0, "triple_f1": 0.0} + return {"triple_precision": 1.0, "triple_recall": 1.0, "triple_f1": 1.0}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/triple_f1.py` around lines 46 - 47, In triple_f1.py, the empty-empty case in the triple F1 computation is inconsistent with the project’s other metrics. Update the empty-input branch in the triple F1 helper so that when both prediction and reference are empty it returns 1.0 for triple_precision, triple_recall, and triple_f1, matching the behavior used in token_f1.py and rouge_l.py. Locate the logic in the triple F1 computation function handling the “prediction and reference both empty” check and adjust that special case accordingly.hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/structural_integrity.py-118-129 (1)
118-129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win端点字段完全缺失的边未被计入
orphan_edge_rate。
out_v and out_v not in vertex_names在out_v为空字符串（即边完全没有outV/source字段）时短路为False，导致这类边不会被判定为孤立边，只要另一端点命中已有顶点即可“蒙混过关”。这会低估该质量指标，掩盖抽取结果中端点信息缺失的问题。🐛 建议修复
if edges: orphan_count = 0 for e in edges: out_v = normalize_answer(str(_edge_out(e)), language) in_v = normalize_answer(str(_edge_in(e)), language) - if out_v and out_v not in vertex_names: - orphan_count += 1 - elif in_v and in_v not in vertex_names: - orphan_count += 1 + if (not out_v or out_v not in vertex_names) or ( + not in_v or in_v not in vertex_names + ): + orphan_count += 1 orphan_edge_rate = orphan_count / len(edges)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/structural_integrity.py` around lines 118 - 129, The orphan edge calculation in the structural integrity metric is missing edges whose endpoint field is entirely absent, so these should still count toward orphan_edge_rate. Update the logic in the orphan counting loop in the metric function that computes orphan_edge_rate to treat a missing or empty outV/source or inV/target as an orphan condition, rather than relying on the current truthy check that short-circuits on empty strings. Make the check explicit for the edge endpoint accessors such as _edge_out and _edge_in so edges with one or both endpoints missing are correctly counted.hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.py-1-1 (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRuff format 检查未通过。
CI 显示
ruff format --check在多个 Python 版本下均报告该文件需要重新格式化，请在提交前本地运行uv run ruff format .与uv run ruff check .。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.py` at line 1, The file is failing Ruff formatting checks, so reformat the Python script to match the repository style before committing. Use the script entry point in generate_text2kgbench_candidates.py and run the project’s formatting and lint commands locally, then ensure the file passes ruff format and ruff check without relying on version-specific behavior.Source: Pipeline failures
hugegraph-llm/scripts/benchmark/fix_car33_edge_ids.py-35-36 (1)
35-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
edge.get("outV", "")无法处理显式null值。若原始 JSON 中
outV/inV字段存在但值为null，dict.get(key, default)会返回None而非默认值，随后_strip_id_prefix(None)会把str(None)即字符串"None"写入结果，产生错误数据而非空字符串。🐛 建议修复
- fixed_edge["outV"] = _strip_id_prefix(edge.get("outV", "")) - fixed_edge["inV"] = _strip_id_prefix(edge.get("inV", "")) + fixed_edge["outV"] = _strip_id_prefix(edge.get("outV") or "") + fixed_edge["inV"] = _strip_id_prefix(edge.get("inV") or "")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/scripts/benchmark/fix_car33_edge_ids.py` around lines 35 - 36, The edge ID normalization in fix_car33_edge_ids.py should handle explicit null values for outV and inV instead of passing None into _strip_id_prefix. Update the logic around the fixed_edge assignments to coalesce None to an empty string before calling _strip_id_prefix, so fix_car33_edge_ids does not serialize "None" into the output. Keep the change localized to the edge field handling for outV and inV.hugegraph-llm/src/hugegraph_llm/benchmark/runners/retrieval_runner.py-76-91 (1)
76-91: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
sample_id处理在校验与执行阶段不一致。
_validate_sample_contract(Line 77)用sample.get("sample_id", "unknown")容忍sample_id缺失,但process_sample(Line 167)用sample["sample_id"]强制下标访问。若样本缺少sample_id,校验阶段不会报错,执行阶段却会抛KeyError,虽然会被_run_samples_concurrent的线程异常兜底捕获,但错误信息会变成不清晰的"__sample__"类型,而不是校验阶段本该给出的明确提示。💚 建议修复
- sample_id = sample["sample_id"] + sample_id = sample.get("sample_id", "unknown")Also applies to: 166-171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/runners/retrieval_runner.py` around lines 76 - 91, _validate_sample_contract and process_sample should use the same sample_id handling so missing IDs are caught during validation instead of failing later as a generic thread error. Update process_sample to avoid direct sample["sample_id"] access and reuse the same safe lookup/validation pattern used in _validate_sample_contract, so any missing sample_id produces a clear, explicit error message before _run_samples_concurrent executes the sample.hugegraph-llm/scripts/benchmark/run_small_datasets_experiment.sh-119-136 (1)
119-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win时间戳提取丢失日期部分。
exp_dir.name形如small_datasets_20260705_120000,split('_')[-1]只取到120000,报告中Timestamp字段会丢失日期,只剩时间。💚 建议修复
-lines.append(f"- **Timestamp**: {exp_dir.name.split('_')[-1]}") +lines.append(f"- **Timestamp**: {'_'.join(exp_dir.name.split('_')[-2:])}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/scripts/benchmark/run_small_datasets_experiment.sh` around lines 119 - 136, The Timestamp extraction in the benchmark report generation is dropping the date because it only uses the last underscore segment from exp_dir.name. Update the report-building logic near load_baseline/ fmt_metrics so the Timestamp line uses the full timestamp portion from the experiment directory name (e.g. the date and time embedded in small_datasets_*), instead of only the HHMMSS part. Keep the change localized to the report header construction where commit and Timestamp are appended.hugegraph-llm/src/hugegraph_llm/benchmark/baseline/store.py-33-45 (1)
33-45: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win为
subprocess.run添加超时保护
_get_git_commit调用外部git进程但未设置timeout，异常环境下（如 git 挂起等待输入）可能导致该调用无限期阻塞，进而拖慢save()。Static analysis 报告的命令注入/路径遍历提示在此场景下（本地离线 CLI 工具，参数硬编码、路径为调用方本地传入）为误报，未采纳。
🛡️ 修复建议
result = subprocess.run( ["git", "rev-parse", "HEAD"], capture_output=True, text=True, + timeout=5, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/baseline/store.py` around lines 33 - 45, The `_get_git_commit` helper currently runs `subprocess.run` without any timeout, so a hung `git rev-parse HEAD` can block `save()` indefinitely. Update `_get_git_commit` to pass a reasonable `timeout` to `subprocess.run`, and handle the timeout case by falling back to "unknown" just like the existing exception path. Keep the fix localized to `_get_git_commit` in `store.py`, and preserve the current return behavior for successful and failed git lookups.hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/graph_structure.py-52-70 (1)
52-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win残留的同名顶点 ID 冲突风险
之前审查中提到的"边端点未规范化"问题已通过
name_to_node_id+canonical_endpoint修复。但name_to_node_id仅以name_str为 key（Line 63），未包含 label。若存在不同 label 但同名的顶点（例如人名 "Washington" 与地名 "Washington"），后处理的顶点会覆盖映射，导致边端点被错误规范化到错误节点，重新引入拓扑指标（num_nodes/density/num_components等）计算偏差。建议在冲突时保留
(label, name)的精确匹配，或在遇到重名冲突时记录警告/跳过而非静默覆盖。🛠️ 修复建议
name_to_node_id: Dict[str, str] = {} + name_collision: Dict[str, bool] = {} # Add nodes for v in vertices: name = v.get("name") if not name and isinstance(v.get("properties"), dict): name = v["properties"].get("name", "") if name: label = str(v.get("label", "")) name_str = str(name) node_id = f"{label}:{name_str}" if label else name_str - name_to_node_id[name_str] = node_id + if name_str in name_to_node_id and name_to_node_id[name_str] != node_id: + name_collision[name_str] = True + name_to_node_id[name_str] = node_id g.add_node(node_id, label=label, name=name_str)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/graph_structure.py` around lines 52 - 70, The current name_to_node_id mapping in graph_structure.py can silently overwrite entries when different labels share the same name, causing canonical_endpoint to resolve edges to the wrong node. Update the add-nodes / canonical_endpoint flow so node lookup prefers an exact (label, name) match or otherwise handles duplicate names safely, and avoid silent overwrites in name_to_node_id. Use the existing canonical_endpoint helper and g.add_node metadata to keep endpoint normalization consistent without breaking topology metrics.hugegraph-llm/src/hugegraph_llm/models/embeddings/openai.py-119-141 (1)
119-141: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win重试循环在最后一次失败后仍会 sleep，增加不必要的失败延迟
_embed_with_retry/_async_embed_with_retry在最后一次尝试失败后仍执行time.sleep(wait)（最长可达 60s）才抛出RuntimeError，调用方本可以更快得到失败反馈。此外两个方法逻辑几乎完全重复，可考虑抽取公共部分。⚡ 建议的修改
except (RateLimitError, APIConnectionError, APITimeoutError) as exc: last_exc = exc - wait = min(2 ** attempt, 60) - log.warning("Embedding request failed (attempt %d/%d): %s; retrying in %ds", attempt + 1, max_retries, exc, wait) - time.sleep(wait) + if attempt < max_retries - 1: + wait = min(2 ** attempt, 60) + log.warning("Embedding request failed (attempt %d/%d): %s; retrying in %ds", attempt + 1, max_retries, exc, wait) + time.sleep(wait) raise RuntimeError(f"Embedding failed after {max_retries} retries: {last_exc}")(async 版本同理，改用
await asyncio.sleep(wait))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/models/embeddings/openai.py` around lines 119 - 141, The retry helpers `_embed_with_retry` and `_async_embed_with_retry` still sleep after the final failed attempt, which adds avoidable latency before surfacing the error. Update the retry loop in both methods so the backoff sleep only happens when another retry remains, and on the last attempt raise the `RuntimeError` immediately after logging the failure. Keep the retry behavior aligned between the sync and async paths (`time.sleep` vs `asyncio.sleep`) and consider sharing the common retry flow if you touch both methods.hugegraph-llm/scripts/benchmark/prepare_benchmark_subsets.py-66-87 (1)
66-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win分层抽样会静默丢弃小类别，与常规采样路径行为不一致
_stratified_sample对每个question_type桶执行k = max(1, int(len(bucket) * fraction)) if len(bucket) * fraction >= 1 else 0，当某个类型的样本数较少（如 MuSiQue 5% 场景下 <20 条）时，k直接为 0，该类型会被完全排除在子集之外；而非分层路径（第 74-75 行）保证k至少为 1。这会导致分层抽样在小类别上出现覆盖缺口，与"按 question_type 分层"的设计初衷不符。🐛 建议的修改
for bucket in buckets.values(): - k = max(1, int(len(bucket) * fraction)) if len(bucket) * fraction >= 1 else 0 - if k > 0: - selected.extend(random.sample(bucket, k)) + k = max(1, int(len(bucket) * fraction)) + selected.extend(random.sample(bucket, min(k, len(bucket))))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/scripts/benchmark/prepare_benchmark_subsets.py` around lines 66 - 87, The `_stratified_sample` logic in `prepare_benchmark_subsets.py` is dropping small `question_type` buckets because it allows `k` to become 0 for low-count groups. Update the bucket sampling in `_stratified_sample` so each non-empty `question_type` bucket still contributes at least one item, matching the behavior of the non-stratified path and preserving coverage for rare categories. Use the existing `buckets` loop and `random.sample` flow, and adjust only the per-bucket `k` calculation/shuffle path.hugegraph-llm/BENCHMARK_DATASETS.md-650-657 (1)
650-657: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExtraction 结果表格全部是占位符 "—"，建议补全或说明未完成原因。
第 8 节其余表格（Retrieval + Answer）都已填入真实跑测数值，但 Extraction 表格所有指标列均为空占位符，容易让读者误认为抽取评测已完成并得到有效结果。建议补充实际数值，或在表格上方明确注明"抽取评测尚未完成/数值待补充"，避免误导。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/BENCHMARK_DATASETS.md` around lines 650 - 657, The Extraction benchmark table in BENCHMARK_DATASETS.md is still filled with placeholder dashes, unlike the other benchmark sections. Update the Extraction section by either replacing the placeholders with the actual values in the relevant table rows or adding a clear note near the table that the extraction evaluation is not yet completed and the numbers are pending; use the existing “Extraction（离线 + LLM-Judge）” table and its metric columns as the place to fix.hugegraph-llm/src/tests/benchmark/test_integration_extraction.py-189-198 (1)
189-198: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win该测试未真正验证"错误被追踪"的行为。
bad_data缺少gold_vertices/candidate_vertices等字段，但这些字段在 runner 中均有默认值兜底（sample.get(key, [])），实际不会触发错误路径；测试也只断言error_count键存在，未断言其值> 0。这使得测试名与实际验证内容不符，无法在未来错误追踪逻辑被破坏时捕获回归。建议构造一个真正会导致 metric 抛异常的样本（如缺少sample_id或字段类型错误），并断言error_count > 0。As per path instructions, "Any code change in
hugegraph-llmmust add or update tests that exercise the changed behavior, regression risk, or failure path."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/tests/benchmark/test_integration_extraction.py` around lines 189 - 198, The test in test_extractionrunnererrortracking_error_tracking_with_bad_sample is not actually exercising the error-tracking path because the missing vertex fields are defaulted by ExtractionRunner. Update the fixture in this test to use a sample that truly makes the metric evaluation fail (for example, an invalid sample shape or missing required identifier that causes the benchmark flow to raise), and then assert not just that BenchmarkResult.metadata contains error_count but that error_count is greater than zero. Keep the focus on the run() path and the error aggregation behavior so the test name matches the verified outcome.Source: Path instructions
hugegraph-llm/src/hugegraph_llm/benchmark/datasets/download.py-79-82 (1)
79-82: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
force未传递到_derive_hotpotqa_corpus，导致强制刷新后 corpus 仍是旧数据。
download_dataset对原始文件的下载遵循force，但衍生的hotpotqa_corpus.json只要文件存在就直接跳过重新生成（Line 154-156），未考虑上游文件是否刚被强制刷新。对于强调可复现性的 benchmark 工具，这会导致--force之后语料与最新原始数据不一致。🛠️ 建议修复
if spec.postprocess == "hotpotqa_corpus": _derive_hotpotqa_corpus( - data_root / "hotpotqa" / "hotpotqa.json", data_root / "hotpotqa" / "hotpotqa_corpus.json" + data_root / "hotpotqa" / "hotpotqa.json", data_root / "hotpotqa" / "hotpotqa_corpus.json", force=force )-def _derive_hotpotqa_corpus(qa_file: Path, corpus_file: Path) -> None: - if corpus_file.exists(): +def _derive_hotpotqa_corpus(qa_file: Path, corpus_file: Path, force: bool = False) -> None: + if corpus_file.exists() and not force: logger.info("Derived corpus already exists: %s", corpus_file) returnAlso applies to: 153-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/datasets/download.py` around lines 79 - 82, The hotpotqa corpus regeneration path in download_dataset/_derive_hotpotqa_corpus ignores force, so a forced refresh can leave hotpotqa_corpus.json stale. Pass the force flag through the hotpotqa_corpus branch and update _derive_hotpotqa_corpus to regenerate when force is true even if the output file already exists, ensuring the derived corpus stays in sync with freshly downloaded hotpotqa.json.hugegraph-llm/src/hugegraph_llm/benchmark/datasets/registry.py-66-70 (1)
66-70: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win将 HotpotQA 下载地址改为 HTTPS
http://curtis.ml.cmu.edu/datasets/hotpot/hotpot_dev_distractor_v1.json有可用的https://版本，建议直接切换，避免明文传输导致基准数据被篡改、影响可复现性。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/datasets/registry.py` around lines 66 - 70, HotpotQA 的下载链接仍使用明文 HTTP，需要改为 HTTPS；请在 registry.py 中定位 HotpotQA 对应的 DownloadFile 配置，将该 URL 切换为可用的 https:// 版本，保持 path 和其它基准数据配置不变，以确保下载链路安全且可复现。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3503cc58-dcdf-464e-bab0-7e3ac8d2d9d6
📒 Files selected for processing (120)
.gitignoredocs/quality/benchmark-code-style-spec.mdhugegraph-llm/BENCHMARK_DATASETS.mdhugegraph-llm/GRAPHRAG_BENCHMARK.mdhugegraph-llm/docs/benchmark/experiment-record.mdhugegraph-llm/docs/benchmark/experiment-report.mdhugegraph-llm/pyproject.tomlhugegraph-llm/scripts/benchmark/README.mdhugegraph-llm/scripts/benchmark/fix_car33_edge_ids.pyhugegraph-llm/scripts/benchmark/generate_hugegraph_retrieval_outputs.pyhugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.pyhugegraph-llm/scripts/benchmark/prepare_benchmark_subsets.pyhugegraph-llm/scripts/benchmark/prepare_car33_benchmark.pyhugegraph-llm/scripts/benchmark/run_benchmarks.pyhugegraph-llm/scripts/benchmark/run_car33_pipeline_extraction.pyhugegraph-llm/scripts/benchmark/run_external_benchmarks.shhugegraph-llm/scripts/benchmark/run_hotpotqa_llm_demo.pyhugegraph-llm/scripts/benchmark/run_hotpotqa_vector_demo.pyhugegraph-llm/scripts/benchmark/run_small_datasets_experiment.shhugegraph-llm/scripts/benchmark/summarize_baselines.pyhugegraph-llm/src/hugegraph_llm/benchmark/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/__main__.pyhugegraph-llm/src/hugegraph_llm/benchmark/baseline/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/baseline/compare.pyhugegraph-llm/src/hugegraph_llm/benchmark/baseline/store.pyhugegraph-llm/src/hugegraph_llm/benchmark/cli.pyhugegraph-llm/src/hugegraph_llm/benchmark/data/samples/ablation_sample.jsonhugegraph-llm/src/hugegraph_llm/benchmark/data/samples/car_extraction_sample.jsonhugegraph-llm/src/hugegraph_llm/benchmark/data/samples/chinese_retrieval_sample.jsonhugegraph-llm/src/hugegraph_llm/benchmark/data/samples/extraction_sample.jsonhugegraph-llm/src/hugegraph_llm/benchmark/data/samples/retrieval_context_sample.jsonhugegraph-llm/src/hugegraph_llm/benchmark/data/samples/retrieval_docid_sample.jsonhugegraph-llm/src/hugegraph_llm/benchmark/datasets/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/datasets/download.pyhugegraph-llm/src/hugegraph_llm/benchmark/datasets/prepare_external_datasets.pyhugegraph-llm/src/hugegraph_llm/benchmark/datasets/registry.pyhugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/base.pyhugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/judge_utils.pyhugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/llm_judge.pyhugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/mock_judge.pyhugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/prompts.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/answer_correctness.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/coverage.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/exact_match.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/faithfulness.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/rouge_l.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/answer/token_f1.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/base.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/conflict_detection.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/entity_f1.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/graph_structure.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/property_f1.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/schema_validity.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/structural_integrity.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/syntax_validity.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/temporal_validity.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/triple_f1.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/registry.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/retrieval/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/retrieval/context_precision.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/retrieval/context_relevancy.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/retrieval/evidence_recall.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/retrieval/hit_at_k.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/retrieval/mrr.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/retrieval/recall_at_k.pyhugegraph-llm/src/hugegraph_llm/benchmark/models/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/models/result.pyhugegraph-llm/src/hugegraph_llm/benchmark/reporters/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/reporters/json_reporter.pyhugegraph-llm/src/hugegraph_llm/benchmark/reporters/markdown_reporter.pyhugegraph-llm/src/hugegraph_llm/benchmark/runners/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/runners/ablation_runner.pyhugegraph-llm/src/hugegraph_llm/benchmark/runners/answer_runner.pyhugegraph-llm/src/hugegraph_llm/benchmark/runners/base_runner.pyhugegraph-llm/src/hugegraph_llm/benchmark/runners/extraction_runner.pyhugegraph-llm/src/hugegraph_llm/benchmark/runners/retrieval_runner.pyhugegraph-llm/src/hugegraph_llm/benchmark/utils/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/utils/graph_extract.pyhugegraph-llm/src/hugegraph_llm/benchmark/utils/normalize.pyhugegraph-llm/src/hugegraph_llm/benchmark/utils/retrieval_adapter.pyhugegraph-llm/src/hugegraph_llm/config/llm_config.pyhugegraph-llm/src/hugegraph_llm/flows/graph_extract.pyhugegraph-llm/src/hugegraph_llm/models/embeddings/openai.pyhugegraph-llm/src/hugegraph_llm/models/llms/openai.pyhugegraph-llm/src/hugegraph_llm/models/rerankers/init_reranker.pyhugegraph-llm/src/hugegraph_llm/models/rerankers/jina.pyhugegraph-llm/src/hugegraph_llm/operators/llm_op/info_extract.pyhugegraph-llm/src/hugegraph_llm/operators/llm_op/property_graph_extract.pyhugegraph-llm/src/hugegraph_llm/operators/llm_op/schema_build.pyhugegraph-llm/src/hugegraph_llm/state/ai_state.pyhugegraph-llm/src/hugegraph_llm/utils/embedding_utils.pyhugegraph-llm/src/tests/benchmark/__init__.pyhugegraph-llm/src/tests/benchmark/test_answer_metrics.pyhugegraph-llm/src/tests/benchmark/test_base_runner.pyhugegraph-llm/src/tests/benchmark/test_baseline.pyhugegraph-llm/src/tests/benchmark/test_cli.pyhugegraph-llm/src/tests/benchmark/test_conflict_detection.pyhugegraph-llm/src/tests/benchmark/test_e2e_car_dataset.pyhugegraph-llm/src/tests/benchmark/test_e2e_cli.pyhugegraph-llm/src/tests/benchmark/test_extraction_metrics.pyhugegraph-llm/src/tests/benchmark/test_graph_extract.pyhugegraph-llm/src/tests/benchmark/test_graph_structure.pyhugegraph-llm/src/tests/benchmark/test_integration_ablation.pyhugegraph-llm/src/tests/benchmark/test_integration_extraction.pyhugegraph-llm/src/tests/benchmark/test_integration_retrieval.pyhugegraph-llm/src/tests/benchmark/test_json_parse_utils.pyhugegraph-llm/src/tests/benchmark/test_llm_judge_metrics.pyhugegraph-llm/src/tests/benchmark/test_markdown_reporter.pyhugegraph-llm/src/tests/benchmark/test_prepare_external_datasets.pyhugegraph-llm/src/tests/benchmark/test_registry_fix.pyhugegraph-llm/src/tests/benchmark/test_reproducibility.pyhugegraph-llm/src/tests/benchmark/test_retrieval_adapter.pyhugegraph-llm/src/tests/benchmark/test_retrieval_metrics.pyhugegraph-llm/src/tests/benchmark/test_retrieval_runner.pyhugegraph-llm/src/tests/benchmark/test_temporal_validity.pyhugegraph-llm/src/tests/document/test_graph_extract_configurable_split.py
Comment on lines
+1
to
+13
| #!/usr/bin/env python3 | ||
| """Fix edge endpoint IDs in car33 pipeline candidate JSONs. | ||
|
|
||
| HugeGraph-AI GRAPH_EXTRACT outputs edges with ``outV``/``inV`` values like | ||
| ``"1:自动远光灯开启指示灯"``, while vertices use the clean ``name`` field | ||
| (``"自动远光灯开启指示灯"``). This mismatch causes the benchmark to treat | ||
| all edges as orphan edges. | ||
|
|
||
| This script reads an existing candidate JSON (which already contains the | ||
| raw LLM outputs) and rewrites the edge endpoints by stripping the ``<id>:`` | ||
| prefix. The fixed JSON can then be fed back into ``hugegraph-benchmark run`` | ||
| without re-running the expensive LLM extraction. | ||
| """ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
缺少 ASF License Header，CI 已报错。
Pipeline 报告 license-eye header check failed，该文件目前没有标准的 Apache License Header。
📄 建议在文件顶部添加
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
#!/usr/bin/env python3
"""Fix edge endpoint IDs in car33 pipeline candidate JSONs.🧰 Tools
🪛 GitHub Actions: License header & 3rd-party check / 1_check-license-header.txt
[error] 1-1: license-eye header check failed: file does not have a valid license header.
🪛 GitHub Actions: License header & 3rd-party check / check-license-header
[error] 1-1: license-eye: file does not have a valid license header
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/scripts/benchmark/fix_car33_edge_ids.py` around lines 1 - 13,
The script file is missing the required Apache License header, causing the CI
license-eye header check to fail. Add the standard ASF license block at the very
top of the file before the module docstring, keeping the existing script content
unchanged; use this file’s top-level entry point and docstring as the location
to insert the header.
Source: Pipeline failures
Comment on lines
+49
to
+52
| REPO_ROOT = Path(__file__).resolve().parents[3] | ||
| SRC_ROOT = REPO_ROOT / "src" | ||
| if str(SRC_ROOT) not in sys.path: | ||
| sys.path.insert(0, str(SRC_ROOT)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files 'hugegraph-llm/scripts/benchmark/*' && printf '\n---\n' && sed -n '1,120p' hugegraph-llm/scripts/benchmark/generate_hugegraph_retrieval_outputs.py && printf '\n---\n' && sed -n '1,120p' hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.pyRepository: hugegraph/hugegraph-ai
Length of output: 10169
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('hugegraph-llm/scripts/benchmark/generate_hugegraph_retrieval_outputs.py').resolve()
print("resolved file:", p)
for i, parent in enumerate(p.parents[:6]):
print(i, parent)
PYRepository: hugegraph/hugegraph-ai
Length of output: 443
REPO_ROOT 层级少一层
Path(__file__).resolve().parents[3] 会越过 hugegraph-llm，把 SRC_ROOT 指到错误的 src；直接运行脚本且未预装依赖时会导入失败。这里应改成 parents[2]。
🐛 建议修复
-REPO_ROOT = Path(__file__).resolve().parents[3]
+REPO_ROOT = Path(__file__).resolve().parents[2]
SRC_ROOT = REPO_ROOT / "src"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| REPO_ROOT = Path(__file__).resolve().parents[3] | |
| SRC_ROOT = REPO_ROOT / "src" | |
| if str(SRC_ROOT) not in sys.path: | |
| sys.path.insert(0, str(SRC_ROOT)) | |
| REPO_ROOT = Path(__file__).resolve().parents[2] | |
| SRC_ROOT = REPO_ROOT / "src" | |
| if str(SRC_ROOT) not in sys.path: | |
| sys.path.insert(0, str(SRC_ROOT)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/scripts/benchmark/generate_hugegraph_retrieval_outputs.py`
around lines 49 - 52, REPO_ROOT is computed one level too high in
generate_hugegraph_retrieval_outputs.py, so SRC_ROOT can point at the wrong src
directory and break imports when the script runs directly. Update the REPO_ROOT
assignment in the script bootstrap section to use the correct
Path(__file__).resolve().parents value, and keep the existing sys.path insertion
logic tied to SRC_ROOT so the script resolves local dependencies properly.
Comment on lines
+539
to
+552
| pipeline = manager.fetch() | ||
| if pipeline is None: | ||
| pipeline = flow.build_flow(query=query, rerank_method="bleu", topk_return_results=topk) | ||
| status = pipeline.init() | ||
| if status.isErr(): | ||
| raise RuntimeError(f"rag_graph_vector init failed: {status.getInfo()}") | ||
| status = pipeline.run() | ||
| if status.isErr(): | ||
| manager.add(pipeline) | ||
| raise RuntimeError(f"rag_graph_vector run failed: {status.getInfo()}") | ||
| state = pipeline.getGParamWithNoEmpty("wkflow_state").to_json() | ||
| result = flow.post_deal(pipeline) | ||
| manager.add(pipeline) | ||
| return {"state": state, "result": result} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
pipeline.init() 失败分支未将 pipeline 归还 manager，与其余分支处理不一致。
第 546-548 行 run() 失败时会先 manager.add(pipeline) 再抛出异常，但第 542-544 行 init() 失败时直接 raise，未调用 manager.add(pipeline)。若 init() 已经分配了底层资源（连接、GParam 注册等），该 pipeline 实例会被直接丢弃且不再进入池中管理，造成资源未被追踪/复用不一致。
🔧 建议修复
status = pipeline.init()
if status.isErr():
+ manager.add(pipeline)
raise RuntimeError(f"rag_graph_vector init failed: {status.getInfo()}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pipeline = manager.fetch() | |
| if pipeline is None: | |
| pipeline = flow.build_flow(query=query, rerank_method="bleu", topk_return_results=topk) | |
| status = pipeline.init() | |
| if status.isErr(): | |
| raise RuntimeError(f"rag_graph_vector init failed: {status.getInfo()}") | |
| status = pipeline.run() | |
| if status.isErr(): | |
| manager.add(pipeline) | |
| raise RuntimeError(f"rag_graph_vector run failed: {status.getInfo()}") | |
| state = pipeline.getGParamWithNoEmpty("wkflow_state").to_json() | |
| result = flow.post_deal(pipeline) | |
| manager.add(pipeline) | |
| return {"state": state, "result": result} | |
| pipeline = manager.fetch() | |
| if pipeline is None: | |
| pipeline = flow.build_flow(query=query, rerank_method="bleu", topk_return_results=topk) | |
| status = pipeline.init() | |
| if status.isErr(): | |
| manager.add(pipeline) | |
| raise RuntimeError(f"rag_graph_vector init failed: {status.getInfo()}") | |
| status = pipeline.run() | |
| if status.isErr(): | |
| manager.add(pipeline) | |
| raise RuntimeError(f"rag_graph_vector run failed: {status.getInfo()}") | |
| state = pipeline.getGParamWithNoEmpty("wkflow_state").to_json() | |
| result = flow.post_deal(pipeline) | |
| manager.add(pipeline) | |
| return {"state": state, "result": result} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/scripts/benchmark/generate_hugegraph_retrieval_outputs.py`
around lines 539 - 552, The init failure path in fetch/build_flow handling is
inconsistent with the run failure path because pipeline.init() raises without
returning the created pipeline to manager. In the flow that creates the
pipeline, add manager.add(pipeline) before raising on an init error so the same
resource management behavior is used as in the pipeline.run() failure branch;
use the pipeline, manager.add, init, and run symbols to update the error
handling consistently.
Comment on lines
+39
to
+42
| REPO_ROOT = Path(__file__).resolve().parents[3] | ||
| SRC_ROOT = REPO_ROOT / "src" | ||
| if str(SRC_ROOT) not in sys.path: | ||
| sys.path.insert(0, str(SRC_ROOT)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 核实脚本相对 hugegraph_llm 包的实际目录深度
fd -t d hugegraph_llm hugegraph-llm/src
fd fix_car33_edge_ids.py generate_text2kgbench_candidates.pyRepository: hugegraph/hugegraph-ai
Length of output: 316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 定位目标文件并查看相关行
git ls-files 'hugegraph-llm/scripts/benchmark/*candidates.py' 'hugegraph-llm/scripts/benchmark/*outputs.py'
echo '---'
for f in $(git ls-files 'hugegraph-llm/scripts/benchmark/*candidates.py' 'hugegraph-llm/scripts/benchmark/*outputs.py'); do
echo "FILE: $f"
wc -l "$f"
echo '--- head/tail ---'
sed -n '1,120p' "$f" | nl -ba | sed -n '1,120p'
echo '===='
done
# 用只读 Python 计算目标脚本的 parents 层级
python3 - <<'PY'
from pathlib import Path
p = Path('hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.py').resolve()
print('resolved:', p)
for i in range(6):
print(i, p.parents[i])
PYRepository: hugegraph/hugegraph-ai
Length of output: 517
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 精确搜索 REPO_ROOT / SRC_ROOT 片段，以及另一个脚本是否同样计算
rg -n "Path\\(__file__\\)\\.resolve\\(\\)\\.parents\\[[0-9]+\\]|SRC_ROOT = REPO_ROOT / \"src\"|sys\\.path\\.insert" hugegraph-llm/scripts/benchmarkRepository: hugegraph/hugegraph-ai
Length of output: 1862
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.py').resolve()
print('resolved file:', p)
for i in range(5):
parent = p.parents[i]
print(f'parents[{i}]: {parent} exists={parent.exists()}')
print('repo_root_candidate (parents[3]):', p.parents[3], 'src_exists=', (p.parents[3] / 'src').exists())
print('hugegraph_llm_src_exists=', (p.parents[2] / 'src').exists())
print('hugegraph_llm_package_exists=', (p.parents[2] / 'src' / 'hugegraph_llm').exists())
PY
echo '---'
git ls-files 'src/**' 'hugegraph-llm/src/**' | sed -n '1,80p'Repository: hugegraph/hugegraph-ai
Length of output: 6049
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import importlib.util, sys
from pathlib import Path
script = Path('hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.py').resolve()
repo_root = script.parents[3]
llm_root = script.parents[2]
wrong_src = repo_root / 'src'
right_src = llm_root / 'src'
def probe(label, paths):
old = sys.path[:]
sys.path[:] = [str(p) for p in paths] + old
spec = importlib.util.find_spec('hugegraph_llm')
print(label, 'paths=', [str(p) for p in paths], 'spec_origin=', None if spec is None else spec.origin)
sys.path[:] = old
print('repo_root=', repo_root)
print('wrong_src_exists=', wrong_src.exists())
print('right_src_exists=', right_src.exists())
print('pkg_exists=', (right_src / 'hugegraph_llm').exists())
probe('wrong_only', [wrong_src])
probe('right_only', [right_src])
probe('both', [wrong_src, right_src])
PYRepository: hugegraph/hugegraph-ai
Length of output: 607
REPO_ROOT 多算了一层，直跑脚本时会找不到 hugegraph_llm
hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.py 和 generate_hugegraph_retrieval_outputs.py 里都用了 Path(__file__).resolve().parents[3]，这会把 REPO_ROOT 指到仓库上一级目录，随后 SRC_ROOT = REPO_ROOT / "src" 变成不存在的路径。未安装包时直接运行脚本会因此 import hugegraph_llm 失败；这里应改成 parents[2]。
建议修复
-REPO_ROOT = Path(__file__).resolve().parents[3]
+REPO_ROOT = Path(__file__).resolve().parents[2]
SRC_ROOT = REPO_ROOT / "src"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| REPO_ROOT = Path(__file__).resolve().parents[3] | |
| SRC_ROOT = REPO_ROOT / "src" | |
| if str(SRC_ROOT) not in sys.path: | |
| sys.path.insert(0, str(SRC_ROOT)) | |
| REPO_ROOT = Path(__file__).resolve().parents[2] | |
| SRC_ROOT = REPO_ROOT / "src" | |
| if str(SRC_ROOT) not in sys.path: | |
| sys.path.insert(0, str(SRC_ROOT)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/scripts/benchmark/generate_text2kgbench_candidates.py` around
lines 39 - 42, `REPO_ROOT` in the benchmark script is computed one level too
high, which makes `SRC_ROOT` point outside the repository and breaks direct
imports of `hugegraph_llm` when running the script standalone. Update the path
setup in `generate_text2kgbench_candidates.py` (and the matching logic in
`generate_hugegraph_retrieval_outputs.py`) to use the correct ancestor from
`__file__`, so `REPO_ROOT` resolves to the repository root and `SRC_ROOT`
remains the actual `src` directory used for `sys.path` insertion.
Comment on lines
+79
to
+104
| def _run_metric_safe( | ||
| self, | ||
| metric: BaseMetric, | ||
| prediction: Any, | ||
| reference: Any, | ||
| sample_id: str, | ||
| **kwargs: Any, | ||
| ) -> Dict[str, float]: | ||
| """Run a metric with error tracking (thread-safe). | ||
|
|
||
| On success returns the metric scores dict. | ||
| On failure records the error in ``self._errors`` and returns ``{}``. | ||
| """ | ||
| try: | ||
| return metric.calculate(prediction=prediction, reference=reference, **kwargs) | ||
| except Exception as e: | ||
| with self._errors_lock: | ||
| self._errors.append( | ||
| { | ||
| "sample_id": sample_id, | ||
| "metric": metric.name, | ||
| "error": str(e), | ||
| } | ||
| ) | ||
| logger.exception("Metric %s failed for sample %s", metric.name, sample_id) | ||
| return {} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
建议在基类中统一校验 metric.requires_llm,而不是让各 runner 自行实现。
BaseMetric 已经有 requires_llm 属性(见 test_base_runner.py 中的 stub 指标)。目前只有 RetrievalRunner 手工维护了一份 _CONTEXT_METRICS 集合来做 fail-fast 校验(llm is None 时提前报错),而 AnswerRunner 完全没有类似校验,依赖需要 LLM 的指标(如 answer_correctness、faithfulness)在缺少 llm 时静默失败并进入 _errors。
建议在 _create_metric_instances 或 _run_metric_safe 中基于 metric.requires_llm 统一做前置校验,这样所有 runner(包括未来新增的)都能获得一致的 fail-fast 行为,减少每个 runner 重复实现校验逻辑的成本。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/src/hugegraph_llm/benchmark/runners/base_runner.py` around
lines 79 - 104, BaseRunner currently runs metrics without checking whether they
require an LLM, so metrics like answer_correctness and faithfulness can fail
silently instead of failing fast. Add a centralized pre-check in BaseRunner,
ideally in _create_metric_instances or _run_metric_safe, that inspects each
BaseMetric.requires_llm and raises/short-circuits when llm is missing; then
remove runner-specific duplication like RetrievalRunner’s _CONTEXT_METRICS so
all runners get consistent behavior.
Comment on lines
50
to
53
| def get_text_embedding(self, text: str) -> List[float]: | ||
| """Comment""" | ||
| response = self.client.embeddings.create(input=text, model=self.model) | ||
| """Get embedding for a single text with retry.""" | ||
| response = self._embed_with_retry([text]) | ||
| return response.data[0].embedding |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
单文本 embedding 路径缺少截断/重试保护，与批量路径不一致
批量方法 (get_texts_embeddings / async_get_texts_embeddings) 都先调用 _truncate_texts 再走 _embed_with_retry / _async_embed_with_retry，但：
get_text_embedding（第 50-53 行）只做了重试，没有截断，超长单文本仍可能超出 provider 的 token 上限（正是引入截断要解决的问题）。async_get_text_embedding（第 97-99 行）既没有截断也没有重试，直接调用self.aclient.embeddings.create，是本类中唯一没有瞬时错误重试保护的外部调用路径。
建议让这两个方法与批量路径保持一致的截断+重试策略。
🛡️ 建议的修改
def get_text_embedding(self, text: str) -> List[float]:
"""Get embedding for a single text with retry."""
- response = self._embed_with_retry([text])
+ response = self._embed_with_retry(self._truncate_texts([text]))
return response.data[0].embedding async def async_get_text_embedding(self, text: str) -> List[float]:
- response = await self.aclient.embeddings.create(input=[text], model=self.model)
+ response = await self._async_embed_with_retry(self._truncate_texts([text]))
return response.data[0].embeddingAlso applies to: 97-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/src/hugegraph_llm/models/embeddings/openai.py` around lines 50
- 53, The single-text embedding paths are inconsistent with the batch APIs:
get_text_embedding and async_get_text_embedding skip the truncation and retry
protections used by get_texts_embeddings and async_get_texts_embeddings. Update
these methods in openai.py to first pass input through the same _truncate_texts
flow as the batch methods, then use _embed_with_retry and
_async_embed_with_retry respectively, so both sync and async single-text calls
match the provider safety behavior.
Comment on lines
+1
to
+16
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
许可证头缺少 "the"，导致 CI 许可证检查失败。
第 5 行为 # to you under Apache License, Version 2.0 (the，缺少 "the"（应为 to you under the Apache License, Version 2.0 (the），与本文件其他两处失败的 license-eye 检查完全吻合。
🐛 建议修复
-# to you under Apache License, Version 2.0 (the
+# to you under the Apache License, Version 2.0 (the📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Licensed to the Apache Software Foundation (ASF) under one | |
| # or more contributor license agreements. See the NOTICE file | |
| # distributed with this work for additional information | |
| # regarding copyright ownership. The ASF licenses this file | |
| # to you under Apache License, Version 2.0 (the | |
| # "License"); you may not use this file except in compliance | |
| # with the License. You may obtain a copy of the License at | |
| # | |
| # http://www.apache.org/licenses/LICENSE-2.0 | |
| # | |
| # Unless required by applicable law or agreed to in writing, | |
| # software distributed under the License is distributed on an | |
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | |
| # KIND, either express or implied. See the License for the | |
| # specific language governing permissions and limitations | |
| # under the License. | |
| # Licensed to the Apache Software Foundation (ASF) under one | |
| # or more contributor license agreements. See the NOTICE file | |
| # distributed with this work for additional information | |
| # regarding copyright ownership. The ASF licenses this file | |
| # to you under the Apache License, Version 2.0 (the | |
| # "License"); you may not use this file except in compliance | |
| # with the License. You may obtain a copy of the License at | |
| # | |
| # http://www.apache.org/licenses/LICENSE-2.0 | |
| # | |
| # Unless required by applicable law or agreed to in writing, | |
| # software distributed under the License is distributed on an | |
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | |
| # KIND, either express or implied. See the License for the | |
| # specific language governing permissions and limitations | |
| # under the License. |
🧰 Tools
🪛 GitHub Actions: License header & 3rd-party check / 1_check-license-header.txt
[error] 1-1: license-eye header check failed: file does not have a valid license header.
🪛 GitHub Actions: License header & 3rd-party check / check-license-header
[error] 1-1: license-eye: file does not have a valid license header
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/src/hugegraph_llm/models/rerankers/jina.py` around lines 1 -
16, License header text is missing “the” in the Apache 2.0 notice, causing the
license check to fail. Update the header in jina.py so the standard ASF license
line matches the expected wording used elsewhere in the repo, keeping the same
header block and correcting the phrase in the license sentence.
Source: Pipeline failures
…dules to origin/main - Delete docs/quality/benchmark-code-style-spec.md, hugegraph-llm/BENCHMARK_DATASETS.md, hugegraph-llm/GRAPHRAG_BENCHMARK.md and hugegraph-llm/docs/benchmark/experiment-*.md. - Delete the entire hugegraph-llm/scripts/benchmark/ directory. - Revert local changes to llm_config.py, graph_extract.py, embeddings/llms openai.py, rerankers, info_extract.py, property_graph_extract.py, schema_build.py, ai_state.py, embedding_utils.py and test_graph_extract_configurable_split.py. - Keep benchmark-only changes: MetricRegistry direction arrows in MarkdownReporter, removal of the misleading Low-performing Samples section, and ruff formatting. - Fix benchmark CLI to only import project llm_settings when no explicit settings are provided, avoiding local .env reranker_type=jina failures in tests.
…other modules to origin/main" This reverts commit 209c388.
…y scope This re-applies the cleanup from 209c388: - Delete docs/quality/benchmark-code-style-spec.md - Delete hugegraph-llm/BENCHMARK_DATASETS.md and GRAPHRAG_BENCHMARK.md - Delete hugegraph-llm/docs/benchmark/experiment-*.md - Delete the entire hugegraph-llm/scripts/benchmark/ directory - Revert local changes to llm_config.py, graph_extract.py, embeddings/llms openai.py, rerankers, info_extract.py, property_graph_extract.py, schema_build.py, ai_state.py, embedding_utils.py and test_graph_extract_configurable_split.py
Rewrite MarkdownReporter into a four-layer inverted-pyramid layout designed for PR/Issue comments: 概览 (TL;DR) → 分析 (programmatic roll-up) → 指标总览 (changed-only) → 退化/改进样例 (one row per sample, detail folded) → 证据层 (folded). - Add metrics/dimensions.py: metric → (domain, sub-dimension) mapping, the single source of truth replacing the old _METRIC_GROUPS table. Presentation-only; does NOT influence regression verdicts. - ComparisonResult: add baseline_overall/candidate_overall (true before/after values, no reverse-engineering from delta), keep analyze() as a pure function with no memoization hacks. - analyze(): roll up by domain/sub-dimension, detect question-type clustering and per-sample concentration; add DEFAULT_RATIO_DELTA (0.01) so sub-1% wobble is treated as 持平 instead of 退化/改进 noise. - Per-sample regression judgment also floors at DEFAULT_RATIO_DELTA, keeping LLM-Judge's higher 0.05 tolerance.
读一份真实 baseline，按 metric 维度分桶注入可控扰动（recall@1 退化、 mrr/hit@5 改进、其余抖动），生成一个有真实感的 candidate，让 compare 能现场演示退化/改进/持平的全谱，无需重跑评测流程。用于演示测评闭环。 可复现（固定 --seed），幅度由 --magnitude 控制。
…ehavior - download.py: support HF parquet format for hotpotqa/2wikimultihopqa/musique - prepare_external_datasets.py: add sentence-level gold_evidence extraction - registry.py: update dataset postprocessing configs - syntax_validity.py: raise ValueError on empty raw_responses instead of silent 0 - .gitignore: add AGENTS.local.md
…SemanticTripleF1, ExtractionFaithfulness) - semantic_entity_f1.py: LLM judge matches candidate↔gold entities via semantic equivalence - semantic_triple_f1.py: LLM judge matches candidate↔gold triples (source/relation/target/direction) - extraction_faithfulness.py: GT-free faithfulness check against input text (deepeval NLI pattern) - prompts.py: add 3 prompt templates × EN/ZH with few-shot examples (car33 评分规则 aligned) - extraction_runner.py: register new metric data mappings + pass input_text for faithfulness - dimensions.py: classify semantic metrics under 'extraction → 语义匹配' sub-dimension
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/__init__.py (1)
40-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win循环导入设计脆弱，建议抽取公共辅助模块
_edge_in/_edge_out定义在本文件内，而semantic_triple_f1.py、extraction_faithfulness.py又反向从本包__init__导入它们（from hugegraph_llm.benchmark.metrics.extraction import _edge_in, _edge_out）。当前能正常工作纯粹依赖__init__.py内部“先定义辅助函数、再导入子模块”的顺序；一旦顺序被打乱（如新增子模块导入插在辅助函数定义之前），将触发ImportError。建议将
_is_edge/_edge_out/_edge_in抽取到独立模块（如_edge_utils.py），由__init__.py和各 metric 文件平等导入，消除对包初始化顺序的隐性依赖。♻️ 建议方案
+# hugegraph_llm/benchmark/metrics/extraction/_edge_utils.py +def _is_edge(item): ... +def _edge_out(item): ... +def _edge_in(item): ...-# extraction/__init__.py 中的 _is_edge/_edge_out/_edge_in 定义 +from hugegraph_llm.benchmark.metrics.extraction._edge_utils import _is_edge, _edge_out, _edge_in-from hugegraph_llm.benchmark.metrics.extraction import _edge_in, _edge_out +from hugegraph_llm.benchmark.metrics.extraction._edge_utils import _edge_in, _edge_out🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/__init__.py` around lines 40 - 50, 当前包初始化依赖顺序脆弱：`_edge_in`/_edge_out`/`_is_edge` 定义在 `hugegraph_llm.benchmark.metrics.extraction.__init__`，而 `semantic_triple_f1.py`、`extraction_faithfulness.py` 又从包入口反向导入这些符号。请把这些公共辅助函数抽到独立模块（例如 `_edge_utils.py`），然后让 `__init__`、`semantic_triple_f1`、`extraction_faithfulness` 统一从该模块导入，避免对 `__init__` 内部定义顺序的隐式依赖。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hugegraph-llm/scripts/benchmark/jitter_baseline.py`:
- Around line 166-168: The Ruff format check is failing in the
jitter_baseline.py benchmark script, so reformat the affected section and any
related surrounding code with the project formatter. Use the existing
benchmark/reporting code around the degradation/improvement printing in the main
comparison flow, then run uv run ruff format . to ensure the file passes ruff
format --check . before resubmitting.
- Around line 65-66: The prefix check in jitter_baseline.py is too broad and
causes recall@10/recall@100 to match recall@1 in the regression bucket logic.
Tighten the matching in the metric classification path inside the function that
returns "down" so it only treats exact top-1 metrics as recall@1, or uses a
delimiter-aware prefix match, while keeping the existing REGRESS_PREFIXES
handling for other metrics.
- Around line 81-94: In the baseline jittering logic that iterates over metrics
in jitter_baseline.py, avoid applying the [0, 1] clamp to
count-like/sample-level metrics such as semantic_entity_matched. Update the
metric mutation path so only normalized ratio metrics are clamped, while integer
count metrics keep their natural non-negative scale after applying noise. Use
the existing direction/_direction_for flow to distinguish count metrics and
preserve valid candidate baseline values for compare reports.
In `@hugegraph-llm/src/hugegraph_llm/benchmark/datasets/download.py`:
- Around line 204-207: The context_list construction in the download dataset
flow is treating string sentences as generic iterables, so list(s) splits them
into characters. Update the logic in the loop/comprehension around context_list
to special-case str before the iterable branch, and only convert non-string
iterables to lists while wrapping plain strings as a single-item list.
In `@hugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/prompts.py`:
- Around line 675-684: The matching rules in prompts.py are inconsistent about
the warning-light exception: the general rule requires exact label equality, but
the special case then permits cross-type matching for Component/Status. Update
the wording around the matching rules section to explicitly state whether this
exception is allowed, and if so, list the exact label combinations it applies
to; also make the example consistent with that rule so the LLM judge behavior is
unambiguous.
- Around line 800-813: The few-shot example in the prompt contains a typo in the
relation name, so update the example inside the prompt template in prompts.py to
use the same relation symbol as the candidate triple. Fix the mismatch in the
example’s reasoning text and keep the relation name consistent with the
HAS_COMPONENT triple referenced in the example.
In
`@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/extraction_faithfulness.py`:
- Line 1: The Python formatting check is failing for the extraction_faithfulness
module, so reformat the codebase with the project’s Ruff formatter and ensure it
matches the existing style. Apply the formatting fix to the touched Python code
in extraction_faithfulness.py, then rerun the required root checks with ruff
format and ruff check before handing off.
- Around line 89-100: The extraction_faithfulness metric silently returns all
None when prediction is neither dict nor list, making upstream data-format
errors indistinguishable from the no-LLM path. In the extraction_faithfulness
logic, add a warning/error log in the invalid-prediction branch before returning
the None result, and include the unexpected prediction type or a safe summary of
its value so callers can trace malformed input while keeping the existing return
shape unchanged.
- Around line 125-146: The extraction faithfulness counting in the judgment
aggregation is using raw verdict entries, which can overcount duplicated or
extra positive items. Update the logic in extraction_faithfulness.py to read the
returned verdicts by idx, ignore out-of-range indices, and deduplicate before
counting faithful items so the total stays consistent with items. Also make sure
the parsing/aggregation around retry_llm_call, _parse_json_response, and the
verdicts loop uses idx (not index) from the model output.
In
`@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_entity_f1.py`:
- Line 122: The current change in semantic_entity_f1.py needs Ruff formatting
cleanup because ruff format --check is failing. Run uv run ruff format . from
the repository root and ensure the formatted result is applied to the
semantic_entity_f1.py logic around the F1 calculation so the file matches Ruff’s
expected style before committing.
- Around line 108-118: The `SemanticEntityF1` match parsing currently trusts
`data["matches"]` too much, which can inflate `matched` when the LLM returns
duplicate, out-of-range, or non-integer indices. Update the match normalization
logic in `semantic_entity_f1.py` to validate each pair against `gold_lines` and
`cand_lines`, discard invalid indices, and deduplicate matches before computing
`matched`, `gold_count`, and `cand_count`. Keep the fix localized to the match
extraction block so `matches` only contains unique, in-bounds integer pairs
before the metric calculation.
- Around line 52-58: 实体行当前是在 semantic_entity_f1.py 的格式化逻辑里手写拼接 JSON-like 字符串，未对
label/name 做转义，容易在包含引号、换行或花括号时破坏 prompt 结构；请在该段遍历 vertices 的代码中改为使用 json.dumps
生成每一行的实体文本，确保 label 和 name 都按 JSON 规则序列化后再 append 到 lines，从而避免特殊字符污染输出。
In
`@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_triple_f1.py`:
- Line 1: CI formatting check failed because the Python file is not
Ruff-formatted. Run the repository root formatting command to apply the required
style, then verify the change in semantic_triple_f1.py and any related Python
edits with the same Ruff workflow before handing off. Use the file’s module
content in hugegraph_llm.benchmark.metrics.extraction.semantic_triple_f1 and
ensure the final diff passes the formatting check.
- Around line 75-81: The empty-triple case in the semantic metric is being
treated as a worst-case miss, but `semantic_triple_f1` should consider
`prediction` and `reference` both empty as a perfect match. Update the
empty-input branch in `semantic_triple_f1` so it returns precision/recall/f1 as
1.0 (with matched count 0 or equivalent) when both lists are empty, while
keeping the existing handling for one-sided empties unchanged. Use the
`semantic_triple_f1` function as the anchor when adjusting this logic.
- Around line 99-124: In the semantic triple F1 metric logic, the current
handling of LLM-returned matches only checks that each entry is a 2-item list,
so duplicate pairs or out-of-range indices can inflate matched counts and make
precision/recall invalid. Update the match normalization in the code path that
uses retry_llm_call and _parse_json_response to validate each (gold_idx,
cand_idx) against gold_lines and cand_lines bounds, deduplicate valid pairs
before counting, and compute matched from the filtered unique set so the
returned semantic_triple_precision, semantic_triple_recall, and
semantic_triple_f1 stay bounded and comparable.
In `@hugegraph-llm/src/hugegraph_llm/benchmark/reporters/markdown_reporter.py`:
- Around line 289-317: In `_section_samples`, the sorting direction for
summarized rows is wrong for the “improvements” case:
`summarized.sort(key=lambda r: r["worst_delta"])` works for regressions but
causes the largest improvements to fall out of the top-N. Update the sort logic
in `_section_samples` (using `change_key`, `summarized`, and `worst_delta`) so
regressions stay ascending by most negative delta, while improvements are
ordered to show the largest positive deltas first before applying `limit`.
---
Nitpick comments:
In `@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/__init__.py`:
- Around line 40-50: 当前包初始化依赖顺序脆弱：`_edge_in`/_edge_out`/`_is_edge` 定义在
`hugegraph_llm.benchmark.metrics.extraction.__init__`，而
`semantic_triple_f1.py`、`extraction_faithfulness.py`
又从包入口反向导入这些符号。请把这些公共辅助函数抽到独立模块（例如 `_edge_utils.py`），然后让
`__init__`、`semantic_triple_f1`、`extraction_faithfulness` 统一从该模块导入，避免对
`__init__` 内部定义顺序的隐式依赖。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9493a029-1242-413f-acf8-18b265d7c491
📒 Files selected for processing (15)
.gitignorehugegraph-llm/scripts/benchmark/jitter_baseline.pyhugegraph-llm/src/hugegraph_llm/benchmark/baseline/compare.pyhugegraph-llm/src/hugegraph_llm/benchmark/datasets/download.pyhugegraph-llm/src/hugegraph_llm/benchmark/datasets/prepare_external_datasets.pyhugegraph-llm/src/hugegraph_llm/benchmark/datasets/registry.pyhugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/prompts.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/dimensions.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/__init__.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/extraction_faithfulness.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_entity_f1.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_triple_f1.pyhugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/syntax_validity.pyhugegraph-llm/src/hugegraph_llm/benchmark/reporters/markdown_reporter.pyhugegraph-llm/src/hugegraph_llm/benchmark/runners/extraction_runner.py
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (4)
- hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/syntax_validity.py
- hugegraph-llm/src/hugegraph_llm/benchmark/runners/extraction_runner.py
- hugegraph-llm/src/hugegraph_llm/benchmark/datasets/registry.py
- hugegraph-llm/src/hugegraph_llm/benchmark/datasets/prepare_external_datasets.py
Comment on lines
+65
to
+66
| if any(metric == p or metric.startswith(p) for p in REGRESS_PREFIXES): | ||
| return "down" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
避免把 recall@10 误判为 recall@1。
Line 65 的 startswith("recall@1") 会同时匹配 recall@10、recall@100，导致非 top-1 指标被错误标记为退化 bucket。
建议修复
- if any(metric == p or metric.startswith(p) for p in REGRESS_PREFIXES):
+ if metric in REGRESS_PREFIXES:
return "down"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if any(metric == p or metric.startswith(p) for p in REGRESS_PREFIXES): | |
| return "down" | |
| if metric in REGRESS_PREFIXES: | |
| return "down" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/scripts/benchmark/jitter_baseline.py` around lines 65 - 66, The
prefix check in jitter_baseline.py is too broad and causes recall@10/recall@100
to match recall@1 in the regression bucket logic. Tighten the matching in the
metric classification path inside the function that returns "down" so it only
treats exact top-1 metrics as recall@1, or uses a delimiter-aware prefix match,
while keeping the existing REGRESS_PREFIXES handling for other metrics.
Comment on lines
+81
to
+94
| for name, value in list(metrics.items()): | ||
| if value is None or not isinstance(value, (int, float)): | ||
| continue | ||
| direction = _direction_for(name) | ||
| if direction == "down": | ||
| # 退化：大概率显著下降 | ||
| delta = -magnitude * (0.5 + rng.random()) | ||
| elif direction == "up": | ||
| # 改进：大概率上升 | ||
| delta = magnitude * (0.5 + rng.random()) | ||
| else: | ||
| # 噪音：小双向 | ||
| delta = (rng.random() - 0.5) * magnitude * 0.4 | ||
| metrics[name] = round(_clamp01(value + delta), 4) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
不要对计数型指标做 [0, 1] 裁剪。
semantic_entity_matched 这类 sample-level 计数会被当前逻辑抖动并裁剪成 0~1，生成的 candidate baseline 会包含不可能的计数值，影响 compare 报告可信度。
建议修复
+COUNT_METRIC_SUFFIXES = ("_matched", "_count")
+
...
for name, value in list(metrics.items()):
if value is None or not isinstance(value, (int, float)):
continue
+ if name.endswith(COUNT_METRIC_SUFFIXES):
+ continue
direction = _direction_for(name)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for name, value in list(metrics.items()): | |
| if value is None or not isinstance(value, (int, float)): | |
| continue | |
| direction = _direction_for(name) | |
| if direction == "down": | |
| # 退化：大概率显著下降 | |
| delta = -magnitude * (0.5 + rng.random()) | |
| elif direction == "up": | |
| # 改进：大概率上升 | |
| delta = magnitude * (0.5 + rng.random()) | |
| else: | |
| # 噪音：小双向 | |
| delta = (rng.random() - 0.5) * magnitude * 0.4 | |
| metrics[name] = round(_clamp01(value + delta), 4) | |
| COUNT_METRIC_SUFFIXES = ("_matched", "_count") | |
| for name, value in list(metrics.items()): | |
| if value is None or not isinstance(value, (int, float)): | |
| continue | |
| if name.endswith(COUNT_METRIC_SUFFIXES): | |
| continue | |
| direction = _direction_for(name) | |
| if direction == "down": | |
| # 退化：大概率显著下降 | |
| delta = -magnitude * (0.5 + rng.random()) | |
| elif direction == "up": | |
| # 改进：大概率上升 | |
| delta = magnitude * (0.5 + rng.random()) | |
| else: | |
| # 噪音：小双向 | |
| delta = (rng.random() - 0.5) * magnitude * 0.4 | |
| metrics[name] = round(_clamp01(value + delta), 4) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/scripts/benchmark/jitter_baseline.py` around lines 81 - 94, In
the baseline jittering logic that iterates over metrics in jitter_baseline.py,
avoid applying the [0, 1] clamp to count-like/sample-level metrics such as
semantic_entity_matched. Update the metric mutation path so only normalized
ratio metrics are clamped, while integer count metrics keep their natural
non-negative scale after applying noise. Use the existing
direction/_direction_for flow to distinguish count metrics and preserve valid
candidate baseline values for compare reports.
Comment on lines
+166
to
+168
| print(f" 退化 {k}: {orig_overall[k]:.4f} -> {new_overall[k]:.4f} ({d:+.4f})", file=sys.stderr) | ||
| for k, d in sorted(improved, key=lambda x: -x[1])[:5]: | ||
| print(f" 改进 {k}: {orig_overall[k]:.4f} -> {new_overall[k]:.4f} ({d:+.4f})", file=sys.stderr) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
先修复 Ruff 格式化失败。
CI 已报告该文件 ruff format --check . 不通过；提交前请运行 uv run ruff format .。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/scripts/benchmark/jitter_baseline.py` around lines 166 - 168,
The Ruff format check is failing in the jitter_baseline.py benchmark script, so
reformat the affected section and any related surrounding code with the project
formatter. Use the existing benchmark/reporting code around the
degradation/improvement printing in the main comparison flow, then run uv run
ruff format . to ensure the file passes ruff format --check . before
resubmitting.
Source: Pipeline failures
Comment on lines
+204
to
+207
| context_list = [ | ||
| [str(t), list(s) if hasattr(s, "__iter__") else [str(s)]] | ||
| for t, s in zip(titles, sentences) | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
避免把单句字符串拆成字符。
当 sentences 的元素已经是字符串时，list(s) 会变成字符数组，写出的 corpus/QA 会被破坏；需要先单独处理 str。
建议修复
- context_list = [
- [str(t), list(s) if hasattr(s, "__iter__") else [str(s)]]
- for t, s in zip(titles, sentences)
- ]
+ context_list = []
+ for t, s in zip(titles, sentences):
+ if isinstance(s, str):
+ sentence_list = [s]
+ elif hasattr(s, "__iter__"):
+ sentence_list = [str(item) for item in s]
+ else:
+ sentence_list = [str(s)]
+ context_list.append([str(t), sentence_list])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| context_list = [ | |
| [str(t), list(s) if hasattr(s, "__iter__") else [str(s)]] | |
| for t, s in zip(titles, sentences) | |
| ] | |
| context_list = [] | |
| for t, s in zip(titles, sentences): | |
| if isinstance(s, str): | |
| sentence_list = [s] | |
| elif hasattr(s, "__iter__"): | |
| sentence_list = [str(item) for item in s] | |
| else: | |
| sentence_list = [str(s)] | |
| context_list.append([str(t), sentence_list]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/src/hugegraph_llm/benchmark/datasets/download.py` around lines
204 - 207, The context_list construction in the download dataset flow is
treating string sentences as generic iterables, so list(s) splits them into
characters. Update the logic in the loop/comprehension around context_list to
special-case str before the iterable branch, and only convert non-string
iterables to lists while wrapping plain strings as a single-item list.
Comment on lines
+675
to
+684
| Matching rules: | ||
| - Entity TYPE (label) must match exactly. Component ≠ Function, Status ≠ Specification. | ||
| - Entity NAME allows: synonym normalization, abbreviation expansion, phrasing variation. | ||
| Example: "制动液" matches "制动液检查/更换" (same core concept, different granularity). | ||
| - Each gold entity can be matched at most once. | ||
| - Each candidate entity can be matched at most once. | ||
| - If two candidate entities match the same gold entity, the first one wins. | ||
| - Special case: A warning-light Component in the gold that is expressed as | ||
| Status in the candidate may still match if the semantic signal is identical | ||
| (e.g., gold "Status(ABS故障警告灯)" ↔ candidate "Status(ABS系统故障指示)"). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
澄清实体类型匹配的例外规则。
这里先要求 label 必须完全一致，随后又允许 warning-light 的 Component/Status 跨类型匹配；英文示例还写成了 Status↔Status。请明确该例外是否真实允许，以及仅限哪些 label 组合，避免 LLM judge 产生不稳定结果。
Also applies to: 725-732
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/src/hugegraph_llm/benchmark/llm_judge/prompts.py` around lines
675 - 684, The matching rules in prompts.py are inconsistent about the
warning-light exception: the general rule requires exact label equality, but the
special case then permits cross-type matching for Component/Status. Update the
wording around the matching rules section to explicitly state whether this
exception is allowed, and if so, list the exact label combinations it applies
to; also make the example consistent with that rule so the LLM judge behavior is
unambiguous.
|
|
||
| precision = matched / cand_count if cand_count > 0 else 0.0 | ||
| recall = matched / gold_count if gold_count > 0 else 0.0 | ||
| f1 = 2 * precision * recall / (precision + recall) if (precision + recall) > 0 else 0.0 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
先修复 Ruff 格式化失败。
CI 已报告该文件 ruff format --check . 不通过；提交前请在仓库根目录运行 uv run ruff format .。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_entity_f1.py`
at line 122, The current change in semantic_entity_f1.py needs Ruff formatting
cleanup because ruff format --check is failing. Run uv run ruff format . from
the repository root and ensure the formatted result is applied to the
semantic_entity_f1.py logic around the F1 calculation so the file matches Ruff’s
expected style before committing.
Source: Pipeline failures
| @@ -0,0 +1,174 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
CI 格式检查失败：需运行 ruff format
流水线报告 ruff format --check . 未通过，需要运行 uv run ruff format . 后重新提交。
As per coding guidelines, "For Python code changes, run root uv run ruff format --check . and uv run ruff check . before handoff."
🧰 Tools
🪛 GitHub Actions: Ruff Code Quality / build (3.10)
[error] 1-1: ruff format --check . reported this file would be reformatted. Run 'uv run ruff format .' to apply formatting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_triple_f1.py`
at line 1, CI formatting check failed because the Python file is not
Ruff-formatted. Run the repository root formatting command to apply the required
style, then verify the change in semantic_triple_f1.py and any related Python
edits with the same Ruff workflow before handing off. Use the file’s module
content in hugegraph_llm.benchmark.metrics.extraction.semantic_triple_f1 and
ensure the final diff passes the formatting check.
Sources: Coding guidelines, Pipeline failures
Comment on lines
+75
to
+81
| if not prediction and not reference: | ||
| return { | ||
| "semantic_triple_precision": 0.0, | ||
| "semantic_triple_recall": 0.0, | ||
| "semantic_triple_f1": 0.0, | ||
| "semantic_triple_matched": 0, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
空三元组集合应视为完全匹配，而非 0 分
当 prediction 和 reference 都为空列表时（即金标准和候选均无三元组），当前返回 precision/recall/f1 全部为 0.0。但语义上这是一次“无遗漏、无误报”的完美匹配，应记为 1.0（或从聚合统计中剔除该样本），而非当作最差情形处理。对于抽取结果本应为空的样本（如纯属性场景），当前实现会系统性拉低整体基准分数。
🐛 建议修复
if not prediction and not reference:
return {
- "semantic_triple_precision": 0.0,
- "semantic_triple_recall": 0.0,
- "semantic_triple_f1": 0.0,
+ "semantic_triple_precision": 1.0,
+ "semantic_triple_recall": 1.0,
+ "semantic_triple_f1": 1.0,
"semantic_triple_matched": 0,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if not prediction and not reference: | |
| return { | |
| "semantic_triple_precision": 0.0, | |
| "semantic_triple_recall": 0.0, | |
| "semantic_triple_f1": 0.0, | |
| "semantic_triple_matched": 0, | |
| } | |
| if not prediction and not reference: | |
| return { | |
| "semantic_triple_precision": 1.0, | |
| "semantic_triple_recall": 1.0, | |
| "semantic_triple_f1": 1.0, | |
| "semantic_triple_matched": 0, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_triple_f1.py`
around lines 75 - 81, The empty-triple case in the semantic metric is being
treated as a worst-case miss, but `semantic_triple_f1` should consider
`prediction` and `reference` both empty as a perfect match. Update the
empty-input branch in `semantic_triple_f1` so it returns precision/recall/f1 as
1.0 (with matched count 0 or equivalent) when both lists are empty, while
keeping the existing handling for one-sided empties unchanged. Use the
`semantic_triple_f1` function as the anchor when adjusting this logic.
Comment on lines
+99
to
+124
| matches: List[List[int]] = [] | ||
| try: | ||
| response = retry_llm_call(llm, prompt) | ||
| data = _parse_json_response(response) | ||
| if data and isinstance(data.get("matches"), list): | ||
| matches = [ | ||
| m for m in data["matches"] | ||
| if isinstance(m, list) and len(m) == 2 | ||
| ] | ||
| except Exception as e: | ||
| logger.warning("Semantic triple matching failed: %s", e) | ||
|
|
||
| gold_count = len(gold_lines) | ||
| cand_count = len(cand_lines) | ||
| matched = len(matches) | ||
|
|
||
| precision = matched / cand_count if cand_count > 0 else 0.0 | ||
| recall = matched / gold_count if gold_count > 0 else 0.0 | ||
| f1 = 2 * precision * recall / (precision + recall) if (precision + recall) > 0 else 0.0 | ||
|
|
||
| return { | ||
| "semantic_triple_precision": round(precision, 4), | ||
| "semantic_triple_recall": round(recall, 4), | ||
| "semantic_triple_f1": round(f1, 4), | ||
| "semantic_triple_matched": matched, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
LLM 返回的 matches 未做索引范围与去重校验，可能导致分数虚高
matches 仅校验元素是长度为 2 的列表，未校验 (gold_idx, cand_idx) 是否落在合法范围内，也未去重。若 LLM 输出重复索引对（例如一个候选三元组被判定匹配多个金标准，反之亦然），matched = len(matches) 会被放大，precision = matched / cand_count 理论上可超过 1.0，破坏该指标的可比较性。
🐛 建议修复：按索引去重并校验范围
matches: List[List[int]] = []
try:
response = retry_llm_call(llm, prompt)
data = _parse_json_response(response)
if data and isinstance(data.get("matches"), list):
- matches = [
- m for m in data["matches"]
- if isinstance(m, list) and len(m) == 2
- ]
+ seen_gold: set = set()
+ seen_cand: set = set()
+ valid_matches = []
+ for m in data["matches"]:
+ if not (isinstance(m, list) and len(m) == 2):
+ continue
+ g_idx, c_idx = m
+ if not (isinstance(g_idx, int) and isinstance(c_idx, int)):
+ continue
+ if not (0 <= g_idx < len(gold_lines) and 0 <= c_idx < len(cand_lines)):
+ continue
+ if g_idx in seen_gold or c_idx in seen_cand:
+ continue
+ seen_gold.add(g_idx)
+ seen_cand.add(c_idx)
+ valid_matches.append(m)
+ matches = valid_matches
except Exception as e:
logger.warning("Semantic triple matching failed: %s", e)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| matches: List[List[int]] = [] | |
| try: | |
| response = retry_llm_call(llm, prompt) | |
| data = _parse_json_response(response) | |
| if data and isinstance(data.get("matches"), list): | |
| matches = [ | |
| m for m in data["matches"] | |
| if isinstance(m, list) and len(m) == 2 | |
| ] | |
| except Exception as e: | |
| logger.warning("Semantic triple matching failed: %s", e) | |
| gold_count = len(gold_lines) | |
| cand_count = len(cand_lines) | |
| matched = len(matches) | |
| precision = matched / cand_count if cand_count > 0 else 0.0 | |
| recall = matched / gold_count if gold_count > 0 else 0.0 | |
| f1 = 2 * precision * recall / (precision + recall) if (precision + recall) > 0 else 0.0 | |
| return { | |
| "semantic_triple_precision": round(precision, 4), | |
| "semantic_triple_recall": round(recall, 4), | |
| "semantic_triple_f1": round(f1, 4), | |
| "semantic_triple_matched": matched, | |
| } | |
| matches: List[List[int]] = [] | |
| try: | |
| response = retry_llm_call(llm, prompt) | |
| data = _parse_json_response(response) | |
| if data and isinstance(data.get("matches"), list): | |
| seen_gold: set = set() | |
| seen_cand: set = set() | |
| valid_matches = [] | |
| for m in data["matches"]: | |
| if not (isinstance(m, list) and len(m) == 2): | |
| continue | |
| g_idx, c_idx = m | |
| if not (isinstance(g_idx, int) and isinstance(c_idx, int)): | |
| continue | |
| if not (0 <= g_idx < len(gold_lines) and 0 <= c_idx < len(cand_lines)): | |
| continue | |
| if g_idx in seen_gold or c_idx in seen_cand: | |
| continue | |
| seen_gold.add(g_idx) | |
| seen_cand.add(c_idx) | |
| valid_matches.append(m) | |
| matches = valid_matches | |
| except Exception as e: | |
| logger.warning("Semantic triple matching failed: %s", e) | |
| gold_count = len(gold_lines) | |
| cand_count = len(cand_lines) | |
| matched = len(matches) | |
| precision = matched / cand_count if cand_count > 0 else 0.0 | |
| recall = matched / gold_count if gold_count > 0 else 0.0 | |
| f1 = 2 * precision * recall / (precision + recall) if (precision + recall) > 0 else 0.0 | |
| return { | |
| "semantic_triple_precision": round(precision, 4), | |
| "semantic_triple_recall": round(recall, 4), | |
| "semantic_triple_f1": round(f1, 4), | |
| "semantic_triple_matched": matched, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@hugegraph-llm/src/hugegraph_llm/benchmark/metrics/extraction/semantic_triple_f1.py`
around lines 99 - 124, In the semantic triple F1 metric logic, the current
handling of LLM-returned matches only checks that each entry is a 2-item list,
so duplicate pairs or out-of-range indices can inflate matched counts and make
precision/recall invalid. Update the match normalization in the code path that
uses retry_llm_call and _parse_json_response to validate each (gold_idx,
cand_idx) against gold_lines and cand_lines bounds, deduplicate valid pairs
before counting, and compute matched from the filtered unique set so the
returned semantic_triple_precision, semantic_triple_recall, and
semantic_triple_f1 stay bounded and comparable.
Comment on lines
+289
to
+317
| summarized: List[Dict[str, Any]] = [] | ||
| for entry in entries: | ||
| changes = entry.get(change_key, {}) | ||
| if not changes: | ||
| continue | ||
| # worst = most negative semantic delta (regression) or most positive (improvement) | ||
| worst_metric, worst_delta = min(changes.items(), key=lambda kv: kv[1]) \ | ||
| if change_key == "regressions" else max(changes.items(), key=lambda kv: kv[1]) | ||
| summarized.append( | ||
| { | ||
| "sample_id": entry["sample_id"], | ||
| "question_type": entry.get("question_type"), | ||
| "worst_metric": worst_metric, | ||
| "worst_delta": worst_delta, | ||
| "n_metrics": len(changes), | ||
| } | ||
| ) | ||
| summarized.sort(key=lambda r: r["worst_delta"]) # worst first | ||
|
|
||
| lines.append("| Sample | 最严重指标 | Δ | 涉及指标数 | 类型 |") | ||
| lines.append("|--------|-----------|-----|-----------|------|") | ||
| for row in summarized[:limit]: | ||
| qt = row["question_type"] or "—" | ||
| lines.append( | ||
| f"| {row['sample_id']} | {row['worst_metric']} | {_fmt_delta(row['worst_delta'])} " | ||
| f"| {row['n_metrics']} | {qt} |" | ||
| ) | ||
| if len(summarized) > limit: | ||
| lines.append(f"| ... | 还有 {len(summarized) - limit} 个样例见下方明细 | | | |") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
"改进样例"排序方向错误，最大改进被截断到折叠区
_section_samples 对退化样例和改进样例统一使用升序排序 (summarized.sort(key=lambda r: r["worst_delta"]))。对退化样例（worst_delta 为最负值）升序确实能把最严重的排在最前面；但对改进样例，worst_delta 存的是该样本内最大的正向提升，升序排序会把提升幅度最小的样本排在最前，当改进样例数超过 limit=5 时，提升幅度最大的样本反而被挤到"还有 N 个样例见下方明细"的折叠区，未在首屏展示，违背了"按严重程度排序展示 top-N"的设计初衷。
🐛 建议修复
- summarized.sort(key=lambda r: r["worst_delta"]) # worst first
+ summarized.sort(key=lambda r: r["worst_delta"], reverse=(change_key == "improvements"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| summarized: List[Dict[str, Any]] = [] | |
| for entry in entries: | |
| changes = entry.get(change_key, {}) | |
| if not changes: | |
| continue | |
| # worst = most negative semantic delta (regression) or most positive (improvement) | |
| worst_metric, worst_delta = min(changes.items(), key=lambda kv: kv[1]) \ | |
| if change_key == "regressions" else max(changes.items(), key=lambda kv: kv[1]) | |
| summarized.append( | |
| { | |
| "sample_id": entry["sample_id"], | |
| "question_type": entry.get("question_type"), | |
| "worst_metric": worst_metric, | |
| "worst_delta": worst_delta, | |
| "n_metrics": len(changes), | |
| } | |
| ) | |
| summarized.sort(key=lambda r: r["worst_delta"]) # worst first | |
| lines.append("| Sample | 最严重指标 | Δ | 涉及指标数 | 类型 |") | |
| lines.append("|--------|-----------|-----|-----------|------|") | |
| for row in summarized[:limit]: | |
| qt = row["question_type"] or "—" | |
| lines.append( | |
| f"| {row['sample_id']} | {row['worst_metric']} | {_fmt_delta(row['worst_delta'])} " | |
| f"| {row['n_metrics']} | {qt} |" | |
| ) | |
| if len(summarized) > limit: | |
| lines.append(f"| ... | 还有 {len(summarized) - limit} 个样例见下方明细 | | | |") | |
| summarized: List[Dict[str, Any]] = [] | |
| for entry in entries: | |
| changes = entry.get(change_key, {}) | |
| if not changes: | |
| continue | |
| # worst = most negative semantic delta (regression) or most positive (improvement) | |
| worst_metric, worst_delta = min(changes.items(), key=lambda kv: kv[1]) \ | |
| if change_key == "regressions" else max(changes.items(), key=lambda kv: kv[1]) | |
| summarized.append( | |
| { | |
| "sample_id": entry["sample_id"], | |
| "question_type": entry.get("question_type"), | |
| "worst_metric": worst_metric, | |
| "worst_delta": worst_delta, | |
| "n_metrics": len(changes), | |
| } | |
| ) | |
| summarized.sort(key=lambda r: r["worst_delta"], reverse=(change_key == "improvements")) | |
| lines.append("| Sample | 最严重指标 | Δ | 涉及指标数 | 类型 |") | |
| lines.append("|--------|-----------|-----|-----------|------|") | |
| for row in summarized[:limit]: | |
| qt = row["question_type"] or "—" | |
| lines.append( | |
| f"| {row['sample_id']} | {row['worst_metric']} | {_fmt_delta(row['worst_delta'])} " | |
| f"| {row['n_metrics']} | {qt} |" | |
| ) | |
| if len(summarized) > limit: | |
| lines.append(f"| ... | 还有 {len(summarized) - limit} 个样例见下方明细 | | | |") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hugegraph-llm/src/hugegraph_llm/benchmark/reporters/markdown_reporter.py`
around lines 289 - 317, In `_section_samples`, the sorting direction for
summarized rows is wrong for the “improvements” case:
`summarized.sort(key=lambda r: r["worst_delta"])` works for regressions but
causes the largest improvements to fall out of the top-N. Update the sort logic
in `_section_samples` (using `change_key`, `summarized`, and `worst_delta`) so
regressions stay ascending by most negative delta, while improvements are
ordered to show the largest positive deltas first before applying `limit`.
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
2 participants
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
Fixes #75.
This PR adds a lightweight, reproducible GraphRAG benchmark module for
hugegraph-llm. It is designed for fast local evaluation after changing graph extraction, retrieval, rerank, context construction, or answer generation logic.The benchmark intentionally keeps the base evaluation path offline and deterministic. LLM-as-Judge is available as an optional enhancement, but basic extraction, retrieval, answer, baseline, and report workflows do not require external LLM services.
Background
Issue #75 asks for a lightweight benchmark capability because HugeGraph-LLM already has GraphRAG features, but did not have a convenient way to answer these questions after changes:
This PR adds that missing benchmark layer as an independent package under
hugegraph-llm, without changing the existing GraphRAG runtime pipeline.What Changed
Benchmark CLI
Adds
hugegraph_llm.benchmarkwith a runnable CLI:Supported commands include:
run: run extraction, retrieval, or ablation evaluationbaseline save: persist a benchmark result as a named baselinecompare: compare candidate results with a baseline and surface metric deltas plus sample-level changeslist-metrics: inspect registered metricsEvaluation Runners
Adds three benchmark runners:
ExtractionRunner: evaluates graph extraction quality from expected graph and candidate graph outputsRetrievalRunner: evaluates evidence retrieval quality from questions, expected evidence, and candidate retrieved contextsAblationRunner: compares answer variants such as raw, vector-only, graph-only, and graph+vector answersThe runners share common behavior for:
Metrics
Adds offline-first metrics for the required base benchmark path.
Extraction metrics:
entity_f1triple_f1property_f1schema_validitystructural_integritysyntax_validitygraph_structureconflict_detectiontemporal_validityRetrieval metrics:
recall_at_khit_at_kmrrcontext_precisioncontext_relevancyevidence_recall_llmAnswer / ablation metrics:
token_f1exact_matchrouge_lanswer_correctnessfaithfulnesscoverageThe LLM-Judge metrics are optional. Offline metrics are sufficient for the default benchmark workflow.
Chinese and English Support
Adds bilingual sample data and normalization utilities for Chinese and English evaluation:
openccis availablejiebaThis addresses the issue requirement that the benchmark should be friendly to Chinese scenarios instead of assuming English-only whitespace tokenization.
Baseline and Candidate Comparison
Adds persistent baseline storage and comparison support:
This is intended for PR workflows where contributors need to show whether a GraphRAG change improves or regresses benchmark behavior.
Reports
Adds both JSON and Markdown reporters:
The Markdown report includes aggregate metrics and per-sample details so reviewers can inspect concrete failure or regression examples.
Dataset Preparation and Experiment Scripts
Adds dataset preparation and helper scripts under
hugegraph-llm/scripts/benchmark/:Generated experiment outputs and downloaded benchmark data are kept out of git by
.gitignore.Documentation
Adds detailed documentation in:
hugegraph-llm/GRAPHRAG_BENCHMARK.mdhugegraph-llm/scripts/benchmark/README.mddocs/quality/benchmark-code-style-spec.mdThe main benchmark document covers:
Requirement Mapping for Issue #75
GRAPHRAG_BENCHMARK.md, including why the implementation borrows metric ideas without adding heavyweight framework dependenciespython -m hugegraph_llm.benchmarkCLI with run / baseline / compare commandsbaseline savecommandGRAPHRAG_BENCHMARK.mdand scripts READMEDesign Notes
hugegraph_llm.benchmarkand does not modify existing GraphRAG runtime behavior.Validation
Local checks passed:
Test result:
The warning is from
jiebaimporting deprecatedpkg_resourcesthrough its dependency path. It does not affect benchmark behavior.Real Dataset Smoke Results
Small real-dataset checks were run before opening this PR. These are intentionally small because the PR should stay lightweight and reproducible.
The extraction oracle and controlled ablation results are sanity checks for metric coverage and distinguishability. They are not presented as production GraphRAG model quality numbers.
Scope and Non-Goals
This PR does not change the existing GraphRAG pipeline implementation. It adds the benchmark layer needed to evaluate future GraphRAG changes.
This PR also does not make external LLM providers mandatory for benchmark usage. LLM-Judge remains optional because Issue #75 explicitly asks that the basic benchmark should not depend on external LLM services.
Summary by CodeRabbit
#75评测方案与完整实验记录/复现步骤。