fix(facebook/feed): obtain the home feed landmark instead of re-navigating - #2
Merged
lawmight merged 1 commit intoSep 5, 2026
Conversation
…ating Live smoke after the anti-bleed gate: path=/, feedFound=no, messengerDom=yes, exit 1. Three causes in the surface loop, all fixed here: - The loop re-ran goto on every attempt (4x ~5s). Facebook home under the Shifter proxy hydrates slower than that, and every re-navigation reset it. Navigate once, then poll the same document for up to 30s (bounded by poll count too, so mocked/broken timers cannot spin). - messengerDom matched the top-nav Messenger icon present on every FB page, so it was a false positive. Chat chrome now means docked chats, chat sidebar, and thread panes outside role=banner/navigation. - Readiness required role=feed only. Modern home variants (see jackwener#2089) mark posts with data-pagelet=FeedUnit_n / aria-posinset / per-post action menus. Accept those as landmarks; on home with no landmark at all still refuse. Also: globally close docked chat windows and dialog Close buttons (never accept buttons), nudge the SPA via the banner Home control once after 9s, scroll the window (not just role=feed) so lazy posts stream, and emit structural diagnostics (landmark, counts, pagelet names, visibilityState, dialogs) in the failure hint for the next round. Co-authored-by: Tom Coustols <tom.coustols@tcdynamics.fr>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThe PR replaces repeated Facebook feed re-navigation with bounded same-document polling and targeted SPA recovery, introduces shared structural landmark/chat classification for modern home layouts, preserves extraction safety against Messenger bleed, improves lazy loading and diagnostics, and adds comprehensive regression coverage. Sequence diagram for Facebook feed surface acquisitionsequenceDiagram
participant FeedCommand
participant BrowserPage
participant FacebookHome
FeedCommand->>BrowserPage: goto(feedNavigationUrl)
loop Poll until ready or timeout
FeedCommand->>BrowserPage: evaluate(buildPrepareFeedScript)
BrowserPage->>FacebookHome: Close docked chats and dialog Close controls
alt Messages route
BrowserPage->>FacebookHome: location.assign(FACEBOOK_HOME)
FeedCommand->>BrowserPage: goto(feedNavigationUrl) once
else No landmark after threshold
BrowserPage->>FacebookHome: Click Home control once
end
FeedCommand->>BrowserPage: evaluate(buildSurfaceCheckScript)
BrowserPage-->>FeedCommand: landmark or diagnostics
alt landmark found
FeedCommand-->>FeedCommand: Continue to extraction
else Not ready
FeedCommand->>BrowserPage: sleep(pollMs)
end
end
Flow diagram for Facebook feed landmark classificationflowchart TD
A[Inspect Facebook DOM] --> B{role=feed outside chat chrome?}
B -->|Yes| C[landmark = role-feed]
B -->|No| D{FeedUnit or aria-posinset outside chat chrome?}
D -->|Yes| E[landmark = feed-units]
D -->|No| F{Post action menu in feed column?}
F -->|Yes| G[landmark = post-menus]
F -->|No| H[landmark = null]
C --> I[Surface ready]
E --> I
G --> I
H --> J[Home extraction returns no_feed]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="clis/facebook/feed.js" line_range="92-93" />
<code_context>
+ return document.querySelector('[role="main"]');
+ }
+ function feedLandmarkEl() {
+ return document.querySelector('[role="main"] [role="feed"]') || document.querySelector('[role="feed"]');
+ }
+ function postMenuEls() {
+ const main = mainEl();
</code_context>
<issue_to_address>
**issue (bug_risk):** `feedLandmarkEl` accepts any `[role="feed"]` in the document, including a feed-like region nested under `role=banner`, `navigation`, or `complementary`. That region makes the surface ready and becomes the extraction root, so navigation/sidebar content is treated as the news feed.
**Triggers:** When Facebook renders a role-feed-like region outside the main feed column.
**Suggested fix:** Require the matched element to be outside `OUTSIDE_FEED_ROLES` and, preferably, contained by `[role="main"]` before accepting it as a landmark.
```suggestion
const feeds = Array.from(document.querySelectorAll('[role="feed"]'));
return feeds.find((el) => !isOutsideFeedColumn(el) && el.closest('[role="main"]'))
|| feeds.find((el) => !isOutsideFeedColumn(el));
}
```
</issue_to_address>
### Comment 2
<location path="clis/facebook/feed.js" line_range="37-39" />
<code_context>
- }
- // Top-level feed posts should carry an author or engagement metrics. Authorless
- // metric-less rows are extraction garbage, commonly Messenger/thread bleed.
+ // Top-level feed posts carry an author or engagement metrics. Authorless,
+ // metric-less rows are extraction garbage — typically docked-chat bleed.
return true;
}
</code_context>
<issue_to_address>
**issue (broader_impact):** Every nonempty row whose author is not extracted and whose three engagement fields are `'-'` is classified as Messenger bleed and discarded or causes the command to fail. A legitimate feed post with an unrecognized author markup and no visible engagement counts therefore cannot be returned, even when its content came from a validated feed landmark.
**Triggers:** When a real post has no engagement metrics and `findAuthor` cannot recognize its author markup.
**Suggested fix:** Retain the stronger Messenger-specific text/container evidence from the old check, or only classify an authorless metric-less row as bleed when its content also matches chat markers or its DOM container is chat chrome.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: clis/facebook/feed.js:93, clis/facebook/feed.js:39
Comment on lines
+92
to
+93
| return document.querySelector('[role="main"] [role="feed"]') || document.querySelector('[role="feed"]'); | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): feedLandmarkEl accepts any [role="feed"] in the document, including a feed-like region nested under role=banner, navigation, or complementary. That region makes the surface ready and becomes the extraction root, so navigation/sidebar content is treated as the news feed.
Triggers: When Facebook renders a role-feed-like region outside the main feed column.
Suggested fix: Require the matched element to be outside OUTSIDE_FEED_ROLES and, preferably, contained by [role="main"] before accepting it as a landmark.
Suggested change
| return document.querySelector('[role="main"] [role="feed"]') || document.querySelector('[role="feed"]'); | |
| } | |
| const feeds = Array.from(document.querySelectorAll('[role="feed"]')); | |
| return feeds.find((el) => !isOutsideFeedColumn(el) && el.closest('[role="main"]')) | |
| || feeds.find((el) => !isOutsideFeedColumn(el)); | |
| } |
Comment on lines
+37
to
39
| // Top-level feed posts carry an author or engagement metrics. Authorless, | ||
| // metric-less rows are extraction garbage — typically docked-chat bleed. | ||
| return true; |
There was a problem hiding this comment.
issue (broader_impact): Every nonempty row whose author is not extracted and whose three engagement fields are '-' is classified as Messenger bleed and discarded or causes the command to fail. A legitimate feed post with an unrecognized author markup and no visible engagement counts therefore cannot be returned, even when its content came from a validated feed landmark.
Triggers: When a real post has no engagement metrics and findAuthor cannot recognize its author markup.
Suggested fix: Retain the stronger Messenger-specific text/container evidence from the old check, or only classify an authorless metric-less row as bleed when its content also matches chat markers or its DOM container is chat chrome.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #1. After that merge, live smoke on
tom.coustolscorrectly refused the Messenger bleed but never obtained a feed landmark:Reading the surface loop against those values, three defects explain it:
ensureNewsFeedSurfacerangotoon every attempt (4× with ~5s settle). Facebook home under the Shifter PAC proxy hydrates slower than that, so the feed column could never appear before the nextgotothrew it away.messengerDom=yeswas a false positive. The check matched[aria-label*="Messenger"]/[aria-label*="Chat"], which hits the top-nav Messenger icon present on every facebook.com page. It did not prove embedded chat.role=feedonly. This repo's own [autofix] facebook/feed: COMMAND_EXEC jackwener/OpenCLI#2089 fix documents that modern FB home variants drop thearticle/feedwrappers. Those variants mark posts withdata-pagelet="FeedUnit_n",aria-posinset, and per-post "Actions for this post" menus. A strictrole=feedrequirement can never succeed there.Related issue:
facebook feedMessenger bleed / no[role=feed]on home (jackwener#2453 stack)Type of Change
What changed
Surface acquisition (
ensureNewsFeedSurface)/messagesroute.href="/"fallback) to nudge the SPA.Close chat/Fermer la discussion/关闭聊天) and presses dialogs' own Close button. Never touches accept/allow buttons.Landmark detection (shared by probe + extractor via
FEED_DOM_HELPERS)landmark∈role-feed|feed-units(data-pagelet^=FeedUnit,aria-posinset) |post-menus|nullConversation with …panes,ChatTab/Messengerpagelets — excludingrole=banner/navigation. Anything in banner/nav/complementary is never a post source.no_feed). No regression to exit 0 with blank-author chat rows;rowLooksLikeMessengerBleedpost-filter kept.Lazy-load scroll
role=feedis not a scroll container — the previous commit had removed this).Diagnostics for the next round
Failure hint now reports:
landmark,roleFeed,feedUnits,postMenus,articles,chatChrome,mainTextLength,visibility,readyState,dialogs, and de-numberedpagelets(e.g.FeedUnit_n,LeftRail,ChatTab). These are structural only — no user content.Tests
npm test -- clis/facebook/feed.test.js→ 42 passed (all 5 Facebook suites: 102 passed). New cases:feed-unitshydrates (assertsgotocalled exactly once)/messagesroutemessengerDom=false; docked chat ⇒ flagged but does not block readinessrole=feed; menus inside chat chrome ignoredClose chat+ dialogCloseonly, neverAllow all cookieslandmark=none,visibility=hidden,pagelets=…,chatChrome=…Checklist
tom.coustolsafter mergeExpected live outcomes and next diagnostic
pagelets=…FeedUnit_n…butlandmark=nonevisibility=hidden--window foreground(or setdefaultWindowMode: 'foreground'on the command).dialogs=[…]mainTextLengthsmall,readyState=completebrowser snapshotofrole=main.Summary by Sourcery
Fix Facebook feed acquisition and extraction so slowly hydrated home feeds are detected without re-navigation or Messenger content bleed.
Bug Fixes:
Enhancements:
Tests:
Summary by cubic
Fixes the facebook feed surface loop so it reliably obtains the home feed landmark, instead of re-navigating every ~5s and resetting Facebook's slow hydration under the proxy. This addresses the Messenger bleed / no
[role=feed]on home issue (jackwener#2453 stack).Bug Fixes
/messagesroutes.role=feed,FeedUnitpagelets,aria-posinsetposts, or per-post action menus as the landmark, matching modern home variants (see [autofix] facebook/feed: COMMAND_EXEC jackwener/OpenCLI#2089).clis/facebook/feed.test.js(102 across all 5 Facebook suites); extraction still refuses on home with no landmark.Written for commit 5b8a6b9. Summary will update on new commits.