feat(sdk-review): batch 6/6 — wire-up + docs + CONTRIBUTING amendment#207
Closed
tiagoek wants to merge 3 commits into
Closed
feat(sdk-review): batch 6/6 — wire-up + docs + CONTRIBUTING amendment#207tiagoek wants to merge 3 commits into
tiagoek wants to merge 3 commits into
Conversation
tiagoek
changed the base branch from
feat/sdk-review-skill
to
feat/sdk-review-batch-5
tiagoek
force-pushed
the
feat/sdk-review-batch-6
branch
from
9d332f7 to
d3570cb
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-5
branch
from
8eb9e39 to
3e1bcdb
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-6
branch
from
d3570cb to
e038203
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-5
branch
from
3e1bcdb to
2bc4612
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-6
branch
from
e038203 to
ae627d3
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-5
branch
from
2bc4612 to
2d1149b
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-6
branch
from
ae627d3 to
7043716
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-5
branch
from
2d1149b to
0d74556
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-6
branch
from
7043716 to
65b3ec2
Compare
Final batch that turns the collection of checks into a working skill: ## Orchestration (.claude/scripts/) - orchestrate.sh — main entry point. Runs preflight, dispatches all 20 checks in parallel, applies baseline + tier gating, posts 4 signals (inline comments, summary, check-run, label) unless --dry-run. - aggregate.sh — merges N check reports into a single summary; applies rules.yaml tiers to each finding. ## Slash command (.claude/commands/) - review-new-module.md — Claude Code slash command that invokes orchestrate.sh ## GitHub Actions (.github/workflows/) - sdk-module-review.yml — auto-trigger on every PR (opened/synchronize/reopened/ready_for_review) + workflow_dispatch - sdk-skill-isolation-check.yml — validates sub-PRs into feat/sdk-review-skill (shellcheck, ruff, bats, size budget, fixtures per check) ## Docs - docs/PR-REVIEW.md — user-facing guide (rules, severities, suppression, BTP dep documentation, breaking changes, tuning noisy rules) - docs/BRANCH-PROTECTION-SETUP.md — admin guide (UI + gh CLI) to configure sdk-module-review as required status check + SHADOW→FLAG→BLOCK rollout ## CONTRIBUTING.md - New section: 'AI-assisted contribution skills — required for review' - States clearly that reviewers will NOT review PRs where sdk-module-review is failing/missing - Documents /review-new-module local + CI auto-trigger ## After merging this batch The skill is fully operational. See docs/BRANCH-PROTECTION-SETUP.md to configure the check-run as a required status check. Part 6 of 6. Ref: AFSDK-3937
…NTRIBUTING
- orchestrate.sh: rename HOSTNAME → GH_HOSTNAME (avoid clashing with the
bash builtin under set -u)
- orchestrate.sh: BASE_SHA resolution prefers origin/${GITHUB_BASE_REF}
before origin/main (correct for PRs targeting a non-main base)
- orchestrate.sh: cap each parallel check with timeout/gtimeout at 60s so
a wedged check can't hang the whole review
- aggregate.sh: skip report-*.json files that fail JSON validation so a
single crashed check doesn't take the aggregation down
- CONTRIBUTING.md: add ## Incremental Delivery section referenced by
PR-SIZE-01 (stacked-PR thresholds and workflow)
tiagoek
force-pushed
the
feat/sdk-review-batch-5
branch
from
0d74556 to
bdd14f1
Compare
tiagoek
force-pushed
the
feat/sdk-review-batch-6
branch
from
65b3ec2 to
58ede70
Compare
…FP rate from 84% to <15%
Wires lib/hunk-filter.sh into 8 check-*.sh scripts so line-scoped findings are
suppressed when the reported line is not in the PR's added-lines set. Adds
config-level fixes and a peer-consistency check surfaced by the empirical
dry-run against the 26-PR consolidated corpus. See docs/plans
09-FP-REMEDIATION.md.
FP-A-01 — hunk attribution (systemic, ~80% of Python FPs)
* lib/hunk-filter.sh: is_line_touched / is_range_touched / is_meta_finding /
emit_finding_if_touched helpers. Permissive when ADDED_LINES_FILE unset.
* Wired into: check-hardcode, check-http-hygiene, check-errors-logging,
check-disclosure, check-secrets, check-telemetry, check-concurrency,
check-constants, check-patterns.
* AST-based emissions (PT-08, EL-01/02, TEL-02, CON-01) filtered post-emit
via ADDED_LINES_FILE, eliminating whole-file scans.
FP-B-01 — HC-01 POM / XML namespace allowlist
* check-hardcode.sh: ignore *.md, *.xml, pom.xml, *.yml, *.yaml, *.properties
* URL allowlist: maven.apache.org/POM/*, w3.org/*/XMLSchema*, xmlsoap.org/*
FP-B-02 — HTTP-01 markdown exclusion
* check-http-hygiene.sh: skip *.md/*.rst/*.txt files and docs/examples/ dirs.
FP-C-01 — BDD-01 glob-based feature detection
* check-bdd.sh: BDD-01 now passes when ANY .feature file exists under
tests/<mod>/integration/, not just <mod>.feature.
FP-C-02 — PY-PT-04 AST-based exception detection
* ast_python_checks.py check_pt_04(module_dir): walks module for classes
subclassing Exception / *Error / *Exception.
* check-patterns.sh: PY-PT-04 uses AST helper, not exceptions.py existence.
FP-D-01 — PY-CON-01 exclusions + per-file cap
* ast_python_checks.py check_con_01: skip _models.py, _generated.py,
_*_api.py generated files; min_len ≥ 3; cap at 3 findings/file.
FP-E-01 — PY-PT-08 line-level baseline
* lib/baseline.sh: is_in_line_baseline / bootstrap_pt_08 helpers.
baseline.json extended with 'line_baseline' array. Bootstrap via:
bash .claude/scripts/lib/baseline.sh bootstrap PY-PT-08 <repo_root>
* check-patterns.sh: PY-PT-08 consults baseline before emit.
FP-G-01 — peer-consistency for PT-01 (replaces 'factory required' law)
* lib/peer-consistency.sh: peer_modules, has_element,
peer_element_fraction, should_flag_peer_divergence.
* check-patterns.sh: PY-PT-01/JV-PT-01 now emit FLAG (not BLOCK) only when
the new module diverges from an element adopted by ≥80% of peer modules.
Elements checked: factory, client, config, user-guide.md, py.typed.
* rules.yaml: PT-01 tier downgraded BLOCK → FLAG.
Regression coverage (tests/sdk-review/test_fp_remediation.bats):
14 new tests pinning every fix above. Full suite: 71 tests, all green.
DEL-01 kept as-is (synthetic src/:1 path, not a per-line finding).
DIS-07/08 kept as-is (PR_BODY metadata, not filtered).
Ref: docs/plans 09-FP-REMEDIATION.md §Pattern A..G.
Contributor
Author
|
Superseded by consolidation into 2 PRs:
Rationale: the 6-batch split proved counterproductive during review — reviewers needed to page across 6 diffs to understand a single feature. Also, the FP-remediation fix (9 patterns eliminated, 94% FP reduction) touched scripts introduced across batches 3-5, making atomic per-batch cascading impractical without hand-editing conflicts. The 2-PR consolidated flow keeps stack discipline (foundation → skill) while making each PR self-contained and reviewable in one sitting. All content from this batch is present on #211 or #212. No functional change was lost. Closing to reduce reviewer confusion. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Final batch — turns the collection of checks into a working skill:
orchestrate.sh+aggregate.sh, GitHub Action workflow (sdk-module-review.yml), sub-PR validator workflow (sdk-skill-isolation-check.yml),docs/PR-REVIEW.md(user-facing),docs/BRANCH-PROTECTION-SETUP.md(admin guide), andCONTRIBUTING.mdamendment (sdk-module-reviewrequired before review). After this batch, the skill is fully operational.This is part 6 of 6 in the incremental delivery of the SDK Module Review skill (AFSDK-3937).
Each sub-PR is scoped to be reviewable in ≤ 30 minutes and satisfies these isolation criteria:
Related Issue
Internal tracking: AFSDK-3937
Type of Change
How to Test
Batch progression
After all 6 batches are merged into
feat/sdk-review-skill, a final cosmetic PR will mergefeat/sdk-review-skillintomain.Checklist