Skip to content

feat(allowlist): add Jinja template support - #1056

Open
dvd233 wants to merge 3 commits into
alibaba:mainfrom
dvd233:codex/feat/jinja-template-support
Open

feat(allowlist): add Jinja template support#1056
dvd233 wants to merge 3 commits into
alibaba:mainfrom
dvd233:codex/feat/jinja-template-support

Conversation

@dvd233

@dvd233 dvd233 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Description

Add first-class review support for Jinja template files as the focused subtask tracked in #1055:

  • recognize .jinja2 and .j2 files case-insensitively;
  • route them to a dedicated Jinja review rule covering escaping contexts, template injection, undefined values, includes/imports/macros, output correctness, and render-time performance;
  • test lowercase and uppercase allowlist and rule resolution;
  • document the new built-in mapping in all four review-rules locale pages.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • make test passes locally
  • Manual testing (described below)

Validation performed on Windows/amd64 with Go 1.27.0, GNU Make 4.4.1, and GCC 16.2.0:

  • make check
  • make test (all 23 Go packages with -race -count=1)
  • make build
  • go test -race -count=1 ./internal/config/allowlist ./internal/config/rules
  • Pages lint, 32 tests, typecheck, production webpack build, bundle-size check, and HTTP smoke checks for the built site

The repository-required ocr review --audience agent --background "briefly summarize the background requirements" command was attempted, but this local environment has no configured LLM endpoint. I completed a separate standards/spec self-review; both axes reported no remaining findings.

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly (if applicable)
  • I have signed the CLA

Related Issues

Closes #1055

Part of #470

@CLAassistant

CLAassistant commented Aug 25, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s).

Comment thread pages/src/content/docs/en/review-rules.md Outdated
@dvd233

dvd233 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@wu21-web The requested documentation wording has been shortened in 46d669c, and the review thread has been resolved. Could you please take another look when convenient? Thanks!

@Qiyuanqiii Qiyuanqiii left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description has been flattened into a single Markdown line, so its headings, lists, and checkboxes do not render correctly. Please edit the PR description to restore the intended line breaks and blank lines.

@Qiyuanqiii

Copy link
Copy Markdown
Contributor
image The problem appears like this

@dvd233

dvd233 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@Qiyuanqiii Fixed — the PR description has been restored with its headings, lists, blank lines, and task checkboxes rendering correctly. The CLA item remains checked. Thanks for catching this!

@Qiyuanqiii Qiyuanqiii left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub Linguist lists .jinja as Jinja’s primary extension. Since this PR is intentionally scoped to .jinja2 and .j2 per #1055, this does not need to block the PR, but it would be useful to track .jinja support in a follow-up issue.

@wu21-web wu21-web left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dvd233 Merge conflicts, please rebase main and I will review.

@dvd233
dvd233 force-pushed the codex/feat/jinja-template-support branch from 46d669c to 7b7b2e9 Compare August 25, 2026 13:31
@dvd233

dvd233 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@wu21-web Rebased onto the latest main (cfbb62e) and resolved the conflicts while preserving both the upstream MATLAB support and this PR's Jinja support. Local validation passed: make check, make test (with -race -count=1), make build, and the Pages lint/test/typecheck/build/size checks. The PR is now conflict-free. Could you please review again? Thanks!

@wu21-web wu21-web left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is complete. I would approve this.

Comment thread internal/config/allowlist/supported_file_types.json Outdated

@wu21-web wu21-web left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dvd233

dvd233 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

All review suggestions are addressed and checks are green — this is ready from my side whenever you'd like to merge. Thanks again for the thorough reviews!

dvd233 and others added 3 commits August 29, 2026 16:54
Co-authored-by: Tao Xin <149216116+wu21-web@users.noreply.github.com>
Co-authored-by: Tao Xin <149216116+wu21-web@users.noreply.github.com>
@dvd233
dvd233 force-pushed the codex/feat/jinja-template-support branch from 7054aa9 to 3736838 Compare August 29, 2026 09:13

@wu21-web wu21-web left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

feat(allowlist): add Jinja template support

4 participants