Skip to content

Governance: enforce approved exceptions and align default-arg severity - #8225

Merged
mohanchen merged 1 commit into
deepmodeling:developfrom
Critsium-xy:fix/governance-exception-and-default-arg-severity
Oct 29, 2026
Merged

mohanchen merged 1 commit into
deepmodeling:developfrom
Critsium-xy:fix/governance-exception-and-default-arg-severity

Conversation

@Critsium-xy

@Critsium-xy Critsium-xy commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Reminder

  • I have read AGENTS.md and docs/developers_guide/agent_governance.md.
  • I have linked an issue or explained why this PR does not need one.
  • I have added adequate unit tests and/or case tests, or explained why not.
  • I have listed the exact verification commands run and their results.
  • I have described user-visible behavior changes, including INPUT parameter changes.
  • I have explained core-module impact for ESolver, HSolver, ElecState, Hamilt, Operator, Psi, or other source/ changes.
  • I have requested any needed governance exception below.

When I'm trying to update the goverance check, I found some problems with it so I made this PR to fix them first.

This change does two things:

  1. Fixes a doc mismatch. The docs said adding a default parameter would block the PR, but the checker has only ever warned. I changed the docs to say it warns, matching the code.
  2. Makes exceptions actually work. Before, the governance doc offered an exception template, but CI never read it, so any error still failed CI. Now:
    • The PR author fills in the ### Governance Exception section in the PR description.
    • A maintainer approves it by adding the governance-exception-approved label.
    • When both are present, the matching error drops to a warning and CI passes. Errors that allow no exception, such as CRLF line endings or #pragma once, still block.

- Checker: parse the `### Governance Exception` PR section and the
  `governance-exception-approved` label from the event payload. A complete,
  approved exception downgrades blocking findings whose rule is named in
  `Rule` to warnings; findings with allow_exception=False are never
  downgraded. Incomplete or unapproved exceptions emit a warning and keep
  the blocker.
- Workflow: re-run on labeled/unlabeled so applying the label takes effect.
- Docs: the rule matrix listed "New default parameter" as block while the
  checker (and its test) warn; align the doc to warn. Document the
  exception approval flow in agent_governance.md and AGENTS.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@ABACUS-PR-BOT ABACUS-PR-BOT 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.

Static review of 3dc028a1abec5930f476b149aa5f07d2d44cd939. The candidate checker/tests and workflows were not executed; CI diagnostics were unavailable.

Non-blocking: Please add the actual checker-regression command and observed result to the PR verification notes, or state which checks were not run and why. The checked reminder currently has no accompanying evidence; a follow-up documentation update is fine.

Comment thread tools/03_code_analysis/agent_governance_check.py
Comment thread tools/03_code_analysis/agent_governance_check.py
Comment thread tools/03_code_analysis/agent_governance_check.py
@mohanchen mohanchen added the Standards Coding standards and governance rules label Oct 29, 2026
@mohanchen
mohanchen merged commit 7ceda1d into deepmodeling:develop Oct 29, 2026
20 checks passed
mohanchen pushed a commit to mohanchen/abacus-mc that referenced this pull request Oct 29, 2026
Resolve conflicts with upstream/develop (PR deepmodeling#8225, deepmodeling#8214, deepmodeling#8230, deepmodeling#8223):

- write_dipole.cpp grid validation: take upstream version, which rejects
  only the inconsistent state (nplane == 0 && nrxx != 0) while allowing
  legitimate empty partitions; a superset of our nplane fix (13b21fb).
- write_dipole.cpp file write: keep our MY_RANK == 0 guard (fixes UB of
  writing to an unopened stream on non-root ranks) wrapping upstream's
  new setw/scientific output format.
- source_io/unittests/CMakeLists.txt: adopt upstream's test/ -> unittests/
  rename and test_<module>.cpp file naming (rule 11), while keeping our
  stru_frame_io.cpp link additions; read_set_globalv.cpp still calls
  stru_frame_io::extract_last_frame, so the dependency remains.

Verification: conflict markers cleared, no stale references to old test
file names, all SOURCES entries point to existing files; build not rerun
locally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Standards Coding standards and governance rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants