Story 2430: Library Subpage Integration#2524
Open
julhoang wants to merge 28 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds structured ChangesWebsite content pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
libraries/management/commands/load_website_adoc_demo.py (1)
46-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winChain the original exception when re-raising in
exceptblocks.Flagged by Ruff (B904). Minor for a dev-only CLI tool, but preserves the original traceback context for debugging.
🔧 Proposed fix
try: library = Library.objects.get(slug=library_slug) - except Library.DoesNotExist: - raise click.ClickException(f"No library with slug '{library_slug}'.") + except Library.DoesNotExist as err: + raise click.ClickException(f"No library with slug '{library_slug}'.") from errtry: library_version = LibraryVersion.objects.get(library=library, version=version) - except LibraryVersion.DoesNotExist: + except LibraryVersion.DoesNotExist as err: raise click.ClickException( f"'{library_slug}' has no LibraryVersion for {version.slug}." - ) + ) from errAlso applies to: 59-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/management/commands/load_website_adoc_demo.py` around lines 46 - 49, Chain the original exception when converting lookup failures into click errors. In the library lookup exception handler and the additional exception handler around the related lookup (the block also flagged at lines 59–64), add explicit exception chaining with “from” when raising click.ClickException, preserving the caught exception context.Source: Linters/SAST tools
versions/tasks.py (1)
31-31: 🚀 Performance & Scalability | 🔵 TrivialAdds another sequential network call per library in an already I/O-heavy import loop.
Functionally correct —
fetch_website_adoc_fieldsnever raises, so it's safe to inline here. Worth noting for a large catalog import: this stacks a third per-library HTTP round-trip (alongsideget_libraries_jsonandget_repo) onto a fully sequential loop, extending total import runtime and GitHub API pressure. Not blocking, just something to watch ifimport_library_versionsstarts timing out on large releases.Also applies to: 477-477
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@versions/tasks.py` at line 31, The import loop adds a third sequential HTTP request per library through fetch_website_adoc_fields, increasing runtime and API pressure. Update import_library_versions to avoid serializing this call for every library, preferably by batching or bounded concurrency while preserving the function’s non-raising behavior and existing data handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/githubhelper.py`:
- Around line 334-353: Add a finite timeout to the requests.get call in
get_website_adoc, and catch requests.exceptions.RequestException instead of only
HTTPError so connection and timeout failures return None consistently. Preserve
the existing informational logging for all request failures.
In `@libraries/godbolt.py`:
- Around line 39-64: Move the response-shape parsing loop in
_fetch_boost_version_map inside the existing try block so malformed JSON
structures are handled defensively. Broaden the exception handling to catch
parsing-related TypeError and AttributeError (or an appropriate safe exception
boundary), log the failure with logger.exception, and return {} for any
unexpected API payload without allowing page rendering to fail.
In `@libraries/migrations/0042_library_slack_url.py`:
- Around line 16-20: The `slack_url` URLField in migration
`0042_library_slack_url` is non-nullable without a database default, which can
fail on existing rows. Add `default=""` to the field definition alongside
`blank=True` so the migration populates existing records safely.
In `@libraries/mixins.py`:
- Around line 343-379: Update build_all_contributors so its library_versions
queryset always includes the selected library_version explicitly, in addition to
the applicable versions returned by Version.objects.minor_versions(). Ensure
patch, beta, and branch builds retain the selected release and avoid duplicate
LibraryVersion rows when combining the querysets.
In `@libraries/tasks.py`:
- Around line 93-106: In store_library_version_website_adoc, guard the
client.get_website_adoc call with exception handling so connection failures and
redirect errors are logged and processing continues to the next library. Update
get_website_adoc in core/githubhelper.py to pass an appropriate timeout to
requests.get, while preserving its existing HTTPError handling.
In `@libraries/website_adoc_template.adoc`:
- Line 127: Update the freeform section instruction in
website_adoc_template.adoc by changing “Markdown formatting is supported” to
“AsciiDoc formatting is supported,” ensuring the guidance matches the
convert_adoc_to_html rendering pipeline.
In `@libraries/website_adoc.py`:
- Around line 169-186: Update _freeform to track whether iteration is inside a
---- source/listing fence, and only skip //-prefixed lines while outside that
fenced block; preserve all lines verbatim, including // comments, while the
fence is active. Follow the fence-state handling used by _segment(), while
retaining the existing heading detection, placeholder validation, and content
assembly.
---
Nitpick comments:
In `@libraries/management/commands/load_website_adoc_demo.py`:
- Around line 46-49: Chain the original exception when converting lookup
failures into click errors. In the library lookup exception handler and the
additional exception handler around the related lookup (the block also flagged
at lines 59–64), add explicit exception chaining with “from” when raising
click.ClickException, preserving the caught exception context.
In `@versions/tasks.py`:
- Line 31: The import loop adds a third sequential HTTP request per library
through fetch_website_adoc_fields, increasing runtime and API pressure. Update
import_library_versions to avoid serializing this call for every library,
preferably by batching or bounded concurrency while preserving the function’s
non-raising behavior and existing data handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 47f1f402-fe53-4d5e-a632-a2846aff2ef9
📒 Files selected for processing (31)
config/celery.pycore/constants.pycore/githubhelper.pycore/views.pylibraries/admin.pylibraries/godbolt.pylibraries/management/commands/import_library_version_website_adoc.pylibraries/management/commands/load_website_adoc_demo.pylibraries/migrations/0042_library_slack_url.pylibraries/migrations/0043_libraryversion_website_adoc_and_more.pylibraries/mixins.pylibraries/models.pylibraries/tasks.pylibraries/tests/test_godbolt.pylibraries/tests/test_mixins.pylibraries/tests/test_models.pylibraries/tests/test_tasks.pylibraries/tests/test_views.pylibraries/tests/test_website_adoc.pylibraries/utils.pylibraries/views.pylibraries/website_adoc.pylibraries/website_adoc_demo.adoclibraries/website_adoc_template.adocstatic/css/v3/library-subpage.cssstatic/css/v3/stats.csstemplates/admin/libraryversion_change_list.htmltemplates/v3/examples/_v3_example_section.htmltemplates/v3/includes/_stats_benchmarks.htmltemplates/v3/libraries/library-subpage.htmlversions/tasks.py
💤 Files with no reviewable changes (1)
- templates/v3/includes/_stats_benchmarks.html
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/v3/includes/_hero_library.html (1)
11-11: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the
added_textdocs and remaining example callers.templates/v3/includes/_hero_library.html:11still documents the full"Added in 1.66.0"text even though the template prependsAdded initself, andtemplates/v3/examples/_v3_example_section.html:92-95still passes the full phrase, which will render asAdded in Added in 1.66.0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/v3/includes/_hero_library.html` at line 11, Update the `added_text` documentation in `_hero_library.html` to show only the version value, such as `1.66.0`, because the template adds the `Added in ` prefix. Update the remaining example invocation in `_v3_example_section.html` to pass only the version value so it does not render a duplicated prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@templates/v3/includes/_hero_library.html`:
- Line 11: Update the `added_text` documentation in `_hero_library.html` to show
only the version value, such as `1.66.0`, because the template adds the `Added
in ` prefix. Update the remaining example invocation in
`_v3_example_section.html` to pass only the version value so it does not render
a duplicated prefix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5348ae1a-438e-4b13-b55f-cf67e50d4dad
📒 Files selected for processing (11)
core/githubhelper.pylibraries/godbolt.pylibraries/management/commands/load_website_adoc_demo.pylibraries/migrations/0042_library_slack_url.pylibraries/mixins.pylibraries/models.pylibraries/tasks.pylibraries/tests/test_website_adoc.pylibraries/website_adoc.pylibraries/website_adoc_template.adoctemplates/v3/includes/_hero_library.html
🚧 Files skipped from review as they are similar to previous changes (10)
- libraries/migrations/0042_library_slack_url.py
- libraries/mixins.py
- core/githubhelper.py
- libraries/management/commands/load_website_adoc_demo.py
- libraries/tasks.py
- libraries/website_adoc_template.adoc
- libraries/models.py
- libraries/tests/test_website_adoc.py
- libraries/website_adoc.py
- libraries/godbolt.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/v3/libraries/library-subpage.html`:
- Line 17: Guard the empty-state message in the library subpage template against
a missing object.first_boost_version before accessing display_name. Render the
first-release sentence only when first_boost_version exists, while preserving
the no-version message for libraries without any LibraryVersion records.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ff518074-8b1a-4ba6-b273-faa0182fd1de
📒 Files selected for processing (6)
libraries/views.pylibraries/website_adoc_demo.adocstatic/css/v3/library-subpage.csstemplates/v3/examples/_v3_example_section.htmltemplates/v3/includes/_hero_library.htmltemplates/v3/libraries/library-subpage.html
✅ Files skipped from review due to trivial changes (1)
- libraries/website_adoc_demo.adoc
🚧 Files skipped from review as they are similar to previous changes (1)
- libraries/views.py
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
1 participant
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.
Issue: #2430
Summary & Context
Renders the library subpage from two sources: (1) data we already ingest (metadata, dependencies, contributors, long-form docs) and (2) a net-new, optional maintainer-authored
meta/website.adocin each library repo that supplies the marketing/quick-start content we don't otherwise collect. All dynamic sections render with graceful empty states.Changes
website.adocingestion pipeline (net-new)GithubAPIClient.get_website_adoc()to fetch a repo's optionalmeta/website.adocfrom raw GitHub, returningNonequietly on the common 404libraries/website_adoc.py— parser that maps stable AsciiDoc section IDs/attributes ([#about],[#designed-for],[#links],[#playground],[#install],[#benchmarks],[#freeform]) into structured fieldsLibraryVersion.website_adocplus the raw source, with per-section status editing in the Wagtail/Django admin (libraries/admin.py, migration0043)update_library_version_website_adoc(scoped to the most recent version, fetched frommaster); a missing file or transient fetch failure leaves any existing value untouched. Skips the refresh while a newer release is in beta —masterhas drifted toward that release, so the current stable keeps its release-tag snapshot until the beta is promotedimport_library_version_website_adocfor manual/backfill ingestionPage header
Library.slack_urladmin field with fallback tohttps://cppalliance.org/slack/(migration0042); surfaced on the hero and the “Discuss in Slack” redirectRender
website.adocsections with empty states[#links](:common-use-case-url:/:code-example-url:), each validated as a Boost-owned URL (boost.org/github.com/boostorg) or falling back to the documentation link[#benchmarks]sub-sections[#freeform]Compiler Explorer link (net-new)
libraries/godbolt.py— builds a godbolt.org/clientstate/<base64>“Edit in Compiler Explorer” URL from the[#playground]code, selecting the Boost version matching the active release dropdown (falling back to the newest version Compiler Explorer offers); the Boost version→id map is fetched from the CE libraries API and cached for a dayContributors & documentation
libraries.json+ commit contributors up to the selected version) with identity-based dedup byCommitAuthorid (never by display name)Demo & tooling
load_website_adoc_democommand,website_adoc_demo.adoc, andwebsite_adoc_template.adoc(maintainer-facing reference)Tests
test_godbolt.py,test_website_adoc.py, and extendtest_mixins.py/test_views.py/test_tasks.pycovering the parser, Compiler Explorer URL building, contributor dedup, view context, and the daily-refresh beta guardimport_library_version_website_adocis implemented but we cannot really test it until a library ships ameta/website.adocfile. The parser is unit-tested, but the full pipeline is untested until a maintainer adds the file to a library repo.mastervs release-tag drift: the daily refresh pulls the current stable'swebsite.adocfrommaster, which drifts toward the next release over the cycle — so the stable page tracks the freshest maintainer content rather than a frozen tag snapshot. This is intended (marketing/quick-start copy), but the worst case (a newer release in beta, wheremasteralready reflects that release) is guarded: the daily task skips the refresh while a newer beta is live. Older/archived versions are never refreshed and stay frozen at their release-tag import snapshot.website.adocfetch in the import loop:import_library_versions(versions/tasks.py) now callsfetch_website_adoc_fieldsper library, adding a third sequential GitHub round-trip alongsideget_libraries_jsonandget_repo. This runs in a background Celery task (not a user request path) and each call is guarded/non-raising, so it's non-blocking — but on large catalogs it extends total import runtime and GitHub API pressure. Left as-is for now; revisit with batching or bounded concurrency (e.g. aThreadPoolExecutorprefetch) only if the import starts timing out.Peer-Testing Guidelines
Setup
docker compose exec web python manage.py migratewebsite.adoccontent onto a library (swapbeastfor any slug):docker compose exec web python manage.py load_website_adoc_demo --library beastTest
slack_url(e.g.beast) and reload: the “Discuss in Slack” link should use it. Clear it and confirm it falls back to https://cppalliance.org/slack/.[#playground]code pre-filled and Boost selected. (Godbolt may take a few minutes to compile.)libraries.jsonauthors/maintainers and doesn't duplicate an author who is already linked.Optional
libraries/website_adoc_demo.adoc(comment out or delete sections), re-run the seed command from step 2, and reload to see how the layout adapts when sections are missing.docker compose exec web python manage.py load_website_adoc_demo --library beast --clearScreenshots
LibraryLibraryVersionSelf-review Checklist
Frontend
Summary by CodeRabbit
Summary by CodeRabbit