feat: add canonical instrument identity and symbol resolution - #59
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Four moderate issues remain involving alias ambiguity, immutable identity generation, snapshot timestamp validation, and asset-type validation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a canonical instrument identity schema and deterministic resolver for US and HK instruments.
Changes:
- Supports canonical IDs, symbols, names, provider aliases, and ambiguity results.
- Adds versioned catalog snapshots and provider mappings.
- Adds resolver and snapshot tests with public exports.
File summaries
| File | Description |
|---|---|
packages/core/src/instrument.ts |
Canonical identity and resolution implementation |
packages/core/src/instrument.test.ts |
Resolver and snapshot coverage |
packages/core/src/index.ts |
Public export |
Review details
Suppressed comments (2)
packages/core/src/instrument.ts:214
snapshot()rejects negative or non-finiteupdatedAt, but this deserialization path only checksschemaVersion. A persisted snapshot with an invalid timestamp is therefore accepted, bypassing the public snapshot contract; validate the timestamp here as well and cover malformed snapshots.
static fromSnapshot(snapshot: InstrumentCatalogSnapshot): InstrumentResolver {
if (snapshot.schemaVersion !== INSTRUMENT_CATALOG_SCHEMA_VERSION) {
throw new Error(`Unsupported instrument catalog schema: ${snapshot.schemaVersion}`);
}
return new InstrumentResolver(snapshot.instruments);
packages/core/src/instrument.ts:121
assetTypeis required byCanonicalInstrumentbut is never checked by this runtime validator. BecausefromSnapshot()feeds persisted data through it, a missing or invalid value can be accepted and returned as a canonical instrument, violating the declared asset-type contract; add an allowed-value check and a regression test before indexing.
['name', instrument.name],
['market', instrument.market],
['country', instrument.country],
['currency', instrument.currency],
];
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /** Build a stable Folio id from an exchange MIC and its local symbol. */ | ||
| export function createInstrumentId(exchangeMic: string, symbol: string): string { | ||
| const mic = normalizedCode(exchangeMic); | ||
| const localSymbol = normalizedCode(symbol); | ||
| if (!mic || !localSymbol) throw new Error('Exchange MIC and symbol are required.'); |
| if (!providerId) { | ||
| const bySymbol = resolutionOf( | ||
| originalQuery, | ||
| 'symbol', | ||
| candidates.filter((instrument) => normalizedCode(instrument.symbol) === code) |
ae63baa to
52c8c11
Compare
52c8c11 to
471034c
Compare
AMBIGUOUS_INSTRUMENT 错误携带候选摘要,defineCapability 从 stamped data 写入 provenance.instrumentId。
helsome
left a comment
There was a problem hiding this comment.
复审通过。上一轮 Copilot 提到的 alias 冲突、snapshot updatedAt、assetType 校验问题已经在后续提交中补上并有回归测试;测试报告也已补齐 Bun/OS、focused tests、baseline 说明。instrumentId 目前按 listing identity 使用,首版 catalog 的 MIC:SYMBOL 作为稳定目录键可以接受,后续证券主数据平台再演进不需要卡本 PR。无 UI 变化。
改动说明
建立 Folio 自己的 canonical instrument identity / resolver,并接到 Longbridge 与 Massive 两个 provider adapter。用户输入 ticker / 公司名 / provider symbol 时先解析;唯一匹配才继续取数,歧义时返回候选而不是静默选错标的。行情、基本面、新闻以及 research evidence/report 通过
instrumentId关联。instrumentId(上市维度MIC:SYMBOL),不再用 ticker 当全局主键ambiguousAAPL.USvsAAPL)instrumentIdinstruments.json),空仓时写入默认真实标的目录关联 Issue
Closes #23
真实标的解析案例(验收要求 ≥5,含 1 个歧义 + 1 个跨 provider 不一致)
AAPL.USXNAS:AAPLAAPL+ providermassiveXNAS:AAPLAAPL,Longbridge 用AAPL.US0700.HK/TencentXHKG:0700TSLAXNAS:TSLABABA.USXNYS:BABAAlibabaXNYS:BABA+XHKG:9988市场 hint:
Alibaba+market=HK→XHKG:9988。测试报告(正式审核前必填)
环境
实际执行命令与结果
已知失败 / Baseline(如有)
AgentKernelHost > builds the kernel on the electron userData store:断言硬编码 POSIX/tmp/finagent-test/store,Windows 上path.join为\tmp\finagent-test\store。与本 PR 无关,可在当前 origin/main 复现(见 [DevEx] Fix unit test failures on Windows (POSIX path literals, symlink privilege, locale-dependent assertions) #68 / fix(tests): 修复 Windows 上的单元测试失败(路径断言/symlink 权限/locale 依赖) #70)。已提供实际测试命令与 pass/fail 结果
已说明测试环境
如果存在已知 baseline / 环境失败,已提供 main 对照或说明
核心改动已有对应 focused test / smoke / integration 验证
UI 截图(涉及 UI 时必填)
Scope / 后续
首版覆盖 US + HK 种子目录、两个 adapter、catalog 持久化与歧义上抛。完整证券主数据平台、运行时向 vendor 补全 ISIN 等不在范围内。