docs: consolidate TDD/HIL customization rules - #147
Conversation
- Narrowed workflow entry point to focus on approved PoKeysHal implementation work. - Updated prompt to apply specifically to C and header files, enhancing clarity for TDD processes. - Refined TDD cycle steps to emphasize the importance of issue traceability and adherence to TDD principles. - Improved documentation structure for better readability and usability in TDD workflows. ## Description Brief description of the changes in this PR. ## Related Issues Fixes # Implements # Part of # ## 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 not work as expected) - [ ] Documentation update - [ ] Real-time / HAL component change ## Traceability - **Requirements**: # - **Architecture / Design**: # - **Tests**: # ## Testing Describe how this was tested: - [ ] `bash test_compile.sh` passes - [ ] `make -f Makefile.noqmake` builds without errors - [ ] HAL userspace component loads (`halrun` / `loadusr`) - [ ] HAL RT component loads (`halrun` / `loadrt`) - [ ] No memory allocation in RT paths - [ ] No blocking calls in RT paths ## Real-Time Safety Checklist (if RT code changed) - [ ] No `malloc`/`free` in real-time thread paths - [ ] No blocking socket operations (all sockets are `O_NONBLOCK`) - [ ] RT function completes within timing budget (<50µs for PREEMPT_RT userspace/soft-RT; <5µs for RTAI/hard-RT kernel module) - [ ] `mlockall` used where required - [ ] HAL pin types are correct (`hal_s32_t`, `hal_bit_t`, `hal_float_t`) ## Checklist - [ ] All tests pass - [ ] Documentation updated alongside code changes - [ ] No unrelated files changed - [ ] Traceability links verified (PR links to implementing issue)
zarfld
left a comment
There was a problem hiding this comment.
Changes required before merge.
The four-file scope, branch topology, canonical HIL terminology changes, and removal of the duplicated Step C outcome table are correct. One semantic regression remains in apply-tdd-infrastructure-plan.prompt.md: the patch removes the existing prohibition on claiming RT-validated and timing-validated, although #146 explicitly says not to otherwise change this prompt. Restore that guard after the legacy-label migration sentence, using canonical RT-validated / Timing-validated spelling.
The PR metadata also needs replacement. The current title and body describe a C99 TDD prompt rewrite, contain empty Fixes # / Part of # placeholders, and include irrelevant RT-code checklists. Use a Phase 3 consolidation title and a concise body containing Closes #146 and Part of #140, the exact four-file scope, and the documentation-only verification evidence.
No CI status contexts validate this documentation patch. The two successful workflow runs associated with the head are unrelated lifecycle issue-creation workflows and should not be cited as verification.
|
|
||
| Use `Implemented` and `configuration-validated` precisely. Do not claim | ||
| `HIL-tested`, `hardware-verified`, `RT-validated`, or `timing-validated`. No newline at end of file | ||
| Use `Implemented` and `configuration-validated` precisely. Do not use legacy labels such as `HIL-tested` or `Hardware-verified`. Use `HIL-observed`, |
There was a problem hiding this comment.
This replacement drops the existing RT-validated / timing-validated claim prohibition. Issue #146 authorizes only terminology migration here and says not to otherwise change the prompt. Keep the new legacy-label warning, then retain a sentence such as: Do not claim RT-validated or Timing-validated without corresponding evidence.
There was a problem hiding this comment.
🟡 Not ready to approve
AGENTS.md uses non-sequential ordered-list numbering for rule IDs, which will render with incorrect numbering on GitHub and should be reformatted to preserve the intended rule numbers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR updates PoKeysHal’s contributor/TDD documentation to standardize hardware-in-the-loop (HIL) evidence vocabulary and reduce duplication by pointing readers to the canonical HIL result schema.
Changes:
- Replaced legacy “Hardware-verified” / “HIL-tested” terminology with
HIL-observed,HIL-test-executed, andHIL-verifiedand added references to the shared schema. - Removed the duplicated preflight outcomes table from the
hil-tddskill doc in favor of the centralreferences/result-schema.md. - Refined the TDD infrastructure plan prompt to discourage legacy labels and direct users to the schema for correct usage.
File summaries
| File | Description |
|---|---|
| AGENTS.md | Updates status-term vocabulary for evidence reporting and points to the HIL schema. |
| .github/skills/hil-tdd/SKILL.md | Removes duplicated outcome table and centralizes outcome/status guidance via schema reference. |
| .github/prompts/apply-tdd-infrastructure-plan.prompt.md | Updates reporting guidance to avoid legacy labels and use schema-defined HIL terms. |
| .github/instructions/engineering-discipline.instructions.md | Replaces legacy hardware-verification terminology with schema-defined HIL evidence terms. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| 6. **No RT shortcuts** — verify the complete call path; do not trust the name `Async`. | ||
| 10. **Report exact evidence** — use precise status terms: Implemented / Compiled / Tested / Hardware-verified / RT-validated / Timing-validated. | ||
| 10. **Report exact evidence** — use precise status terms: Implemented / Compiled / Tested / HIL-observed / HIL-test-executed / HIL-verified / RT-validated / Timing-validated. For HIL status definitions and outcomes, use `.github/skills/hil-tdd/references/result-schema.md`. |
zarfld
left a comment
There was a problem hiding this comment.
Final review against head 23003a72a671964ce568fc5c557afd711721b812.
No blocking findings remain.
Verified:
- PR title/body now match Phase 3 scope and include
Closes #146/Part of #140; - exactly the four files approved by #146 are changed;
- the prompt retains the original prohibition on claiming
RT-validatedandTiming-validatedwhile adding the canonical HIL migration warning; AGENTS.mdand the engineering-discipline instruction useHIL-observed,HIL-test-executed, andHIL-verifiedand defer definitions to the shared result schema;- the duplicated Step C outcome table is removed from the HIL skill while the zero-results invariant remains;
- no agents, Phase 2 prompts, fixtures, tests, production C/HAL, or result-schema files changed.
The Copilot comment about non-sequential numbering in AGENTS.md concerns pre-existing structure and is outside issue #146. It should be handled separately rather than broadening this PR.
The documentation-specific verification reported in the PR is sufficient for this scoped change. The workflow runs attached to the head are unrelated lifecycle issue-creation workflows and are not treated as validation.
PR #147 is ready to merge. Keep #140 open until the final Phase 3/parent-issue closure check is recorded.
Closes #146
Part of #140
Summary
Hardware-verifiedterminology with the canonicalHIL-observed,HIL-test-executed, andHIL-verifiedstatuses..github/skills/hil-tdd/references/result-schema.mdas the singleauthority for HIL status and outcome definitions.
Scope
Only these files changed:
AGENTS.md.github/instructions/engineering-discipline.instructions.md.github/skills/hil-tdd/SKILL.md.github/prompts/apply-tdd-infrastructure-plan.prompt.mdVerification
Hardware-verifieddefinition.git diff --checkpasses.