Skip to content

Less requests and some smaller fixes#174

Closed
jakobkogler wants to merge 6 commits into
nmcassa:mainfrom
jakobkogler:feature/less-requests-and-fixes
Closed

Less requests and some smaller fixes#174
jakobkogler wants to merge 6 commits into
nmcassa:mainfrom
jakobkogler:feature/less-requests-and-fixes

Conversation

@jakobkogler
Copy link
Copy Markdown
Contributor

@jakobkogler jakobkogler commented May 16, 2026

I'm digging through the codebase a bit, and noticed a couple of things that really bugged me.
So far I've fixed a couple of the most prominent things, but I'll write an issue with some improvement suggestions as well.

  • The movie details page (https://letterboxd.com/film/v-for-vendetta/details) is actually an exact duplicate of the non-detailed page (https://letterboxd.com/film/v-for-vendetta/). It loads exactly the same HTML, except that it makes sure that the appropriate tab is hidden (but the HTML of the hidden tabs is still there).
    So I removed the request for the details page, and moved the details logic to the profile page.
  • I would argue that 99% of the users don't care about the watch-stats of a movie. So it doesn't make sense to preload it whenever you load a movie. So I delayed the request so that it's only done when somebody wants those infos.
  • I noticed a bug in the list extractor. Pagination didn't work if you used a limit, because the stop condition was initialized wrong.
  • Parsing the release year didn't work (at least not without the JSON fallback).
  • Until now it didn't support Python 3.13 / 3.14
    image

@jakobkogler jakobkogler force-pushed the feature/less-requests-and-fixes branch 2 times, most recently from 862e173 to 759e3e3 Compare May 16, 2026 12:05
@jakobkogler jakobkogler force-pushed the feature/less-requests-and-fixes branch from 759e3e3 to c632484 Compare May 16, 2026 16:57
@jakobkogler jakobkogler force-pushed the feature/less-requests-and-fixes branch from c632484 to 758aeca Compare May 16, 2026 17:30
@fastfingertips
Copy link
Copy Markdown
Collaborator

I'll try to review it tomorrow.

@fastfingertips fastfingertips self-assigned this May 24, 2026
@jakobkogler jakobkogler mentioned this pull request May 29, 2026
@fastfingertips
Copy link
Copy Markdown
Collaborator

The original design behind movie_details.py followed a strict rule: one file per distinct URL endpoint in letterboxdpy/pages/ (e.g., movie_profile.py -> /film/slug/, movie_lists.py -> /film/slug/lists, etc.). When I first implemented this, I either missed the fact that /details and the main profile page return identical HTML, or perhaps Letterboxd behaved differently back then (I don't quite remember).

But you're absolutely right—we verified that they return the exact same HTML (line count and structure are identical). There's no point in wasting an extra network request when we can grab the details directly from the profile page's DOM. Your approach of moving the logic and deleting movie_details.py is the right call.

One small request though: Could you split these changes into separate, smaller PRs? I am trying to avoid bundling too many unrelated changes (the details optimization, the lists pagination bug fix, Python 3.13/3.14 support, etc.) into a single PR so we can review, test, and merge them more easily and safely.

Keeping the rest of the file-per-URL structure for truly unique pages, but this optimization is very much welcomed!

@jakobkogler
Copy link
Copy Markdown
Contributor Author

Closed in favor of #182, #181, #183, #184, #179, #180, #185.

@jakobkogler jakobkogler closed this Jun 2, 2026
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