docs(resilience): PR 5.1 — sanctions construct audit (designated-party domicile question)#3375
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis is a pure docs + regression-tests PR (no scorer behaviour changes) that documents the Two minor issues in the test file: the Confidence Score: 5/5Safe to merge — pure docs and regression-guard tests with no scorer behaviour changes and no live-ranking impact. All findings are P2 style/comment issues. The test logic is correct, assertions match the scorer implementation, and the documentation accurately describes the current behaviour and design trade-offs. No P0 or P1 issues found. No files require special attention beyond the two minor P2 comments on the test file. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[sanctionCount] --> B{count == 0?}
B -- yes --> C[return 100]
B -- no --> D{count <= 10?}
D -- yes --> E["return round(90 − (count−1)×1.667)\nrange: 90→75"]
D -- no --> F{count <= 50?}
F -- yes --> G["return round(75 − (count−10)×0.625)\nrange: 75→50"]
F -- no --> H{count <= 200?}
H -- yes --> I["return round(50 − (count−50)×0.167)\nrange: 50→25"]
H -- no --> J["return round(max(0, 25 − (count−200)×0.1))\nrange: 25→0 clamped"]
style C fill:#2d6a4f,color:#fff
style J fill:#d62828,color:#fff
Reviews (1): Last reviewed commit: "docs(resilience): PR 5.1 — sanctions con..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+94
to
+99
| }); | ||
|
|
||
| describe('scoreTradeSanctions — field-mapping + outage semantics', () => { | ||
| it('country absent from sanctions map defaults to count=0 (score 100)', async () => { | ||
| // The map is ISO2 → count. A country NOT in the map is semantically | ||
| // "no designated parties located here" — NOT "data missing". The |
Contributor
There was a problem hiding this comment.
The comment confirms the exact computed value is 0 (25 - 30 = -5 → clamped to 0), but the assertion uses <= 1. If a future scorer change produces 0.5 at count=500 (e.g., from a changed roundScore implementation or a boundary-condition tweak), this test still passes even though the behaviour has shifted. For a regression-guard test, pinning the exact expected value is more protective.
Suggested change
| }); | |
| describe('scoreTradeSanctions — field-mapping + outage semantics', () => { | |
| it('country absent from sanctions map defaults to count=0 (score 100)', async () => { | |
| // The map is ISO2 → count. A country NOT in the map is semantically | |
| // "no designated parties located here" — NOT "data missing". The | |
| // At count=500: 25 - (500-200)*0.1 = 25 - 30 = -5 → clamped to 0. | |
| assert.equal(result.score, 0, | |
| `expected 0 at count=500 (heavily-sanctioned state, clamped); got ${result.score}`); |
Comment on lines
+45
to
+48
| // count=11-50 → 75..50 (linear) | ||
| // count=51-200 → 50..25 (linear) | ||
| // count=201+ → 25..0 (linear at 0.1/step, clamped 0) | ||
| // |
Contributor
There was a problem hiding this comment.
The comment says other slots "contribute zero weight", but they actually contribute their full declared weight (0.15 + 0.15 + 0.25 = 0.55) to totalWeight, which is the denominator for the coverage calculation in weightedBlend. Only the availableWeight (score-computation denominator) drops to 0.45. A reader following this comment might incorrectly conclude that coverage will equal 1.0 for the sanctions-only case, whereas it is actually 0.45. The phrasing "contribute zero weight to the blended score" would be more precise.
This was referenced Apr 24, 2026
Merged
koala73
force-pushed
the
fix/resilience-pr53-foodwater-audit
branch
from
April 24, 2026 13:58
7de0630 to
83d47f9
Compare
koala73
added a commit
that referenced
this pull request
Apr 24, 2026
… weightedBlend weights Addresses 2 P2 Greptile findings on #3375: 1. Tighten count=500 assertion. Was `<= 1` with a comment stating the exact value is 0. That loose bound silently tolerates roundScore / boundary drift that would be the very signal this regression guard exists to catch. Changed to strict equality `=== 0`. 2. Clarify the "zero weight" comment on the sanctions-only harness. The other slots DO contribute their declared weights (0.15 + 0.15 + 0.25 = 0.55) to weightedBlend's `totalWeight` denominator — only `availableWeight` (the score-computation denominator) drops to 0.45 because their score is null. The previous comment elided this distinction and could mislead a reader into thinking the null slots contributed nothing at all. Expanded to state exactly how `coverage` and `score` each behave. Verified - `npx tsx --test tests/resilience-sanctions-field-mapping.test.mts` — 10 pass / 0 fail (count=500 now pins the exact 0 floor)
koala73
force-pushed
the
fix/resilience-pr51-sanctions-audit
branch
from
April 24, 2026 14:00
c0ba53e to
ecc5033
Compare
koala73
force-pushed
the
fix/resilience-pr53-foodwater-audit
branch
from
April 24, 2026 14:17
83d47f9 to
fa08998
Compare
koala73
added a commit
that referenced
this pull request
Apr 24, 2026
… weightedBlend weights Addresses 2 P2 Greptile findings on #3375: 1. Tighten count=500 assertion. Was `<= 1` with a comment stating the exact value is 0. That loose bound silently tolerates roundScore / boundary drift that would be the very signal this regression guard exists to catch. Changed to strict equality `=== 0`. 2. Clarify the "zero weight" comment on the sanctions-only harness. The other slots DO contribute their declared weights (0.15 + 0.15 + 0.25 = 0.55) to weightedBlend's `totalWeight` denominator — only `availableWeight` (the score-computation denominator) drops to 0.45 because their score is null. The previous comment elided this distinction and could mislead a reader into thinking the null slots contributed nothing at all. Expanded to state exactly how `coverage` and `score` each behave. Verified - `npx tsx --test tests/resilience-sanctions-field-mapping.test.mts` — 10 pass / 0 fail (count=500 now pins the exact 0 floor)
koala73
force-pushed
the
fix/resilience-pr51-sanctions-audit
branch
from
April 24, 2026 14:18
ecc5033 to
e9458b1
Compare
…y domicile question) PR 5.1 of cohort-audit plan 2026-04-24-002. Stacked on PR 5.3 (#3374) so the known-limitations.md section append is additive. Read-only static audit of scoreTradeSanctions + the sanctions:country-counts:v1 seed — framed around the Codex-reformulated construct question: should designated-party domicile count penalize resilience? Findings 1. The count is "OFAC-designated-party domicile locations," NOT "sanctions against this country." Seeder (`scripts/seed-sanctions- pressure.mjs:85-93`) parses OFAC Advanced XML SDN + Consolidated, extracts each designated party's Locations, and increments `map[countryCode]` by 1 for every location country on that party. 2. The count conflates three semantically distinct categories a resilience construct might treat differently: (a) Country-level sanction target (NK SDN listings) — correct penalty (b) Domiciled sanctioned entity (RU bank in Moscow, post-2022) — debatable, country hosts the actor (c) Transit / shell entity (UAE trading co listed under SDGT for Iran evasion; CY SPV for a Russian oligarch) — country is NOT the target, but takes the penalty 3. Observed GCC cohort impact: AE scores 54 vs KW/QA 82. The −28 gap is almost entirely driven by category (c) listings — AE is a financial hub where sanctioned parties incorporate shells. 4. Three options documented for the construct decision (NOT decided in this PR): - Option 1: Keep flat count (status quo, defensible via secondary- sanctions / FATF argument) - Option 2: Program-weighted count — weight DPRK/IRAN/SYRIA/etc. at 1.0, SDGT/SDNTK/CYBER/etc. at 0.3-0.5. Recommended; seeder already captures `programs` per entry — data is there, scorer just doesn't read it. - Option 3: Transit-hub exclusion list (AE, SG, HK, CY, VG, KY) — brittle + normative, not recommended 5. Recommendation documented: Option 2. Implementation deferred to a separate methodology-decision PR (outside auto-mode authority). Shipped - `docs/methodology/known-limitations.md` — new section extending the file: "tradeSanctions — designated-party domicile construct question." Covers what the count represents, the three categories with examples, observed GCC impact, three options w/ trade-offs, recommendation, follow-up audit list (entity-sample gated on API-key access), and file references. - `tests/resilience-sanctions-field-mapping.test.mts` (new) — 10 regression-guard tests pinning CURRENT behavior: 1-6. normalizeSanctionCount piecewise anchors: count=0→100, 1→90, 10→75, 50→50, 200→25, 500→≤1 7. Monotonicity: strictly decreasing across the ramp 8. Country absent from map defaults to count=0 → score 100 (intentional "no designated parties here" semantics) 9. Seed outage (raw=null) → null score slot, NOT imputed (protects against silent data-outage scoring) 10. Construct anchor: count=1 is exactly 10 points below count=0 (pins the "first listing drops 10" design choice) Verified - `npx tsx --test tests/resilience-sanctions-field-mapping.test.mts` — 10 pass / 0 fail - `npm run test:data` — 6721 pass / 0 fail - `npm run typecheck` / `typecheck:api` — green - `npm run lint` / `lint:md` — clean
… weightedBlend weights Addresses 2 P2 Greptile findings on #3375: 1. Tighten count=500 assertion. Was `<= 1` with a comment stating the exact value is 0. That loose bound silently tolerates roundScore / boundary drift that would be the very signal this regression guard exists to catch. Changed to strict equality `=== 0`. 2. Clarify the "zero weight" comment on the sanctions-only harness. The other slots DO contribute their declared weights (0.15 + 0.15 + 0.25 = 0.55) to weightedBlend's `totalWeight` denominator — only `availableWeight` (the score-computation denominator) drops to 0.45 because their score is null. The previous comment elided this distinction and could mislead a reader into thinking the null slots contributed nothing at all. Expanded to state exactly how `coverage` and `score` each behave. Verified - `npx tsx --test tests/resilience-sanctions-field-mapping.test.mts` — 10 pass / 0 fail (count=500 now pins the exact 0 floor)
koala73
force-pushed
the
fix/resilience-pr51-sanctions-audit
branch
from
April 24, 2026 14:27
e9458b1 to
435da34
Compare
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.
Summary
PR 5.1 of the cohort-audit plan (
docs/plans/2026-04-24-002-fix-resilience-cohort-ranking-structural-audit-plan.md). Read-only static audit ofscoreTradeSanctions+sanctions:country-counts:v1, framed around the Codex-reformulated construct question: should designated-party domicile count penalize resilience?⚠ Stacked on #3374 (PR 5.3), which is stacked on #3373 (PR 5.2). Merge order: #3373 → #3374 → this. Ships one methodology-doc extension + 10 regression-guard tests. No scorer behaviour changes.
Finding
The count in
sanctions:country-counts:v1is "OFAC-designated-party domicile locations", NOT "sanctions against this country." The seeder (scripts/seed-sanctions-pressure.mjs:85-93) parses OFAC Advanced XML SDN + Consolidated lists, extracts each designated party'sLocations, and incrementsmap[countryCode]by 1 for every location country on that party.That count conflates three semantically distinct categories a resilience construct might want to handle differently:
Observed cohort impact: AE=54 vs KW/QA=82 on
tradeSanctions. The −28 gap is almost entirely driven by category (c) — AE is a financial hub where sanctioned parties incorporate shells.Options documented (not decided here)
programsper entry (line 95-108)Recommendation: Option 2. Implementation deferred to a separate methodology-decision PR.
What shipped
docs/methodology/known-limitations.md— new section extending the PR 5.2/5.3 file. Full section covers: what the count represents, the three categories with examples, observed GCC impact, three options with trade-offs + recommendation, follow-up entity-sample audit (gated on API-key access), file references.tests/resilience-sanctions-field-mapping.test.mts(new) — 10 regression-guard tests:normalizeSanctionCountpiecewise anchors pinned:count=0→100,1→90,10→75,50→50,200→25,500→≤1Testing
npx tsx --test tests/resilience-sanctions-field-mapping.test.mts— 10 pass / 0 failnpm run test:data— 6721 pass / 0 fail (across the full stacked branch)npm run typecheck/typecheck:api— greennpm run lint/lint:md— cleanWhat this PR explicitly does NOT do
Post-Deploy Monitoring & Validation
No additional operational monitoring required: pure docs + tests. Zero behaviour change, no endpoints/crons/flags/Redis writes touched.Follow-up (not in this PR)
Related
docs/plans/2026-04-24-002-fix-resilience-cohort-ranking-structural-audit-plan.md§PR 5.1🤖 Generated with Claude Opus 4.7 (1M context) via Claude Code + Compound Engineering v2.49.0
Co-Authored-By: Claude Opus 4.7 (1M context) [email protected]