Skip to content

feat(sdk-review): batch 5/6 — depth checks + PR-size advisory#206

Closed
tiagoek wants to merge 1 commit into
feat/sdk-review-skillfrom
feat/sdk-review-batch-5
Closed

feat(sdk-review): batch 5/6 — depth checks + PR-size advisory#206
tiagoek wants to merge 1 commit into
feat/sdk-review-skillfrom
feat/sdk-review-batch-5

Conversation

@tiagoek

@tiagoek tiagoek commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Adds 10 checks: testing-depth, errors-logging, http-hygiene, concurrency, deletion-hygiene, deps-supply, constants (AST), binding-shape (BND-02 BLOCK_LOCKED), quality-gate-parity, pr-size (SHADOW, advisory).

This is part 5 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:

  • Compiles alone (shellcheck / ruff pass)
  • Testable alone (bats/pytest pass on top of the prior batches)
  • Revert-safe (prior batches keep working if this one is reverted)
  • No breaking changes for subsequent batches

Related Issue

Internal tracking: AFSDK-3937

Type of Change

  • New feature (non-breaking change that adds functionality)

How to Test

git checkout feat/sdk-review-batch-5
bats tests/sdk-review/          # if applicable to this batch
shellcheck -x --severity=error .claude/scripts/**/*.sh

Batch progression

After all 6 batches are merged into feat/sdk-review-skill, a final cosmetic PR will merge feat/sdk-review-skill into main.

Checklist

  • Follows Conventional Commits
  • Isolated (see criteria above)
  • No sensitive information (fixture strings are placeholder patterns, not real credentials)

@tiagoek tiagoek changed the base branch from feat/sdk-review-skill to feat/sdk-review-batch-4 July 6, 2026 15:06
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-4 branch from 5afd4f7 to ab90a3c Compare July 6, 2026 15:18
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-5 branch from 8eb9e39 to 3e1bcdb Compare July 6, 2026 15:18
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-4 branch from ab90a3c to aa08669 Compare July 6, 2026 15:52
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-5 branch from 3e1bcdb to 2bc4612 Compare July 6, 2026 15:52
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-4 branch from aa08669 to 582b50f Compare July 6, 2026 15:59
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-5 branch from 2bc4612 to 2d1149b Compare July 6, 2026 15:59
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-4 branch from 582b50f to 869b41d Compare July 6, 2026 19:30
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-5 branch from 2d1149b to 0d74556 Compare July 6, 2026 19:30
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-4 branch from 869b41d to 1a7ccc8 Compare July 6, 2026 20:13
@tiagoek tiagoek force-pushed the feat/sdk-review-batch-5 branch from 0d74556 to bdd14f1 Compare July 6, 2026 20:13
…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.
Base automatically changed from feat/sdk-review-batch-4 to feat/sdk-review-skill July 7, 2026 15:08
@tiagoek

tiagoek commented Jul 7, 2026

Copy link
Copy Markdown
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.

@tiagoek tiagoek closed this Jul 7, 2026
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.

1 participant