Skip to content

ENG-9414: Remove hardcoded docs urls#6395

Open
carlosabadia wants to merge 2 commits intomainfrom
carlos/bring-back-docs-links
Open

ENG-9414: Remove hardcoded docs urls#6395
carlosabadia wants to merge 2 commits intomainfrom
carlos/bring-back-docs-links

Conversation

@carlosabadia
Copy link
Copy Markdown
Contributor

No description provided.

@carlosabadia carlosabadia requested review from a team and Alek99 as code owners April 27, 2026 11:36
@linear
Copy link
Copy Markdown

linear Bot commented Apr 27, 2026

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 27, 2026

Merging this PR will not alter performance

✅ 9 untouched benchmarks


Comparing carlos/bring-back-docs-links (6869b0d) with main (44271c5)

Open in CodSpeed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

This PR replaces ~230 hardcoded /docs/... URL strings across 68 documentation markdown files with dynamic {namespace.path} expressions, backed by a new _resolve_link_target helper that uses re.sub + eval to expand brace expressions against the doc execution environment. The core rendering change converts _render_spans from a free function to an instance method so link resolution can reach self.env, and a fix in __init__.py ensures top-level page namespaces retain their .path attribute after being shadowed by subpage SimpleNamespace objects.

Confidence Score: 4/5

Safe to merge; changes are confined to internal documentation infrastructure with no runtime product impact.

Only P2 findings: a soft eval sandbox (mitigated by the fully repo-controlled input) and missing error context for NameError/AttributeError in the link resolver. No logic bugs or data-path issues identified.

docs/app/reflex_docs/docgen_pipeline.py — the _resolve_link_target eval call and error handling.

Security Review

  • eval with {"__builtins__": {}} in _resolve_link_target (docgen_pipeline.py) provides only a soft sandbox. Expressions come exclusively from repo-controlled markdown files today, but objects in env (e.g. the rx module) expose full Python object graphs that can be used to escape the restriction via attribute traversal. If inputs ever became user-supplied this would be a code-execution risk; for the current use-case it is a low-severity concern.

Important Files Changed

Filename Overview
docs/app/reflex_docs/docgen_pipeline.py Converted _render_spans from free function to instance method; added _resolve_link_target using regex + eval to expand {expr} hrefs; updated all call sites. Two P2 concerns: soft eval sandbox and missing error context for NameError/AttributeError.
docs/app/reflex_docs/pages/docs/init.py Copies .path attribute from top-level page objects onto the shadowed SimpleNamespace objects so markdown can reference {library.path} and {custom_components.path} correctly.
docs/app/tests/test_docgen_link_targets.py New test file covering plain URL passthrough, brace expr substitution, and anchor composition. No issues found.
docs/getting_started/basics.md All hardcoded /docs/... URLs replaced with {namespace.path} expressions; corresponding imports added to the python exec block.
docs/enterprise/overview.md All hardcoded enterprise component links replaced with dynamic path references; enterprise namespace imported in exec block.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Markdown source\n[text]({ns.attr.path})"] --> B["_exec_code\nbuilds env namespace"]
    B --> C["ReflexDocTransformer\n(holds self.env)"]
    C --> D["self._render_spans(spans)"]
    D --> E{LinkSpan?}
    E -- yes --> F["_resolve_link_target(target, self.env)"]
    F --> G["_BRACE_RE.sub(_sub, target)"]
    G --> H["eval(expr, {__builtins__:{}}, env)\nresolves ns.attr.path → str"]
    H --> I["doclink2(text, href=resolved)"]
    E -- no --> J["other span types\n(text, bold, code, img...)"]
    I --> K["rx.Component tree"]
    J --> K
Loading

Reviews (1): Last reviewed commit: "ENG-9414: Remove hardcoded docs urls" | Re-trigger Greptile

Comment thread docs/app/reflex_docs/docgen_pipeline.py Outdated
Comment thread docs/app/reflex_docs/docgen_pipeline.py
@adhami3310
Copy link
Copy Markdown
Member

i would prefer if we didn't, it makes less digestable to AI

@carlosabadia
Copy link
Copy Markdown
Contributor Author

i would prefer if we didn't, it makes less digestable to AI

that's true indeed

see this @Alek99

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