🐛 fix: should record unique case id in eval dataset#13129
Conversation
|
@cy948 is attempting to deploy a commit to the LobeHub OSS Team on Vercel. A member of the Team first needs to authorize it. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR updates multiple dataset presets so that an optional Sequence diagram for dataset ingestion with optional case_id capturesequenceDiagram
actor User
participant UI as DatasetUploadUI
participant Importer as DatasetImporter
participant Presets as DatasetPresetConfig
participant Store as DatasetStorage
participant Tracker as CaseTrackingService
User->>UI: Upload dataset file
UI->>Importer: sendFile(file)
Importer->>Presets: getPreset(formatKey)
Presets-->>Importer: DatasetPreset(requiredFields, optionalFields)
Importer->>Importer: validate requiredFields
Importer->>Importer: map fields including optional case_id
alt row has case_id
Importer->>Store: saveRecord(input, expected, case_id)
Store-->>Importer: recordId
Importer->>Tracker: linkCase(recordId, case_id)
else row missing case_id
Importer->>Store: saveRecord(input, expected)
Store-->>Importer: recordId
end
Importer-->>UI: ImportResult(total, successes, failures)
Flow diagram for dataset presets recognizing optional case_idflowchart LR
A[Start dataset ingestion] --> B[Select dataset preset]
B --> C{Preset supports
case_id in
optionalFields?}
C -->|Yes| D[Check row
for case_id column]
C -->|No| G[Ignore case_id
in incoming rows]
D --> E{Row has
case_id value?}
E -->|Yes| F[Attach case_id
to internal record
for tracking]
E -->|No| H[Create record
without case_id]
G --> H
H --> I[Finish ingestion
with created records]
F --> I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- For presets where
idis now accepted, consider updating the correspondingformatDescriptionstrings and anyfieldInference/identifier mapping so the presence and role ofidis clear and consistently handled across all presets.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For presets where `id` is now accepted, consider updating the corresponding `formatDescription` strings and any `fieldInference`/identifier mapping so the presence and role of `id` is clear and consistently handled across all presets.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@ONLY-yours - This is a bug fix in the eval dataset presets config. Please take a look. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## canary #13129 +/- ##
==========================================
- Coverage 74.56% 74.56% -0.01%
==========================================
Files 1516 1516
Lines 124941 124941
Branches 14539 16569 +2030
==========================================
- Hits 93166 93164 -2
- Misses 31664 31666 +2
Partials 111 111
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
4a446ec to
8661642
Compare
1cd92ff to
04a8f89
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- If
case_idis meant to be consumed downstream (e.g., used as a primary identifier), consider adding it to any relevantfieldInferencemappings so it can be auto-detected even when column names vary (e.g.,id,caseId). - Double-check that all dataset presets that should support external case tracking now include
case_idinoptionalFieldsto avoid inconsistent behavior between different preset types.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If `case_id` is meant to be consumed downstream (e.g., used as a primary identifier), consider adding it to any relevant `fieldInference` mappings so it can be auto-detected even when column names vary (e.g., `id`, `caseId`).
- Double-check that all dataset presets that should support external case tracking now include `case_id` in `optionalFields` to avoid inconsistent behavior between different preset types.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
❤️ Great PR @cy948 ❤️ The growth of project is inseparable from user feedback and contribution, thanks for your contribution! If you are interesting with the lobehub developer community, please join our discord and then dm @arvinxx or @canisminor1990. They will invite you to our private developer channel. We are talking about the lobe-chat development or sharing ai newsletter around the world. |
💻 Change Type
🔗 Related Issue
🔀 Description of Change
补齐 id 用于外部套件与内部 case 追踪
🧪 How to Test
📸 Screenshots / Videos
📝 Additional Information
Summary by Sourcery
New Features: