feat: add unified annotation system - #436
Conversation
|
✅ Preview is ready!
↩️ Previous: ⚡️ 🤖 Powered by surge-preview |
|||||||||||||||
There was a problem hiding this comment.
🟡 Changes recommended
Annotation normalization currently allows string values for channel: 'y' reference annotations, which can silently generate invalid numeric-axis marks without diagnostics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a unified, reusable Annotation system for GPT-Vis and wires it into the Line chart as the first adapted chart type, including initial test coverage for annotation compilation and layering.
Changes:
- Added a new
src/annotation/module (types, theme, normalization, and Cartesian compilation) to support reference lines/bands and data-targeted highlight/callout annotations. - Integrated annotation compilation into
src/vis/line/index.ts, inserting background annotations before data marks and foreground annotations after data marks, with diagnostics reporting. - Added Vitest coverage for annotation normalization/compilation and Line chart child-mark ordering with annotations.
File summaries
| File | Description |
|---|---|
| src/vis/line/index.ts | Accepts annotations in LineConfig and injects compiled annotation marks into the G2 children pipeline. |
| src/index.ts | Re-exports public annotation-related TypeScript types from the package entry point. |
| src/annotation/types.ts | Defines the unified annotation protocol (types + diagnostics) and AnnotatableConfig. |
| src/annotation/theme.ts | Adds tone-based and theme-aware visual tokens for annotations. |
| src/annotation/normalize.ts | Normalizes/validates annotation inputs and produces diagnostics for invalid entries. |
| src/annotation/index.ts | Barrel exports for annotation module APIs and types. |
| src/annotation/cartesian.ts | Compiles normalized Cartesian annotations into G2 mark definitions (background/foreground) and reports diagnostics. |
| tests/line.test.ts | Adds Line annotation rendering/ordering tests with a mocked G2 Chart. |
| tests/annotation.test.ts | Adds unit tests for annotation normalization and Cartesian compilation behavior. |
Review details
Suppressed comments (1)
src/annotation/normalize.ts:119
reference-bandvalidation also allows stringfrom/towhenchannel: 'y'. For numeric Y axes this can lead to invalidrangeYmarks without any diagnostic. Tighten the check so Y bands require finite numbers, while X bands can still be string/number as today.
const validValues =
isValidValue(annotation.from) &&
isValidValue(annotation.to) &&
hasValidOptionalLabel(annotation.label);
const matchingTypes = typeof annotation.from === typeof annotation.to;
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (annotation.type === 'reference-line') { | ||
| if ( | ||
| (annotation.channel !== 'x' && annotation.channel !== 'y') || | ||
| !isValidValue(annotation.value) || | ||
| !hasValidOptionalLabel(annotation.label) | ||
| ) { | ||
| diagnostics.push({ | ||
| code: 'INVALID_ANNOTATION_VALUE', | ||
| message: `Invalid reference line at index ${index}.`, | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| normalized.push(annotation as unknown as Annotation); | ||
| continue; | ||
| } |
|
@interstellarmt 有几种 annotation？分别长什么样子，目前看示例中的圆点的 ui 不好看 |
里程碑
新增标注类型
reference-linereference-bandhighlightcallout配置说明
type'reference-band'channel'y''x'时表示 X 轴区间from90to110from类型一致且大于起始值label'健康区间'tone'positive'neutral、info、warning、negative