fix: save generated data to configured dataset - #229
Open
Iams4kura wants to merge 2 commits into
Open
Conversation
审查者指南更新 QA 生成逻辑,使其根据配置的数据集目录和 dataset_info 映射,将输出保存到选定的文件中,返回并记录实际路径,并增加回归测试以防止写入过时的硬编码默认数据集。 配置的 QA 数据集输出的时序图sequenceDiagram
participant Generator as QA_Generator
participant DatasetInfo as dataset_info.json
participant File as ConfiguredDatasetFile
participant CDF as cutoff_len_script
Generator->>DatasetInfo: read dataset_info.json
DatasetInfo-->>Generator: file_name for configured dataset
Generator->>File: write generated QA data
File-->>Generator: output_path
Generator->>CDF: _execute_length_cdf_script()
Generator-->>Generator: log output_path
文件级变更
提示和命令与 Sourcery 交互
自定义你的使用体验访问你的控制面板以:
获取帮助Original review guide in EnglishReviewer's GuideUpdates QA generation to save output to the file selected by the configured dataset directory and dataset_info mapping, returns and logs the actual path, and adds regression coverage preventing writes to the stale hardcoded default dataset. Sequence diagram for configured QA dataset outputsequenceDiagram
participant Generator as QA_Generator
participant DatasetInfo as dataset_info.json
participant File as ConfiguredDatasetFile
participant CDF as cutoff_len_script
Generator->>DatasetInfo: read dataset_info.json
DatasetInfo-->>Generator: file_name for configured dataset
Generator->>File: write generated QA data
File-->>Generator: output_path
Generator->>CDF: _execute_length_cdf_script()
Generator-->>Generator: log output_path
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
您好——我发现了 1 个问题
面向 AI Agent 的提示
请处理此代码审查中的评论:
## 单独的评论
### 评论 1
<location path="weclone/data/qa_generator.py" line_range="704-705" />
<code_context>
+ with open(dataset_info_path, "r", encoding="utf-8") as f:
+ dataset_info = json.load(f)
+
+ file_name = dataset_info.get(self.c.dataset, {}).get("file_name")
+ if not file_name:
+ raise ValueError(f"Dataset '{self.c.dataset}' must define file_name in {dataset_info_path}")
+
</code_context>
<issue_to_address>
**问题 (bug_risk):** 当 `dataset_info.json` 中选定的数据集条目为 `null` 或其他非对象值时,`dataset_info.get(self.c.dataset, {}).get("file_name")` 会引发 `AttributeError`,而不是使用预期的 `ValueError` 报告无效的数据集配置。
**触发条件:** 自定义的 `dataset_info.json` 包含选定的数据集名称,但其对应的值是格式错误的非对象值。
**建议修复:** 在调用 `.get` 之前验证选定的条目是否为映射,并对格式错误的条目抛出相同的描述性 `ValueError`。
```suggestion
dataset_entry = dataset_info.get(self.c.dataset, {})
if not isinstance(dataset_entry, dict):
raise ValueError(f"Dataset '{self.c.dataset}' must define file_name in {dataset_info_path}")
file_name = dataset_entry.get("file_name")
```
</issue_to_address>帮助我变得更有用!请对每条评论点击 👍 或 👎,我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="weclone/data/qa_generator.py" line_range="704-705" />
<code_context>
+ with open(dataset_info_path, "r", encoding="utf-8") as f:
+ dataset_info = json.load(f)
+
+ file_name = dataset_info.get(self.c.dataset, {}).get("file_name")
+ if not file_name:
+ raise ValueError(f"Dataset '{self.c.dataset}' must define file_name in {dataset_info_path}")
+
</code_context>
<issue_to_address>
**issue (bug_risk):** When the selected dataset entry in `dataset_info.json` is `null` or another non-object value, `dataset_info.get(self.c.dataset, {}).get("file_name")` raises `AttributeError` instead of reporting the invalid dataset configuration with the intended `ValueError`.
**Triggers:** When a customized `dataset_info.json` contains the selected dataset name with a malformed non-object value.
**Suggested fix:** Validate that the selected entry is a mapping before calling `.get`, and raise the same descriptive `ValueError` for malformed entries.
```suggestion
dataset_entry = dataset_info.get(self.c.dataset, {})
if not isinstance(dataset_entry, dict):
raise ValueError(f"Dataset '{self.c.dataset}' must define file_name in {dataset_info_path}")
file_name = dataset_entry.get("file_name")
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Regression evidence
Before:
PYTHONDONTWRITEBYTECODE=1 uv run --no-sync pytest -q tests/test_qa_generator.py::test_save_result_updates_configured_dataset_fileexited1After:
PYTHONDONTWRITEBYTECODE=1 uv run --no-sync pytest -q tests/test_qa_generator.py::test_save_result_updates_configured_dataset_fileexited0Verification
PYTHONDONTWRITEBYTECODE=1 uv run --no-sync pytest -q tests/test_qa_generator.pyuv run --no-sync ruff check .uv run --no-sync ruff format --check .uv run --no-sync pyright tests/test_qa_generator.pyScope
Sourcery 摘要
通过配置的数据集映射保存生成的 QA 结果,以便下游处理使用当前数据。
错误修复:
测试:
Original summary in English
Summary by Sourcery
Save generated QA results through the configured dataset mapping so downstream processing uses the current data.
Bug Fixes:
Tests: