Skip to content

Commit a50074d

Browse files
Benjamin-eecsjackwener
Benjamin-eecs
and
jackwener
authored
fix(adapters): drop silent-sentinel row fallbacks across 6 read commands (#1631)
* fix(adapters): drop silent-sentinel row fallbacks across 6 read commands Continues the audit-baseline cleanup started in #1611 (lesswrong) and the direction set by #1599 / #1603 / #1604. Replaces the `silent-sentinel` row-data fallbacks (`'Unknown'` / `'-'` / `'unknown'` that mask missing fields) with the empty-string signal so agents can tell apart "field really has the value Unknown" from "upstream returned no value". Touched 6 read adapters, 10 baseline entries: - wikipedia/trending: title, description - 36kr/article: author, date, body - xiaoyuzhou/download: podcast - xiaoyuzhou/transcript: podcast - zhihu/collection: dedup key + type field (the empty prefix still produces a unique-per-content dedup key, just without the `unknown:` noise) - zhihu/download: author Intentionally skipped (line-by-line audited): - v2ex/me.js: `'Unknown'` is an in-band control-flow sentinel. Line 35 initialises `let username = 'Unknown';`, line 41 uses `if (username === 'Unknown')` to trigger the profileEl fallback selector, line 75 uses the same check to raise the auth error. Empty would silently bypass both checks and return a row with an empty username as if auth succeeded. - v2ex/daily.js: `'未知'` is user-facing 签到 success text in the rendered status message, not a row field. Empty would render a broken sentence. - weibo/comments.js, weibo/feed.js: the sentinel sits inside an in-IIFE error-message string composition (`'API error: ' + (data.msg || 'unknown')`), not in a returned row. Empty would silently truncate diagnostic output. Both stay on baseline. Verified live: `opencli wikipedia trending --limit 3` and `opencli 36kr hot --limit 2` both return populated rows; the empty-string signal only kicks in when the upstream value is actually missing. * test(adapters): add empty-signal coverage for the cluster-2 sentinel swap Per owner's pattern in 7164615 (douyin/user-videos.test.js + jike/read.test.js + weread/search-regression.test.js), pairs the silent-sentinel value swap in this PR with focused unit tests that mock the upstream to return null / missing fields and assert the row surfaces an empty-string signal instead of the old fabricated 'Unknown' / '-' / 'unknown' sentinel. Coverage: - clis/wikipedia/trending.test.js (new): mocks wikiFetch to return three articles - one with both title + description populated, one with no title and no description, one with title only. Asserts the missing fields render as '' (was '-' before this PR). - clis/36kr/article.test.js (new): mocks page.evaluate to return a scrape where title is present but author / date / body are empty. Asserts those three fields render as '' in the row pair output (was '-' before this PR). Also covers the NOT_FOUND and INVALID_ARGUMENT error paths that already existed. - clis/zhihu/collection.test.js (+1 case): mocks the zhihu collection API to return an item with content.id but no content.type. Asserts type renders as '' (was 'unknown' before this PR); the new dedup key prefix is :id rather than unknown:id, semantically identical for dedup purposes. The other three files in this PR (xiaoyuzhou/download, xiaoyuzhou/transcript, zhihu/download) use the same `|| 'unknown'` -> `|| ''` value swap with no downstream sentinel consumer. They are covered by the same JS language semantics the three tests above demonstrate. * fix(adapters): fail typed on missing row identity * fix(adapters): tighten sentinel row identity guards --------- Co-authored-by: jackwener <jakevingoo@gmail.com>
1 parent 368581e commit a50074d

10 files changed

Lines changed: 180 additions & 91 deletions

File tree

‎clis/36kr/article.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,15 @@ cli({
5252
if (!data?.title) {
5353
throw new CliError('NOT_FOUND', 'Article not found or failed to load', 'Check the article ID');
5454
}
55+
if (!data.body) {
56+
throw new CliError('PARSE_ERROR', 'Article body not found', '36kr page loaded but no article body paragraphs were extracted');
57+
}
5558
return [
5659
{ field: 'title', value: data.title },
57-
{ field: 'author', value: data.author || '-' },
58-
{ field: 'date', value: data.date || '-' },
60+
{ field: 'author', value: data.author || '' },
61+
{ field: 'date', value: data.date || '' },
5962
{ field: 'url', value: `https://36kr.com/p/${articleId}` },
60-
{ field: 'body', value: data.body || '-' },
63+
{ field: 'body', value: data.body || '' },
6164
];
6265
},
6366
});

‎clis/36kr/article.test.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
import { getRegistry } from '@jackwener/opencli/registry';
3+
import { CliError } from '@jackwener/opencli/errors';
4+
import './article.js';
5+
6+
function makePage(evaluateResult) {
7+
return {
8+
installInterceptor: vi.fn().mockResolvedValue(undefined),
9+
goto: vi.fn().mockResolvedValue(undefined),
10+
wait: vi.fn().mockResolvedValue(undefined),
11+
evaluate: vi.fn().mockResolvedValue(evaluateResult),
12+
};
13+
}
14+
15+
describe('36kr article', () => {
16+
it('emits empty-string for missing optional author / date instead of a sentinel', async () => {
17+
const command = getRegistry().get('36kr/article');
18+
expect(command?.func).toBeDefined();
19+
const page = makePage({ title: 'Real Title', author: '', date: '', body: 'Real article body' });
20+
const rows = await command.func(page, { id: '1234567' });
21+
const byField = Object.fromEntries(rows.map((r) => [r.field, r.value]));
22+
expect(byField.title).toBe('Real Title');
23+
expect(byField.author).toBe('');
24+
expect(byField.date).toBe('');
25+
expect(byField.body).toBe('Real article body');
26+
expect(byField.url).toBe('https://36kr.com/p/1234567');
27+
});
28+
29+
it('throws CliError NOT_FOUND when the page exposes no title', async () => {
30+
const command = getRegistry().get('36kr/article');
31+
const page = makePage({ title: '', author: 'x', date: 'y', body: 'z' });
32+
await expect(command.func(page, { id: '1234567' })).rejects.toBeInstanceOf(CliError);
33+
});
34+
35+
it('throws CliError PARSE_ERROR when the page exposes title but no body', async () => {
36+
const command = getRegistry().get('36kr/article');
37+
const page = makePage({ title: 'Real Title', author: 'x', date: 'y', body: '' });
38+
await expect(command.func(page, { id: '1234567' })).rejects.toMatchObject({ code: 'PARSE_ERROR' });
39+
});
40+
41+
it('throws CliError INVALID_ARGUMENT when no numeric id can be parsed', async () => {
42+
const command = getRegistry().get('36kr/article');
43+
const page = makePage({});
44+
await expect(command.func(page, { id: 'not-a-url' })).rejects.toBeInstanceOf(CliError);
45+
});
46+
});

‎clis/wikipedia/trending.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,14 @@ cli({
2626
const articles = data?.mostread?.articles;
2727
if (!articles?.length)
2828
throw new CliError('NOT_FOUND', 'No trending articles available', 'Try a different language with --lang');
29-
return articles.slice(0, limit).map((a, i) => ({
29+
const selectedArticles = articles.slice(0, limit);
30+
if (selectedArticles.some((article) => !String(article?.title || '').trim())) {
31+
throw new CliError('PARSE_ERROR', 'Wikipedia trending returned an article without title', 'Trending rows require a title so they can be opened with wikipedia page.');
32+
}
33+
return selectedArticles.map((a, i) => ({
3034
rank: i + 1,
31-
title: a.title ?? '-',
32-
description: (a.description ?? '-').slice(0, DESC_MAX_LEN),
35+
title: a.title,
36+
description: (a.description ?? '').slice(0, DESC_MAX_LEN),
3337
views: a.views ?? 0,
3438
}));
3539
},

‎clis/wikipedia/trending.test.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { getRegistry } from '@jackwener/opencli/registry';
3+
4+
const { wikiFetchMock } = vi.hoisted(() => ({ wikiFetchMock: vi.fn() }));
5+
vi.mock('./utils.js', async () => {
6+
const actual = await vi.importActual('./utils.js');
7+
return { ...actual, wikiFetch: wikiFetchMock };
8+
});
9+
10+
import './trending.js';
11+
12+
describe('wikipedia trending', () => {
13+
beforeEach(() => {
14+
wikiFetchMock.mockReset();
15+
});
16+
17+
it('emits empty-string for missing description instead of a sentinel', async () => {
18+
const command = getRegistry().get('wikipedia/trending');
19+
expect(command?.func).toBeDefined();
20+
wikiFetchMock.mockResolvedValueOnce({
21+
mostread: {
22+
articles: [
23+
{ title: 'Has_Both', description: 'A real description', views: 100 },
24+
{ title: 'Has_Title_Only', views: 25 },
25+
],
26+
},
27+
});
28+
const rows = await command.func({ limit: 5, lang: 'en' });
29+
expect(rows).toHaveLength(2);
30+
expect(rows[0]).toMatchObject({ title: 'Has_Both', description: 'A real description', views: 100 });
31+
expect(rows[1].title).toBe('Has_Title_Only');
32+
expect(rows[1].description).toBe('');
33+
});
34+
35+
it('fails typed when a trending article is missing title identity', async () => {
36+
const command = getRegistry().get('wikipedia/trending');
37+
wikiFetchMock.mockResolvedValueOnce({
38+
mostread: { articles: [{ views: 50 }] },
39+
});
40+
await expect(command.func({ limit: 5, lang: 'en' })).rejects.toMatchObject({ code: 'PARSE_ERROR' });
41+
});
42+
43+
it('validates only rows selected by --limit', async () => {
44+
const command = getRegistry().get('wikipedia/trending');
45+
wikiFetchMock.mockResolvedValueOnce({
46+
mostread: {
47+
articles: [
48+
{ title: 'Selected', views: 100 },
49+
{ views: 50 },
50+
],
51+
},
52+
});
53+
await expect(command.func({ limit: 1, lang: 'en' })).resolves.toEqual([
54+
{ rank: 1, title: 'Selected', description: '', views: 100 },
55+
]);
56+
});
57+
});

‎clis/xiaoyuzhou/download.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ cli({
4545
});
4646
return [{
4747
title,
48-
podcast: ep.podcast?.title || '-',
48+
podcast: ep.podcast?.title || '',
4949
status: result.success ? 'success' : 'failed',
5050
size: result.success ? formatBytes(result.size) : (result.error || 'unknown error'),
5151
file: result.success ? destPath : '-',

‎clis/xiaoyuzhou/transcript.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ cli({
6767
}
6868
return [{
6969
title: episode.title || 'episode',
70-
podcast: episode.podcast?.title || '-',
70+
podcast: episode.podcast?.title || '',
7171
status: 'success',
7272
segments: kwargs.text === false ? '-' : String(segmentCount),
7373
json_file: kwargs.json === false ? '-' : jsonPath,

‎clis/zhihu/collection.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,18 @@ async function fetchCollectionPage(page, collectionId, offset, limit) {
5656

5757
function itemKey(item) {
5858
const content = item?.content || {};
59-
return `${content.type || 'unknown'}:${content.id || content.url || JSON.stringify(content).slice(0, 80)}`;
59+
return `${content.type || ''}:${content.id || content.url || JSON.stringify(content).slice(0, 80)}`;
6060
}
6161

6262
function mapCollectionItem(item, rank) {
6363
const content = item.content || {};
64-
const type = content.type || 'unknown';
64+
const type = content.type || '';
65+
if (!['answer', 'article', 'pin'].includes(type)) {
66+
throw new CommandExecutionError(
67+
`Zhihu collection returned unsupported content type: ${type || 'missing'}`,
68+
'Collection items require a supported content.type so the row identity, title, and URL are not silently blank.',
69+
);
70+
}
6571

6672
let title = '';
6773
let excerpt = '';
@@ -90,6 +96,13 @@ function mapCollectionItem(item, rank) {
9096
votes = content.reaction_count || 0;
9197
}
9298

99+
if (!String(title || '').trim() || !String(url || '').trim() || url.includes('undefined')) {
100+
throw new CommandExecutionError(
101+
'Zhihu collection returned a malformed item without title or URL identity',
102+
'Collection item rows require type, title, and URL so malformed payloads do not become blank listing rows.',
103+
);
104+
}
105+
93106
return {
94107
rank,
95108
type,

‎clis/zhihu/collection.test.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,4 +287,50 @@ describe('zhihu collection', () => {
287287
await expect(cmd.func(page, { id: '83283292', offset: 0, limit: 20 }))
288288
.rejects.toBeInstanceOf(EmptyResultError);
289289
});
290+
291+
it('fails typed for missing content.type instead of emitting a blank identity row', async () => {
292+
const cmd = getRegistry().get('zhihu/collection');
293+
const evaluate = vi.fn().mockResolvedValue({
294+
data: [
295+
{
296+
content: {
297+
id: 555,
298+
question: { id: 666, title: 'No-type Question' },
299+
author: { name: 'a' },
300+
voteup_count: 1,
301+
content: '<p>x</p>',
302+
url: 'https://www.zhihu.com/question/666',
303+
},
304+
},
305+
],
306+
paging: { totals: 1 },
307+
});
308+
const page = { goto: vi.fn().mockResolvedValue(undefined), evaluate };
309+
await expect(cmd.func(page, { id: '83283292', offset: 0, limit: 20 }))
310+
.rejects.toBeInstanceOf(CommandExecutionError);
311+
});
312+
313+
it('fails typed for supported collection items missing title/url identity', async () => {
314+
const cmd = getRegistry().get('zhihu/collection');
315+
const page = {
316+
goto: vi.fn().mockResolvedValue(undefined),
317+
evaluate: vi.fn().mockResolvedValue({
318+
data: [
319+
{
320+
content: {
321+
type: 'answer',
322+
id: 555,
323+
question: { id: 666, title: '' },
324+
author: { name: 'a' },
325+
voteup_count: 1,
326+
content: '<p>x</p>',
327+
},
328+
},
329+
],
330+
paging: { totals: 1 },
331+
}),
332+
};
333+
await expect(cmd.func(page, { id: '83283292', offset: 0, limit: 20 }))
334+
.rejects.toBeInstanceOf(CommandExecutionError);
335+
});
290336
});

‎clis/zhihu/download.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ cli({
4141
4242
// Get author
4343
const authorEl = document.querySelector('.AuthorInfo-name, .UserLink-link');
44-
result.author = authorEl?.textContent?.trim() || 'unknown';
44+
result.author = authorEl?.textContent?.trim() || '';
4545
4646
// Get publish time
4747
const timeEl = document.querySelector('.ContentItem-time, .Post-Time');

‎scripts/typed-error-lint-baseline.json‎

Lines changed: 0 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -855,30 +855,6 @@
855855
"text": "const limit = Math.max(1, Math.min(Number(args.limit) || 10, 25));",
856856
"occurrence": 0
857857
},
858-
{
859-
"rule": "silent-sentinel",
860-
"command": "36kr/article",
861-
"file": "clis/36kr/article.js",
862-
"line": 57,
863-
"text": "{ field: 'author', value: data.author || '-' },",
864-
"occurrence": 0
865-
},
866-
{
867-
"rule": "silent-sentinel",
868-
"command": "36kr/article",
869-
"file": "clis/36kr/article.js",
870-
"line": 60,
871-
"text": "{ field: 'body', value: data.body || '-' },",
872-
"occurrence": 0
873-
},
874-
{
875-
"rule": "silent-sentinel",
876-
"command": "36kr/article",
877-
"file": "clis/36kr/article.js",
878-
"line": 58,
879-
"text": "{ field: 'date', value: data.date || '-' },",
880-
"occurrence": 0
881-
},
882858
{
883859
"rule": "silent-sentinel",
884860
"command": "apple-podcasts/search",
@@ -1263,22 +1239,6 @@
12631239
"text": "if (!data.ok) return { error: 'API error: ' + (data.msg || 'unknown') };",
12641240
"occurrence": 0
12651241
},
1266-
{
1267-
"rule": "silent-sentinel",
1268-
"command": "wikipedia/trending",
1269-
"file": "clis/wikipedia/trending.js",
1270-
"line": 32,
1271-
"text": "description: (a.description ?? '-').slice(0, DESC_MAX_LEN),",
1272-
"occurrence": 0
1273-
},
1274-
{
1275-
"rule": "silent-sentinel",
1276-
"command": "wikipedia/trending",
1277-
"file": "clis/wikipedia/trending.js",
1278-
"line": 31,
1279-
"text": "title: a.title ?? '-',",
1280-
"occurrence": 0
1281-
},
12821242
{
12831243
"rule": "silent-sentinel",
12841244
"command": "xiaohongshu/download",
@@ -1287,22 +1247,6 @@
12871247
"text": "result.author = authorEl?.textContent?.trim() || 'unknown';",
12881248
"occurrence": 0
12891249
},
1290-
{
1291-
"rule": "silent-sentinel",
1292-
"command": "xiaoyuzhou/download",
1293-
"file": "clis/xiaoyuzhou/download.js",
1294-
"line": 48,
1295-
"text": "podcast: ep.podcast?.title || '-',",
1296-
"occurrence": 0
1297-
},
1298-
{
1299-
"rule": "silent-sentinel",
1300-
"command": "xiaoyuzhou/transcript",
1301-
"file": "clis/xiaoyuzhou/transcript.js",
1302-
"line": 70,
1303-
"text": "podcast: episode.podcast?.title || '-',",
1304-
"occurrence": 0
1305-
},
13061250
{
13071251
"rule": "silent-sentinel",
13081252
"command": "yollomi/edit",
@@ -1351,30 +1295,6 @@
13511295
"text": "return [{ status: 'saved', file: path.relative('.', fp), size: fmtBytes(size), credits: credits ?? '-', url: videoUrl }];",
13521296
"occurrence": 0
13531297
},
1354-
{
1355-
"rule": "silent-sentinel",
1356-
"command": "zhihu/collection",
1357-
"file": "clis/zhihu/collection.js",
1358-
"line": 64,
1359-
"text": "const type = content.type || 'unknown';",
1360-
"occurrence": 0
1361-
},
1362-
{
1363-
"rule": "silent-sentinel",
1364-
"command": "zhihu/collection",
1365-
"file": "clis/zhihu/collection.js",
1366-
"line": 59,
1367-
"text": "return `${content.type || 'unknown'}:${content.id || content.url || JSON.stringify(content).slice(0, 80)}`;",
1368-
"occurrence": 0
1369-
},
1370-
{
1371-
"rule": "silent-sentinel",
1372-
"command": "zhihu/download",
1373-
"file": "clis/zhihu/download.js",
1374-
"line": 44,
1375-
"text": "result.author = authorEl?.textContent?.trim() || 'unknown';",
1376-
"occurrence": 0
1377-
},
13781298
{
13791299
"rule": "silent-sentinel",
13801300
"command": "zsxq/dynamics",

0 commit comments

Comments
 (0)