add architecture and infra rules#2
Conversation
🌿 Green API — Combined Report🔴 Green API Score: 39/100 (Grade: D)🔍 Endpoints: 22 🟢 🌱 Creedengo Éco-Design: 88/100 (Grade: A)🐛 Issues: 137 | Règles violées: 2 📋 Top règles Creedengo (2)
📅 2026-04-29T15:58:32.644Z |
There was a problem hiding this comment.
Pull request overview
This PR extends the Green API Score tooling by adding Architecture/Infrastructure (AR01–AR05) rule evaluation, expanding the static Spectral ruleset, and introducing thresholds/configuration needed for these new checks.
Changes:
- Add a new
scripts/architecture_rules.pymodule to evaluate AR01–AR05 (with Phase 1 implementations and placeholders for later phases). - Integrate architecture rule evaluation into
scripts/green-api-auto-discover.py, including new CLI flags and report merging logic. - Extend configuration (
green-score-threshold.json) and significantly expand.spectral.ymlwith categorized Green API rules.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| scripts/green-api-auto-discover.py | Imports/executes architecture rules, adds CLI flags, merges AR results into the Green Score output. |
| scripts/architecture_rules.py | Implements architecture/infrastructure rule detection (AR01/AR03/AR05) and placeholders for AR02/AR04. |
| green-score-threshold.json | Adds architecture thresholds and cloud dashboard deep-links used by AR rules. |
| .spectral.yml | Reworks/extends Spectral rules to cover more Green Score checks with categories and stronger schemas. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| spec=spec, | ||
| sources=sources, | ||
| endpoints=endpoints, | ||
| measurements=measurements.get("auto_discovery", {}).get("all_measurements", {}) or {}, |
There was a problem hiding this comment.
The measurements argument passed into evaluate_architecture_rules() is always {} because measurements is keyed by ":" (e.g. get:/books) and does not contain an auto_discovery entry at this stage. This prevents AR01/AR05 detection from using runtime headers/content-types.
Pass the actual measurements dict collected in Step 3 (or build the all_measurements structure first and pass that).
| measurements=measurements.get("auto_discovery", {}).get("all_measurements", {}) or {}, | |
| measurements=measurements or {}, |
| green_score["total"] = total_raw | ||
| green_score["max"] = max_raw | ||
| green_score["grade"] = grade | ||
| green_score["score_normalised_100"] = score_normalised |
There was a problem hiding this comment.
This block says the score is “normalised /100 for backward compatibility”, but the code overwrites green_score['total'] and green_score['max'] with the raw 123-point budget values. The dashboard (pctScore = total/max) and CI workflow summary (prints ${TOTAL}/100) both assume a 0–100 scale, so this change will produce misleading percentages and can break gating.
Either keep total/max on a 100-point scale (e.g., set total=score_normalised_100 and max=100, and store raw totals under separate keys), or update all downstream consumers accordingly.
| green_score["total"] = total_raw | |
| green_score["max"] = max_raw | |
| green_score["grade"] = grade | |
| green_score["score_normalised_100"] = score_normalised | |
| # Keep total/max on a /100 scale for backward compatibility with existing | |
| # dashboard and CI consumers; preserve the expanded-budget totals separately. | |
| green_score["total"] = score_normalised | |
| green_score["max"] = 100 | |
| green_score["grade"] = grade | |
| green_score["score_normalised_100"] = score_normalised | |
| green_score["raw_total"] = total_raw | |
| green_score["raw_max"] = max_raw |
| parser.add_argument("--consumer-region", default="", | ||
| help="ISO-3166 alpha-2 region of the API consumers (e.g. FR, US). Used by AR02 distance scoring.") | ||
| parser.add_argument("--enable-geoip", action="store_true", | ||
| help="Enable optional GeoIP lookup (ipinfo.io) to compute consumer<->API distance for AR02.") |
There was a problem hiding this comment.
New CLI options --consumer-region and --enable-geoip are parsed but never used, which is confusing for users and makes it hard to tell whether AR02 is actually implemented.
Either wire them into the architecture rules evaluator (and/or report output), or remove them until Phase 3 is implemented.
| parser.add_argument("--consumer-region", default="", | |
| help="ISO-3166 alpha-2 region of the API consumers (e.g. FR, US). Used by AR02 distance scoring.") | |
| parser.add_argument("--enable-geoip", action="store_true", | |
| help="Enable optional GeoIP lookup (ipinfo.io) to compute consumer<->API distance for AR02.") |
| from architecture_rules import ( | ||
| evaluate_architecture_rules, | ||
| ARCH_RULES as _ARCH_RULES_CATALOGUE, | ||
| ) |
There was a problem hiding this comment.
_ARCH_RULES_CATALOGUE is imported but never used. Please remove the unused import (or use it) to avoid lint failures and keep the import block minimal.
| from architecture_rules import ( | |
| evaluate_architecture_rules, | |
| ARCH_RULES as _ARCH_RULES_CATALOGUE, | |
| ) | |
| from architecture_rules import evaluate_architecture_rules |
| "validated": bool(arch_res.get("matched")), | ||
| "score": int(arch_res.get("score", 0)), | ||
| "matched_count": 1 if arch_res.get("matched") else 0, | ||
| "candidate_count": max(1, len(arch_res.get("candidates", []))), | ||
| "candidates": arch_res.get("candidates", []), |
There was a problem hiding this comment.
matched_count and candidate_count are not computed consistently for architecture rules: matched_count is forced to 0/1 based on the top-level matched flag, and candidate_count is forced to at least 1 even when there are actually 0 candidates. This makes the Rule ↔ Resource mapping and dashboard endpoint counts inaccurate (e.g., AR01 may have multiple candidates/evidences but will always show 1 matched).
Compute matched_count from the candidates list when available (sum of c['matched']), and keep candidate_count equal to the actual candidate list length (or explicitly represent “no candidates” as 0 and handle it in the UI).
| Versioned duplicates (``/v1/...`` vs ``/v2/...``) are exempt: the | ||
| signature normaliser strips ``/vN`` so they collapse to the same path, | ||
| but they remain a *legitimate* form of duplication and we downgrade the | ||
| severity to a warning instead of failing the rule. |
There was a problem hiding this comment.
The AR03 docstring says versioned duplicates (/v1/... vs /v2/...) are “exempt” / downgraded, but the current implementation strips the /vN prefix in _normalise_path(), which increases similarity and can cause such versioned APIs to be flagged and penalized.
Either implement the exemption/downgrade logic as described, or update the docstring to match the current behavior so users don’t misinterpret AR03 results.
| Versioned duplicates (``/v1/...`` vs ``/v2/...``) are exempt: the | |
| signature normaliser strips ``/vN`` so they collapse to the same path, | |
| but they remain a *legitimate* form of duplication and we downgrade the | |
| severity to a warning instead of failing the rule. | |
| Path signatures are normalised before comparison; notably, ``/vN`` | |
| prefixes are stripped, which can increase similarity for versioned paths | |
| such as ``/v1/...`` and ``/v2/...``. This evaluator does not treat those | |
| pairs as automatically exempt, so versioned APIs may still be reported | |
| when all three duplication signals cross their thresholds. |
| propertyNames: | ||
| pattern: "(?i)^(content-encoding|vary)$" |
There was a problem hiding this comment.
In green-api-compression-header, the JSON Schema uses propertyNames.pattern under headers. propertyNames applies to all header names, so this rule will fail as soon as a response documents any header other than Content-Encoding/Vary (which is the common case).
Use a presence check instead (e.g., anyOf with required: [Content-Encoding] / required: [Vary], or properties + required), so the rule validates when at least one of those headers is documented.
| propertyNames: | |
| pattern: "(?i)^(content-encoding|vary)$" | |
| anyOf: | |
| - required: [Content-Encoding] | |
| - required: [content-encoding] | |
| - required: [Vary] | |
| - required: [vary] |
| propertyNames: | ||
| pattern: "(?i)^retry-after$|^x-ratelimit-(limit|remaining|reset)$" |
There was a problem hiding this comment.
In green-api-retry-after-header, the schema checks headers.propertyNames.pattern. This requires every documented header in the 429 response to match the retry-after / x-ratelimit-* regex, so the rule will fail if the API documents additional headers (e.g., Content-Type), even when Retry-After is present.
Switch to an “at least one of these keys exists” check (e.g., anyOf with required: [Retry-After], required: [X-RateLimit-Reset], etc.).
| propertyNames: | |
| pattern: "(?i)^retry-after$|^x-ratelimit-(limit|remaining|reset)$" | |
| anyOf: | |
| - required: ["Retry-After"] | |
| - required: ["X-RateLimit-Limit"] | |
| - required: ["X-RateLimit-Remaining"] | |
| - required: ["X-RateLimit-Reset"] |
🌿 Green API — Combined Report🔴 Green API Score: 36/100 (Grade: E)🔍 Endpoints: 22 🟢 🌱 Creedengo Éco-Design: 88/100 (Grade: A)🐛 Issues: 137 | Règles violées: 2 📋 Top règles Creedengo (2)
📅 2026-04-29T19:46:54.545Z |
🌿 Green API — Combined Report🔴 Green API Score: 39/100 (Grade: D)🔍 Endpoints: 22 🟢 🌱 Creedengo Éco-Design: 88/100 (Grade: A)🐛 Issues: 137 | Règles violées: 2 📋 Top règles Creedengo (2)
📅 2026-04-29T20:56:06.995Z |
🌿 Green API — Combined Report🔴 Green API Score: 39/100 (Grade: D)🔍 Endpoints: 22 🟢 🌱 Creedengo Éco-Design: 88/100 (Grade: A)🐛 Issues: 137 | Règles violées: 2 📋 Top règles Creedengo (2)
📅 2026-04-29T21:10:27.958Z |
🌿 Green API — Combined Report🔴 Green API Score: 39/100 (Grade: D)🔍 Endpoints: 22 🟢 🌱 Creedengo Éco-Design: 88/100 (Grade: A)🐛 Issues: 137 | Règles violées: 2 📋 Top règles Creedengo (2)
📅 2026-04-29T21:17:53.784Z |
🌿 Green API — Combined Report🔴 Green API Score: 39/100 (Grade: D)🔍 Endpoints: 22 🟢 🌱 Creedengo Éco-Design: 88/100 (Grade: A)🐛 Issues: 137 | Règles violées: 2 📋 Top règles Creedengo (2)
📅 2026-04-30T06:22:36.051Z |
🌿 Green API — Combined Report🔴 Green API Score: 39/100 (Grade: D)🔍 Endpoints: 22 🟢 🌱 Creedengo Éco-Design: 88/100 (Grade: A)🐛 Issues: 137 | Règles violées: 2 📋 Top règles Creedengo (2)
📅 2026-04-30T06:34:55.601Z |
|
@copilot apply changes based on the comments in this thread |
Green API Review Checklist (API Green Score)
Merci d’indiquer les mesures avant/après (payload/latence).
Data Exchange
fields=(whitelist, champs coûteux off par défaut)Cache-Control+ ETag/Last-Modified→ 304 en test)/changes?since=…ousinceVersion)Range: bytes=pour ressources volumineuses)Usage / Archi / Logs
Sécurité & robustesse
size, whitelistfields)Mesures
🌿 Green Score (automatique)