Skip to content

Accuracy review: typing contract, docstrings, README, examples, and docs site #13

Description

@mikeckennedy

Context

With the new Great Docs documentation site and the package now shipping a py.typed marker, the type annotations and docs became a public contract. A thorough multi-agent accuracy review (types, docstrings, README, example apps, rendered site) confirmed 14 issues; 12 other candidate findings were investigated and refuted.

Confirmed and fixed (landing in the next commit)

Typing (py.typed contract)

  • Five public register_* functions missing -> None — consumers silently got Any from e.g. x = register_environment(env)
  • Union[Markup, str] on render_partial collapses to str under mypy (Markup is a str subtype), making Markup-specific calls like .unescape() false errors — fixed with @overloads keyed on markup (Markup by default, str for markup=False); mirrored on generate_render_partial

Docstrings

  • Starlette no-app fallback claimed it "uses the global executor" — it actually renders directly for sync environments (the Starlette default) and only uses the shared executor for enable_async=True
  • FastAPI/Starlette "properly shut down when the app stops" overstated — shutdown only happens when the lifespan exits normally, one startup/shutdown cycle per registration
  • PartialsJinjaExtension: environment parameter undocumented; added note that the extension path bypasses flask.render_template, so Flask context processors do not apply inside partials (verified by execution)

README

  • Same Flask context-processor caveat for the declarative extension approach
  • "Each example demonstrates the same UI" — only the Flask example matches the screenshot
  • Python >= 3.10 requirement never stated (older Pythons silently pip-install a pre-async release)
  • video_image.html snippet used ' '.join(classes) but no caller passes classes — crashes under StrictUndefined; now classes | default([]) with corrected prose

Examples

  • Quart example used register_environment instead of the documented register_quart_extensions
  • Flask example Option 1/Option 2 comments said the opposite of what the code does
  • Quart/FastAPI/Starlette requirements.piptools omitted jinja_partials (and jinja2), so the examples couldn't run from their own requirements; lockfiles recompiled to 0.3.1

Rendered docs site

  • Smart typography leaked into code spans (Callable[…, Any] with a Unicode ellipsis, curly-quoted literals) — copied text was invalid Python
  • Class Usage signature rendered a misleading no-arg constructor PartialsJinjaExtension()

Open follow-ups

  • Executor lifecycle re-entrancy: the FastAPI/Starlette executor is created at registration time and shut down after the first lifespan cycle — a second startup of the same app object (e.g. entering TestClient twice) fails with "cannot schedule new futures after shutdown". Fix: create the executor inside the lifespan wrapper and shut it down in a try/finally, looking it up via app.state in the renderer.
  • ty diagnostics: 11 pre-existing diagnostics from uvx ty check — 4 from the optional-import module = None pattern, 6 from jinja2's unannotated Environment.globals (ty infers the value type from DEFAULT_NAMESPACE), 1 unused-blanket-ignore warning on a comment pyrefly needs. Decide whether to suppress (checker-specific ignores) or leave. pyrefly reports 0 errors.

Verification

pytest 17/17, ruff clean, griffe docstring parse clean, pyrefly 0 errors, docs site rebuilt with all post-process fix counts as expected.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions