fix(sinafinance): match stock symbol in addition to name#1158
fix(sinafinance): match stock symbol in addition to name#1158jackwener merged 3 commits intojackwener:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the sinafinance stock adapter’s best-match selection so user searches can match on ticker symbol as well as the Chinese display name, fixing incorrect results when searching by symbol (e.g., AAPL).
Changes:
- Extend the scoring function to consider
e.symbolin addition toe.name. - Prioritize exact symbol/name matches and add partial symbol matching before falling back to name-only matching.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (s.includes(needle)) | ||
| return needle.length / s.length; | ||
| if (n.includes(needle)) | ||
| return needle.length / n.length; | ||
| return 0; |
There was a problem hiding this comment.
The scoring logic returns the symbol-based ratio as soon as s.includes(needle) is true, even when e.name would be a closer/better match (e.g., a short exact-ish display name but a longer symbol containing the needle). This can under-score an entry and change ranking unexpectedly. Consider computing symbolScore and nameScore separately and returning the better score (or compare a tuple like [isExactSymbol, hasSymbolMatch, score] to preserve symbol priority without penalizing better name matches).
| if (s.includes(needle)) | |
| return needle.length / s.length; | |
| if (n.includes(needle)) | |
| return needle.length / n.length; | |
| return 0; | |
| const symbolScore = s.includes(needle) ? needle.length / s.length : 0; | |
| const nameScore = n.includes(needle) ? needle.length / n.length : 0; | |
| return Math.max(symbolScore, nameScore); |
| // Pick best match: score by name/symbol similarity, tiebreak by market priority | ||
| const needle = key.toLowerCase(); | ||
| const score = (e) => { | ||
| const n = e.name.toLowerCase(); | ||
| if (n === needle) | ||
| const s = e.symbol.toLowerCase(); | ||
| if (s === needle || n === needle) | ||
| return 1; | ||
| if (s.includes(needle)) | ||
| return needle.length / s.length; |
There was a problem hiding this comment.
This change fixes a regression-prone ranking behavior, but there isn’t a test exercising sinafinance stock symbol-vs-name matching (e.g., the AAPL vs AAPLU case from #1157). Consider adding an e2e regression test that runs opencli sinafinance stock AAPL -f json and asserts the returned Symbol is AAPL, with the same “expected Chinese site restriction” skip logic used elsewhere in the e2e suite.
The scoring function only compared user input against the Chinese display name (p[4] from suggest API), so searching "AAPL" matched "AAPLU" (score 0.8) over Apple Inc. whose name field is "苹果" (score 0). Check the symbol field first for exact and partial matches. Fixes jackwener#1157
Benjamin-eecs
force-pushed
the
fix/sinafinance-stock-match
branch
from
5eb7231 to
4938e85
Compare
The index table only listed `news` for sinafinance. Added the other three commands (`rolling-news`, `stock`, `stock-rank`) and updated the mode from Public to hybrid since rolling-news and stock-rank require a browser. Fixes jackwener#1156
Description
The scoring function in
sinafinance stockonly compared user input against the Chinese display name (p[4]from suggest API), ignoring the symbol field (p[3]). This caused searches by ticker symbol to mismatch when the Chinese name doesn't contain the ticker.For example, searching "AAPL" scored Apple Inc. at 0 (name="苹果" doesn't contain "aapl") while AAPLU scored 0.8 (name="AAPLU" contains "aapl"), returning wrong data.
The fix adds symbol matching with higher priority than name matching, so exact symbol matches (e.g. "aapl" === "aapl") score 1.0 and partial symbol matches rank above name-only matches.
Fixes #1157
Type of Change
Checklist
Documentation (if adding/modifying an adapter)
N/A
Screenshots / Output
Before:
After:
Other markets verified no regression: