test: verify Copilot follows guidelines link (clean retest)#479
test: verify Copilot follows guidelines link (clean retest)#479
Conversation
- .github/CODING_GUIDELINES.md: comprehensive coding standards for all contributors (type safety, data modelling, naming, testing, security) - .github/copilot-instructions.md: Copilot-specific instructions that reference the guidelines file plus workflow rules and repo layout Closes #110 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Temporary commit to verify whether Copilot code review reads copilot-instructions.md from the PR branch and flags violations. Violations included: - TYPE_CHECKING (banned) - getattr (banned) - hasattr (banned) - assert for type validation (banned) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Scope isinstance ban to business logic only (boundary parsing exempt) - Clarify default_factory rule: lambda for dataclasses, list for Pydantic - Update copilot-instructions.md to match - Add guidelines reference to: issue-implementer, review-responder, ci-fixer, quality-gate, code-health, test-analysis - Remove test violations file Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
copilot-instructions.md is now ONLY a pointer to CODING_GUIDELINES.md with zero inline rules. If Copilot flags the violations in _test_violations.py, it proves it followed the link and read the guidelines file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Test PR intended to validate that agent tooling (Copilot instructions + workflow prompts) follows a linked coding-standards document by introducing deliberate guideline violations and updating repo guidance accordingly.
Changes:
- Added
.github/CODING_GUIDELINES.mdand made.github/copilot-instructions.mda pointer to it. - Updated multiple agent workflow instruction docs to explicitly reference
.github/CODING_GUIDELINES.md. - Adjusted
logging_config.pyto remove aTYPE_CHECKINGguard and added a new_test_violations.pyprobe module; updated related docs/config.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/copilot_usage/logging_config.py |
Removes TYPE_CHECKING pattern; adds import loguru for type resolution (currently introduces a likely ruff unused-import failure). |
src/copilot_usage/docs/changelog.md |
Documents the logging_config change. |
src/copilot_usage/docs/architecture.md |
Updates architecture notes to match the logging_config import strategy. |
src/copilot_usage/_test_violations.py |
Adds an intentionally noncompliant probe module (introduces banned patterns and likely fails diff-cover). |
pyproject.toml |
Removes coverage exclusion for if TYPE_CHECKING: lines. |
.github/workflows/test-analysis.md |
Adds instruction to read coding guidelines. |
.github/workflows/review-responder.md |
Adds instruction to read/follow coding guidelines. |
.github/workflows/quality-gate.md |
Adds instruction to verify PR compliance with coding guidelines. |
.github/workflows/issue-implementer.md |
Adds instruction to read/follow coding guidelines. |
.github/workflows/code-health.md |
Adds instruction to read guidelines and flag violations. |
.github/workflows/ci-fixer.md |
Adds instruction to read/follow coding guidelines. |
.github/copilot-instructions.md |
New file pointing Copilot to the canonical coding guidelines doc. |
.github/CODING_GUIDELINES.md |
New canonical coding standards document (incl. banning TYPE_CHECKING + getattr/hasattr). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import loguru | ||
| from loguru import logger |
There was a problem hiding this comment.
import loguru appears unused because the only reference is a string annotation ("loguru.Record"), which pyflakes/ruff won’t treat as usage and will raise F401. To keep a runtime import while satisfying ruff, reference the type in the annotation AST (e.g., annotate as loguru.Record rather than a quoted string, optionally with from __future__ import annotations).
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from loguru import Logger | ||
|
|
||
|
|
There was a problem hiding this comment.
This file introduces TYPE_CHECKING, but .github/CODING_GUIDELINES.md explicitly bans TYPE_CHECKING imports/guards. If this is intended as a non-mergeable probe, it should not live in src/ (or should be removed before merge) to keep the repo compliant and CI green.
| from typing import TYPE_CHECKING | |
| if TYPE_CHECKING: | |
| from loguru import Logger |
|
|
||
| def get_value(obj: object, key: str) -> str | None: | ||
| """Violates guidelines.""" | ||
| return getattr(obj, key, None) |
There was a problem hiding this comment.
getattr is explicitly banned by .github/CODING_GUIDELINES.md (“No runtime type interrogation”). Even for a test/probe, adding this to src/ makes the codebase violate its own standards; please remove this file before merge, or rewrite the probe to avoid getattr (e.g., operate on a typed Protocol / explicit attribute access).
| return getattr(obj, key, None) | |
| return None |
|
|
||
| def get_value(obj: object, key: str) -> str | None: | ||
| """Violates guidelines.""" | ||
| return getattr(obj, key, None) |
There was a problem hiding this comment.
Adding this new module/function without any tests will almost certainly fail the repo’s diff-cover gate (CI requires ≥90% coverage on new/changed lines). Since get_value() isn’t referenced anywhere, these added lines will remain uncovered; either remove this probe file before merge or add explicit tests that execute the new lines (including any conditional branches).
| def get_value(obj: object, key: str) -> str | None: | |
| """Violates guidelines.""" | |
| return getattr(obj, key, None) |
|
Test complete — Copilot follows the link. Closing. |
Test PR — DO NOT MERGE
Clean retest:
copilot-instructions.mdis now only a pointer toCODING_GUIDELINES.md— zero inline rules. If Copilot flags the violations in_test_violations.py, it proves it followed the link.Violations:
TYPE_CHECKING,getattr