Skip to content

docs: add issue and pull request templates (#58) - #69

Open
mhaye9545 wants to merge 1 commit into
trueagi-io:mainfrom
mhaye9545:mhaye9545/issue-and-pr-templates
Open

docs: add issue and pull request templates (#58)#69
mhaye9545 wants to merge 1 commit into
trueagi-io:mainfrom
mhaye9545:mhaye9545/issue-and-pr-templates

Conversation

@mhaye9545

Copy link
Copy Markdown

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:
    • Prepend pointer to troubleshooting guide (docs/user_guides/16_troubleshooting.md).
    • Fields for FabricPC version, install extras checkboxes (cpu, cuda12, cuda13, tfds, viz, all, dev).
    • JAX environment diagnostic output (jax.print_environment_info(), jax.default_backend(), jax.devices()).
    • Flags and setup state (XLA_FLAGS, JAX_PLATFORMS, FABRICPC_SKIP_XLA_FLAGS, FABRICPC_DISABLE_TRITON_GEMM, setup_jax() call timing).
    • Key package versions list (pip list | grep -Ei "jax|nvidia|tensorflow|fabricpc").
    • NVIDIA GPU & driver line from nvidia-smi.
    • Minimal reproduction script with import order and expected vs observed behavior.
  • .github/ISSUE_TEMPLATE/feature_request.yml:
    • Fields for problem statement, proposed behavior, and alternatives considered.
  • .github/ISSUE_TEMPLATE/config.yml:
    • Disables blank issues (blank_issues_enabled: false) and links CONTRIBUTING.md.
  • .github/PULL_REQUEST_TEMPLATE.md:
    • Standard PR structure including linked issue, summary, design plan link, testing checklist, and divergence notes mirroring CONTRIBUTING.md.
  • docs/dev_plans_archive/issue_and_pr_templates_plan.md:
    • Committed design document for this change as required by CONTRIBUTING.md.

Design Document

Testing & Verification

  • YAML schemas validated using yaml.safe_load.
  • Links to troubleshooting guide and contributing guidelines verified.

@matthewbehrend matthewbehrend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 bf

Nothing 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-config

check-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 in pyproject.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; in description, reporters retype every name.
  • For the cuBLAS failure class that motivated the extras field (#45), the decisive diagnostics on Linux are echo $LD_LIBRARY_PATH and ldconfig -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 #58 looking like a PR number. Closes #58 in 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Name the command so reporters do not guess: python -c "import fabricpc; print(fabricpc.__version__)".

Comment on lines +25 to +31
- label: cpu
- label: cuda12
- label: cuda13
- label: tfds
- label: viz
- label: all
- label: dev

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment on lines +38 to +43
```python
import jax
jax.print_environment_info()
print("default_backend:", jax.default_backend())
print("devices:", jax.devices())
```

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment on lines +53 to +54
- `XLA_FLAGS`:
- `JAX_PLATFORMS`:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 -->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Once GitHub hides the comment this renders as a dangling Closes #. Keep the whole placeholder inside the comment:

Suggested change
Closes #<!-- Issue number if applicable -->
<!-- Closes #NNN (the issue this PR resolves; trivial fixes need none) -->


## What Changed

- <!-- Bullet points of changes -->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Renders as an empty bullet.

Suggested change
- <!-- 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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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:

Suggested change
- [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`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment on lines +31 to +32
- 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`).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

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.

Issue and pull request templates

2 participants