Skip to content

Add Discourse and Hacker News adapters (Phase 1) - #12

Merged
kaihuang1425 merged 6 commits into
mainfrom
claude/unmerged-prs-app-dev-gc6g23
Jul 15, 2026
Merged

Add Discourse and Hacker News adapters (Phase 1)#12
kaihuang1425 merged 6 commits into
mainfrom
claude/unmerged-prs-app-dev-gc6g23

Conversation

@kaihuang1425

@kaihuang1425 kaihuang1425 commented Jun 28, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

This PR completes the two remaining Phase 1 roadmap items: Discourse adapter and Hacker News adapter. PRs #1-11 are already merged into main, so this branch starts from the current main HEAD.

Both ship as hand-written extractors in packages/parser (extractThreadDiscourse, extractThreadHackerNews), matching the existing extractThreadGeneric shape. The Phase 2 JSON/adapter-sdk runtime described in docs/ADAPTERS.md does not exist yet, so building these as JSON/TypeScript adapters per that doc is not possible until Phase 2 lands. apps/extension/src/extract.ts now tries Hacker News, then Discourse, then falls back to the generic parser.

  • Discourse: reads article.topic-post, .cooked post bodies, .names .username for author. Resolves role: "op" from the topic-owner class (Discourse applies this to every post by the original poster, not just the first) and detects staff/moderator/admin badges.
  • Hacker News: reads the flat tr.athing.comtr row list, reconstructs parentId/depth from each row's indent attribute, and marks the story submitter's own comments with role: "op".
  • Factored resolveUrl/documentBaseUrl into packages/parser/src/url.ts and ExtractedThread/ExtractOptions into src/types.ts now that three extractors share them.
  • Updated README.md, ROADMAP.md, Initial Plan.md, docs/ADAPTERS.md, and the parser/extension package READMEs to reflect Phase 1 completion.

Type of change

  • Documentation
  • New or updated adapter
  • Bug fix
  • Feature
  • Tooling / build
  • Other:

Checklist

  • The change is focused (no unrelated features, broad refactors, or premature abstractions).
  • Relevant documentation is updated in this PR.
  • No new tracking, analytics, or hidden third-party network requests.
  • No user data is sent off-device by default; any AI feature stays opt-in.
  • Extension permissions are unchanged.
  • Adds adapters: both fail gracefully on missing fields (deleted/flagged HN comments, missing Discourse badges) and are tested against HTML fixtures, never live sites.
  • I verified my change, and I describe how below.

How was this verified?

  • pnpm -r typecheck: all 4 packages (core, storage, parser, extension) typecheck cleanly.
  • pnpm test: 152 tests passing across 16 test files, including 24 new tests for the two adapters (packages/parser/test/discourse.test.ts, packages/parser/test/hackernews.test.ts) and 2 new extraction-dispatch tests in apps/extension/test/extract.test.ts. All pre-existing tests pass unchanged.
  • pnpm --filter @forumforge/extension build: succeeds, producing dist/sidepanel.js, dist/content.js, dist/background.js with sourcemaps.
  • Not done: manual click-through against a live Discourse/Hacker News page in a real browser (requires a real browser environment); per docs/FIXTURES.md, adapter tests intentionally use synthetic HTML fixtures rather than live sites.

Notes for reviewers

No permission changes. No new network requests; both adapters operate on the already-loaded page DOM, same as the generic parser. The review-comments/merge-earliest-to-latest portion of the original task required no action in this branch because PRs #1-11 were already merged.

Implements the two remaining Phase 1 roadmap items. Both ship as
hand-written extractors in packages/parser (extractThreadDiscourse,
extractThreadHackerNews), same shape as extractThreadGeneric, since the
Phase 2 JSON/adapter-sdk runtime described in docs/ADAPTERS.md doesn't
exist yet. apps/extension/src/extract.ts now tries Hacker News, then
Discourse, then falls back to the generic parser.

- Discourse: reads article.topic-post, .cooked bodies, .names .username;
  resolves op role from topic-owner (applied to every post by the
  original poster, not just the first) and staff/moderator/admin badges.
- Hacker News: reads the flat tr.athing.comtr row list, reconstructs
  parentId/depth from each row's indent attribute, and marks the
  submitter's own comments with role "op".
- Factored resolveUrl/documentBaseUrl into packages/parser/src/url.ts and
  ExtractedThread/ExtractOptions into src/types.ts now that three
  extractors share them.
- Updated README.md, ROADMAP.md, Initial Plan.md, docs/ADAPTERS.md, and
  the parser/extension package READMEs to reflect Phase 1 completion.
- Added fixture-based tests for both adapters (24 new tests) plus
  extraction-dispatch tests in apps/extension; full suite verified at
  152 passing tests, clean typecheck across all 4 packages, and a clean
  extension build.

The unmerged-PR review/merge portion of this task was already completed
by prior sessions (PRs #1-11 all merged into main); this session
continues development per the roadmap as instructed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AozWWKRq4epEc52PanY6Vy

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8629c6fd29

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/parser/src/discourse.ts Outdated
Some Discourse themes render staff/mod/admin badges as icon-only elements
with no visible text (just a class like staff-admin), so checking only
textContent missed them — and if that user also happened to be the topic
owner, the post was labeled only "op" with no staff signal at all.
resolveRole() now also checks the matched badge element's className.

Adds a fixture post (class-only admin badge, no text) and a test
covering it. Addresses automated review feedback on PR #12.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AozWWKRq4epEc52PanY6Vy

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 609382c197

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/parser/src/hackernews.ts Outdated
Ask HN/Show HN/text submissions render their body as .toptext directly
above the comment rows, outside any tr.athing.comtr — so the comment-row
loop never picked it up, and the side panel dropped the question/
announcement entirely (most visible on a text post with zero comments,
where it showed "No posts found" for an otherwise valid thread).

extractThreadHackerNews now also reads .toptext (when present) and
prepends it as an OP-authored post, with id/author/timestamp/permalink
read from the same story/subtext markup already used for submitter
detection. Link-only stories have no .toptext, so they're unaffected.

Verified the real markup against a live Ask HN page before writing the
selectors (research only; the new test fixture is synthetic, per
docs/FIXTURES.md). Addresses automated review feedback on PR #12.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AozWWKRq4epEc52PanY6Vy

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4867d89262

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/parser/src/discourse.ts
Comment thread packages/parser/src/discourse.ts Outdated
article.topic-post matched zero elements on a real Discourse forum —
the topic-post/topic-owner classes and data-post-number live on the
outer wrapper div, while the nested article only carries
boxed/onscreen-post. Likewise .post-date never contains a <time
datetime> element; Discourse renders a .relative-date span with
data-time (epoch ms) and a title attribute instead. Verified both
against Discourse's own open-source frontend (post.gjs,
meta-data/date.gjs, lib/formatter.js).

Selects .topic-post directly, reads timestamps from
.post-date .relative-date, and updates the fixture to mirror the real
nested div > article structure.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AozWWKRq4epEc52PanY6Vy

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c6e843a87

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/parser/src/hackernews.ts Outdated
claude added 2 commits June 28, 2026 08:17
#hnmain is HN's page wrapper on every page, including listing pages
(front page, /new, /ask, ...) that this adapter can't read — it only
knows tr.athing.comtr and .toptext. Dispatching those pages here
produced a bogus empty thread instead of falling back to the generic
parser.

Verified via a live fetch of news.ycombinator.com that HN tags page
type on <html op="...">: only item (thread) pages get op="item";
listing pages get op="news"/"newest"/"ask"/"show". Gate on that
instead of #hnmain alone, and update fixtures to carry the real
attribute.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AozWWKRq4epEc52PanY6Vy
CI caught what the parser package's test run alone didn't: this
fixture's inline HTML lacked the op="item" attribute now required by
isHackerNewsPage, so it fell through to the generic parser instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AozWWKRq4epEc52PanY6Vy
@kaihuang1425
kaihuang1425 merged commit d4db453 into main Jul 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants