docs: add issue and pull request templates (#58) - #69
Conversation
matthewbehrend
left a comment
There was a problem hiding this comment.
Thanks for taking this on. The PR follows the #58 checklist item by item; the three forms parse, field ids are unique, the render languages are valid, and the bug and enhancement labels exist in the repo. Three defects have to be fixed before merge and four more should go in the same pass. The inline comments carry line-level detail and suggestions; this summary carries the reasoning.
The plan file is straightforward enough it can be removed from version control and copied into PR comments.
Must fix
1. Every file begins with a UTF-8 byte-order mark
All five files start with the bytes EF BB BF, an artifact of some Windows editors (Notepad, PowerShell Set-Content). No other file in the repository has one. PyYAML strips the mark before parsing, so a yaml.safe_load check cannot see it. GitHub inserts PULL_REQUEST_TEMPLATE.md verbatim into each new PR body, which would then begin with U+FEFF before ## Summary; CommonMark does not treat U+FEFF as whitespace, so the heading is not guaranteed to render as one.
Strip it and confirm:
for f in .github/ISSUE_TEMPLATE/*.yml .github/PULL_REQUEST_TEMPLATE.md \
docs/dev_plans_archive/issue_and_pr_templates_plan.md; do
sed -i '1s/^\xEF\xBB\xBF//' "$f"
done
head -c 3 .github/ISSUE_TEMPLATE/bug_report.yml | xxd # must not start with ef bb bfNothing in CI inspects .yml or .md files today; the lint workflow runs only ruff and black. Please add these hooks to .pre-commit-config.yaml in this PR so the defect cannot recur, then run pre-commit run --all-files:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: <current release>
hooks:
- id: fix-byte-order-marker
- id: check-yaml
- repo: https://github.com/python-jsonschema/check-jsonschema
rev: <current release>
hooks:
- id: check-github-issue-forms
- id: check-github-issue-configcheck-github-issue-forms validates .github/ISSUE_TEMPLATE/*.yml against GitHub's issue-form schema. That is the check the design document describes, and it is not what yaml.safe_load performs.
2. The "XLA flag profile" field is ahead of the feature
Issue #58 says this field is added only after the profiles issue ships. #50 is still open and nothing in fabricpc/ defines a profile, so reporters would be asked about something they cannot have used. Remove the line; the PR that closes #50 adds it back.
3. The design document's testing claims
The Testing & Verification section says the YAML schema was validated and template rendering was checked. GitHub renders issue forms only from a repository's default branch, and the fork's main has no .github/ISSUE_TEMPLATE/, so the forms have not been rendered. yaml.safe_load checks YAML syntax, not GitHub's issue-form schema. CONTRIBUTING.md holds authors accountable for every claim in a design document, so please replace the section with what was actually run, then do the real checks:
- Push this branch to the fork's default branch and open its New issue chooser: two forms, no blank-issue option, the contact link resolves.
- Open a draft PR on the fork: the body is pre-filled, headings render, no dangling
Closes #, no live link with a comment as its href. - Run the schema hooks from item 1.
Update the document if the checks change anything; CONTRIBUTING.md requires it to reflect the final implementation.
Should fix
4. Route questions to GitHub Discussions and update CONTRIBUTING.md to match
config.yml tells users to ask in Discussions but links the Questions section of CONTRIBUTING.md, which says to ask on an issue or open a new one. With blank_issues_enabled: false, a question has no form. Decision for this PR: point the contact link at https://github.com/trueagi-io/FabricPC/discussions (Discussions are enabled on the repo), and revise the Questions section at the end of CONTRIBUTING.md to send questions to Discussions. Reflect the change in the design document's Issue Config bullet.
5. PR template placeholders render as broken content
Closes #<!-- ... --> leaves a dangling Closes # once GitHub hides the comment. - <!-- ... --> renders an empty bullet. [Design Plan](<!-- docs/dev_plans_archive/... -->) renders as a live link: CommonMark parses <...> inside the parentheses as a pointy-bracket link destination, so the href is !-- docs/dev_plans_archive/... --. Inline suggestions replace each with a comment that stays invisible until filled in.
6. Two diagnostic fields duplicate the first, and the first has a timing hazard
jax.print_environment_info() prints every JAX_* and XLA_* environment variable and the full nvidia-smi output. This holds at the installed jax 0.10.2 and at the declared 0.7.0 floor (checked against the jax-v0.7.0 tag). The separate requests for XLA_FLAGS and JAX_PLATFORMS, and the nvidia-smi field, repeat what the first field captures. Keep the nvidia-smi and pip list fields, because they are the only diagnostics available when import jax itself fails, and say so in their descriptions. Trim the environment-variables field to the two FABRICPC_* variables plus the setup_jax() timing question.
The hazard: print_environment_info() calls jax.devices(), which initializes the JAX backend. setup_jax() binds platform, XLA flags, and memory settings at backend initialization, so a reporter who runs the diagnostic in the same interpreter before the reproduction changes the behavior the form is asking about. Tell reporters to run it in a separate interpreter.
7. Tracebacks land in a textarea without render
"Expected vs Observed Behavior" asks for the full traceback but has no render, so an unfenced traceback is interpreted as markdown (__init__ becomes bold, indented lines become code blocks). Add a separate traceback textarea with render: shell and keep this field for prose.
Minor
- Install extras: add
experiments(defined inpyproject.toml) and a "none (base install from PyPI)" option. - FabricPC Version: name the command,
python -c "import fabricpc; print(fabricpc.__version__)". - Put the environment-variables skeleton in
value:so it pre-fills the textarea; indescription, reporters retype every name. - For the cuBLAS failure class that motivated the extras field (#45), the decisive diagnostics on Linux are
echo $LD_LIBRARY_PATHandldconfig -p | grep -E 'cublas|cudnn'. Consider adding them to the GPU field. - Drop
(#58)from the title and commit subject. Squash-merge appends the PR number, so the commit would read... (#58) (#69)with#58looking like a PR number.Closes #58in the body links the issue.
What was verified for this review
Raw bytes of every file at the PR head commit; field ids, render values, and labels against the repository; every field against the #58 checklist; jax.print_environment_info source at jax 0.10.2 and at the jax-v0.7.0 tag; the fork's default branch contents.
| @@ -0,0 +1,91 @@ | |||
| name: Bug Report | |||
There was a problem hiding this comment.
Byte-order mark. This file and the other four in the PR begin with the bytes EF BB BF (UTF-8 BOM), an artifact of some Windows editors. No other file in the repository carries one, and PyYAML strips it before parsing, so yaml.safe_load cannot detect it. Strip it from all five files (command in the review summary, item 1) and add the pre-commit hooks named there so lint catches it next time.
| id: version | ||
| attributes: | ||
| label: FabricPC Version | ||
| description: What version of FabricPC are you using? (e.g. `0.5.0`, or git commit SHA) |
There was a problem hiding this comment.
Name the command so reporters do not guess: python -c "import fabricpc; print(fabricpc.__version__)".
| - label: cpu | ||
| - label: cuda12 | ||
| - label: cuda13 | ||
| - label: tfds | ||
| - label: viz | ||
| - label: all | ||
| - label: dev |
There was a problem hiding this comment.
Add experiments (defined in pyproject.toml) and a none (base install from PyPI) option, so a report from a plain pip install fabricpc is distinguishable from one that skipped the checkboxes.
| ```python | ||
| import jax | ||
| jax.print_environment_info() | ||
| print("default_backend:", jax.default_backend()) | ||
| print("devices:", jax.devices()) | ||
| ``` |
There was a problem hiding this comment.
Add one sentence: run this in a separate interpreter from the reproduction. jax.print_environment_info() calls jax.devices(), which initializes the JAX backend. setup_jax() binds platform, XLA flags, and memory settings at backend initialization, so running the diagnostic first in the same process changes the behavior the next field asks about.
| - `XLA_FLAGS`: | ||
| - `JAX_PLATFORMS`: |
There was a problem hiding this comment.
jax.print_environment_info() (field above) already prints every JAX_* and XLA_* environment variable, at the declared jax 0.7.0 floor as well as current releases. These two lines duplicate it. Keep the two FABRICPC_* lines and the setup_jax() question. Also consider moving this skeleton from description into value: so it pre-fills the textarea; as written, reporters retype every name.
|
|
||
| <!-- Provide a brief explanation of what this pull request does and why. --> | ||
|
|
||
| Closes #<!-- Issue number if applicable --> |
There was a problem hiding this comment.
Once GitHub hides the comment this renders as a dangling Closes #. Keep the whole placeholder inside the comment:
| Closes #<!-- Issue number if applicable --> | |
| <!-- Closes #NNN (the issue this PR resolves; trivial fixes need none) --> |
|
|
||
| ## What Changed | ||
|
|
||
| - <!-- Bullet points of changes --> |
There was a problem hiding this comment.
Renders as an empty bullet.
| - <!-- Bullet points of changes --> | |
| <!-- One bullet per change --> |
| ## Design Document | ||
|
|
||
| <!-- If this is a non-trivial change, link the committed design document in docs/dev_plans_archive/ --> | ||
| - [Design Plan](<!-- docs/dev_plans_archive/... -->) (if applicable) |
There was a problem hiding this comment.
CommonMark parses <...> inside the parentheses as a pointy-bracket link destination, so this renders as a live link whose href is !-- docs/dev_plans_archive/... --. Line 13 already explains the requirement; leave an example the author uncomments:
| - [Design Plan](<!-- docs/dev_plans_archive/... -->) (if applicable) | |
| <!-- - [Design document](docs/dev_plans_archive/<name>.md) --> |
| 2. **GitHub Issue Form for Feature Requests (`.github/ISSUE_TEMPLATE/feature_request.yml`)**: | ||
| - Collects problem statement, proposed behavior, alternatives considered, and links to `CONTRIBUTING.md`. | ||
| 3. **Issue Config (`.github/ISSUE_TEMPLATE/config.yml`)**: | ||
| - Disables blank issues (`blank_issues_enabled: false`) and redirects questions to discussions and `CONTRIBUTING.md`. |
There was a problem hiding this comment.
Update to match the change requested on config.yml: the contact link goes to GitHub Discussions and the Questions section of CONTRIBUTING.md is revised to point there. CONTRIBUTING.md requires the design document to reflect the final implementation.
| - Validated YAML schema syntax of issue templates and config. | ||
| - Checked template rendering and links against existing documentation (`docs/user_guides/16_troubleshooting.md` and `CONTRIBUTING.md`). |
There was a problem hiding this comment.
These two claims do not hold as written. GitHub renders issue forms only from a repository's default branch; the fork's main has no .github/ISSUE_TEMPLATE/, so the forms have not been rendered. yaml.safe_load checks YAML syntax, not GitHub's issue-form schema, and it silently strips the BOM. Replace with what was actually run, then do the real checks: push this branch to the fork's default branch and open its New issue chooser (two forms, no blank-issue option, contact link resolves); open a draft PR on the fork to see the pre-filled body; run pre-commit run --all-files with the check-github-issue-forms and check-github-issue-config hooks from the summary.
Closes #58.
Summary
Adds GitHub issue form templates and a pull request template to establish clear diagnostic requirements up front and enforce the repository's design-first contributing workflow.
What Changed
.github/ISSUE_TEMPLATE/bug_report.yml:docs/user_guides/16_troubleshooting.md).cpu,cuda12,cuda13,tfds,viz,all,dev).jax.print_environment_info(),jax.default_backend(),jax.devices()).XLA_FLAGS,JAX_PLATFORMS,FABRICPC_SKIP_XLA_FLAGS,FABRICPC_DISABLE_TRITON_GEMM,setup_jax()call timing).pip list | grep -Ei "jax|nvidia|tensorflow|fabricpc").nvidia-smi..github/ISSUE_TEMPLATE/feature_request.yml:.github/ISSUE_TEMPLATE/config.yml:blank_issues_enabled: false) and linksCONTRIBUTING.md..github/PULL_REQUEST_TEMPLATE.md:CONTRIBUTING.md.docs/dev_plans_archive/issue_and_pr_templates_plan.md:CONTRIBUTING.md.Design Document
Testing & Verification
yaml.safe_load.