From d95fee49ed4210247391392f4bf54277595302ba Mon Sep 17 00:00:00 2001 From: gcl-coder <88359731+gcl-coder@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:51:48 +0800 Subject: [PATCH] refactor(issue-fix): single-source the metrics supplement vocabulary Refs #4447 Track A: the projection restated both the 19 counter names and the supplement schema name that metrics_supplement already defines. The copy was load-bearing in two directions - it decided which rows the projection renders and which document shape it accepts - while the module that builds and validates that document decided the same two things from its own literals. Import both names from metrics_supplement, the direction repository_snapshot already uses for the schema name. PROJECTION_SCHEMA_VERSION is left alone on both sides: same name, two different documents, values differ, so merging it would change a wire value. Measured on this machine against the same base commit: same-runtime forks 16 -> 15, fork definitions 37 -> 35, schema-version forks 6 -> 5, multi-value twins unchanged at 12. The registry ratchet and the smoke anchor move to those numbers together, which also closes one unit of slack the anchor already carried above the untouched tree. Signed-off-by: gcl-coder <88359731+gcl-coder@users.noreply.github.com> --- examples/semantic-vocabulary-drift-smoke.py | 6 +- .../issue_fix/metrics_projection.py | 31 ++---- loopx/semantics/vocabulary_v0.json | 6 +- ...ics_supplement_vocabulary_single_source.py | 97 +++++++++++++++++++ 4 files changed, 112 insertions(+), 28 deletions(-) create mode 100644 tests/capabilities/test_metrics_supplement_vocabulary_single_source.py diff --git a/examples/semantic-vocabulary-drift-smoke.py b/examples/semantic-vocabulary-drift-smoke.py index d567141e95..54a4941c5c 100755 --- a/examples/semantic-vocabulary-drift-smoke.py +++ b/examples/semantic-vocabulary-drift-smoke.py @@ -232,11 +232,11 @@ TWIN_ROOT_ANCHOR = "loopx/control_plane" TWIN_BUDGET_ANCHOR = 43 BUDGET_ANCHOR = { - "same_runtime_forks": 17, - "same_runtime_fork_definitions": 39, + "same_runtime_forks": 15, + "same_runtime_fork_definitions": 35, "conflicting_values": 16, "conflicting_definitions": 55, - "schema_version_same_runtime_forks": 7, + "schema_version_same_runtime_forks": 5, "multi_value_twins": 13, "multi_value_forks": 2, "multi_value_forks_semantic": 1, diff --git a/loopx/capabilities/issue_fix/metrics_projection.py b/loopx/capabilities/issue_fix/metrics_projection.py index 1ff57671e8..464103ef2a 100644 --- a/loopx/capabilities/issue_fix/metrics_projection.py +++ b/loopx/capabilities/issue_fix/metrics_projection.py @@ -3,9 +3,17 @@ from datetime import datetime from typing import Any +# Refs #4447: one definition for this vocabulary. The supplement builds and +# validates these fields and carries the schema name this projection compares +# against, so it owns both; `repository_snapshot` already imports the schema +# name from there rather than restating it. `PROJECTION_SCHEMA_VERSION` below is +# a different document's version that happens to share a name prefix, so it stays. +from .metrics_supplement import ( + _SUPPLEMENT_FIELDS, + SUPPLEMENT_SCHEMA_VERSION, +) SNAPSHOT_SCHEMA_VERSION = "issue_fix_repository_reporting_snapshot_v0" -SUPPLEMENT_SCHEMA_VERSION = "issue_fix_metrics_supplement_v0" PROJECTION_SCHEMA_VERSION = "issue_fix_metrics_projection_v0" _FLOW_FIELDS = ( @@ -15,27 +23,6 @@ "pull_requests_closed", "pull_requests_merged", ) -_SUPPLEMENT_FIELDS = ( - "human_interventions", - "automatic_terminal_closeouts", - "duplicate_external_writes", - "loopx_capability_gaps_found", - "loopx_capability_gaps_fixed", - "loopx_capability_gaps_real_callsite_verified", - "memory_retrievals", - "memory_verified_decision_influence", - "memory_verified_patch_influence", - "memory_stale_results", - "useful_public_comments", - "triage_outcomes", - "issues_screened", - "issue_close_recommendations", - "issue_close_requests_published", - "issue_closes_observed", - "issue_reopens_observed", - "first_push_ci_passed", - "first_push_ci_total", -) def _nonnegative_int(value: Any, *, field: str) -> int: diff --git a/loopx/semantics/vocabulary_v0.json b/loopx/semantics/vocabulary_v0.json index 422be32d17..bcc3d81464 100644 --- a/loopx/semantics/vocabulary_v0.json +++ b/loopx/semantics/vocabulary_v0.json @@ -1126,11 +1126,11 @@ }, "inventory_ratchets": { "meaning": "Counts read from the generated inventory. A same-runtime fork is one constant name with one value defined in two or more modules of the same runtime; a conflicting value is one name with different values. Both the number of affected names and the number of definitions are budgets, so a third spelling of an already-conflicting name is still a regression.", - "same_runtime_forks": 17, - "same_runtime_fork_definitions": 39, + "same_runtime_forks": 15, + "same_runtime_fork_definitions": 35, "conflicting_values": 16, "conflicting_definitions": 55, - "schema_version_same_runtime_forks": 7, + "schema_version_same_runtime_forks": 5, "multi_value_twins": 13, "multi_value_forks": 2, "multi_value_fork_definitions": 6, diff --git a/tests/capabilities/test_metrics_supplement_vocabulary_single_source.py b/tests/capabilities/test_metrics_supplement_vocabulary_single_source.py new file mode 100644 index 0000000000..6a3da74f93 --- /dev/null +++ b/tests/capabilities/test_metrics_supplement_vocabulary_single_source.py @@ -0,0 +1,97 @@ +"""Refs #4447: one definition for the issue-fix metrics supplement vocabulary. + +`_SUPPLEMENT_FIELDS` (19 counter names) and `SUPPLEMENT_SCHEMA_VERSION` were each +defined twice with identical values: once in `metrics_supplement`, which builds the +document and validates its schema, and once in `metrics_projection`, which reads that +document and rejects a supplement whose schema name differs. The projection's copy was +therefore load-bearing in both directions — it decides which fields are rendered *and* +which document shape is accepted — while the supplement decided the same two things from +its own literals. + +`metrics_supplement` is the owner: `repository_snapshot` already imports +`SUPPLEMENT_SCHEMA_VERSION` from it instead of restating it, so this only extends a +direction the package already uses. + +What this deliberately does not merge: `PROJECTION_SCHEMA_VERSION` names the +projection's own document and reads `issue_fix_metrics_projection_v0`, while the +supplement module's `PROJECTION_SCHEMA_VERSION` names the *supplement's* projection and +reads `issue_fix_metrics_supplement_projection_v0`. Same name, two different documents, +so collapsing them would change a wire value. +""" + +from __future__ import annotations + +import ast +import inspect + +from loopx.capabilities.issue_fix import metrics_projection, metrics_supplement + +SHARED_NAMES = frozenset({"_SUPPLEMENT_FIELDS", "SUPPLEMENT_SCHEMA_VERSION"}) +EXPECTED_FIELDS = ( + "human_interventions", + "automatic_terminal_closeouts", + "duplicate_external_writes", + "loopx_capability_gaps_found", + "loopx_capability_gaps_fixed", + "loopx_capability_gaps_real_callsite_verified", + "memory_retrievals", + "memory_verified_decision_influence", + "memory_verified_patch_influence", + "memory_stale_results", + "useful_public_comments", + "triage_outcomes", + "issues_screened", + "issue_close_recommendations", + "issue_close_requests_published", + "issue_closes_observed", + "issue_reopens_observed", + "first_push_ci_passed", + "first_push_ci_total", +) +EXPECTED_SUPPLEMENT_SCHEMA = "issue_fix_metrics_supplement_v0" + + +def test_vocabulary_is_unchanged() -> None: + """Merging the copies must not change a value or a position.""" + # Order is part of the meaning: the projection renders these names as rows. + assert metrics_supplement._SUPPLEMENT_FIELDS == EXPECTED_FIELDS + assert list(metrics_supplement._SUPPLEMENT_FIELDS) == list(EXPECTED_FIELDS) + assert metrics_supplement.SUPPLEMENT_SCHEMA_VERSION == EXPECTED_SUPPLEMENT_SCHEMA + + +def test_the_projection_reads_the_owner_values() -> None: + """The projection still accepts and renders exactly what it did before.""" + assert ( + metrics_projection._SUPPLEMENT_FIELDS is metrics_supplement._SUPPLEMENT_FIELDS + ) + assert ( + metrics_projection.SUPPLEMENT_SCHEMA_VERSION + == metrics_supplement.SUPPLEMENT_SCHEMA_VERSION + == EXPECTED_SUPPLEMENT_SCHEMA + ) + + +def test_the_two_projection_schema_names_stay_separate() -> None: + """Named the same, owned by different documents: merging would change a wire value.""" + assert ( + metrics_projection.PROJECTION_SCHEMA_VERSION + == "issue_fix_metrics_projection_v0" + ) + assert ( + metrics_supplement.PROJECTION_SCHEMA_VERSION + == "issue_fix_metrics_supplement_projection_v0" + ) + + +def test_the_projection_defines_no_second_copy() -> None: + """A module-level re-assignment here would fork the vocabulary again.""" + tree = ast.parse(inspect.getsource(metrics_projection)) + bound = { + target.id + for node in tree.body + if isinstance(node, ast.Assign) + for target in node.targets + if isinstance(target, ast.Name) + } + + assert not bound & SHARED_NAMES, sorted(bound & SHARED_NAMES)