fix(compat): the floor was already in the matrix, and I wrote a different number #19
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -221,21 +221,24 @@ fun clientModeFrom( | |
| /** | ||
| * The oldest node this client can drive. | ||
| * | ||
| * **This is `node_min` from `compat/matrix.json`, and it must stay equal to it.** | ||
| * `CompatibilityFloorMatchesMatrixTest` fails if they diverge. | ||
| * | ||
| * I first wrote 0.5.190 here, which was the SERVER's floor for the opposite | ||
| * question. CIRISServer#497 declares `ciris-client>=0.5.190,<0.6` — which | ||
| * CLIENT versions the server supports. This constant answers which NODE | ||
| * versions the client supports, and the repo already had that answer, recorded | ||
| * with a reason in the compatibility matrix and unchanged at 0.5.168 since | ||
| * 0.5.185. Taking the server's number for it would have nagged on every node | ||
| * between 0.5.168 and 0.5.190 — nodes the matrix says are supported — which is | ||
| * the permanent nag the whole change was made to remove, moved to a different | ||
| * boundary. | ||
| * | ||
| * So the floor is not "unearned" as I described it: it is stated and justified | ||
| * in the matrix, one row per release, append-only. What was missing was | ||
| * anything keeping the two copies equal. That is the test. | ||
| */ | ||
| const val MIN_NODE_VERSION: String = "0.5.168" | ||
|
There was a problem hiding this comment.
This commit changes AGENTS.md reference: AGENTS.md:L26-L26 Useful? React with 👍 / 👎. |
||
|
|
||
| /** | ||
| * Compare two `major.minor.patch` versions NUMERICALLY. Negative, zero, positive. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ def _vertuple(v: str) -> tuple[int, ...]: | |
| return tuple(int(x) for x in v.split(".")) | ||
|
|
||
|
|
||
| def validate(repo_root: Path, client_tree: Path | None = None) -> list[str]: | ||
| problems: list[str] = [] | ||
| path = repo_root / "compat" / "matrix.json" | ||
| if not path.is_file(): | ||
|
|
@@ -106,6 +106,97 @@ def validate(repo_root: Path) -> list[str]: | |
| f"exactly one row must match VERSION ({version}); found {len(matches)} — " | ||
| f"a release without its matrix row does not merge (FSD §6)" | ||
| ) | ||
| problems.extend(check_kotlin_floor(client_tree or (repo_root / 'client'), rows, version)) | ||
| return problems | ||
|
|
||
|
|
||
| # ANCHORED TO A LIVE DECLARATION. A commented-out old value sitting above a | ||
| # changed live one made this capture the comment and pass while the compiled | ||
| # constant disagreed with the matrix — a drift gate reporting green on the drift | ||
| # it exists to catch (Codex, PR #19, reproduced there). | ||
| MIN_NODE_RE = re.compile( | ||
| r'^(?!\s*(?://|\*|/\*))\s*(?:internal\s+|public\s+)?const val MIN_NODE_VERSION' | ||
| r':\s*String\s*=\s*"([^"]+)"', | ||
|
Comment on lines
+118
to
+119
There was a problem hiding this comment.
When an old declaration is retained inside a normal multiline comment whose interior lines do not begin with AGENTS.md reference: AGENTS.md:L52-L52 Useful? React with 👍 / 👎. |
||
| re.M, | ||
| ) | ||
|
Comment on lines
+117
to
+121
There was a problem hiding this comment.
When AGENTS.md reference: AGENTS.md:L44-L44 Useful? React with 👍 / 👎. |
||
| CLIENT_MODE_REL = "shared/src/commonMain/kotlin/ai/ciris/mobile/shared/models/ClientMode.kt" | ||
|
|
||
|
|
||
| def _client_version_of(client_tree: Path, fallback: str) -> str: | ||
| """ | ||
| The version of the tree being graded. | ||
|
|
||
| `--client-tree` points readiness at a consumer's vendored copy, which can be | ||
| at a different release than this repo. Selecting the row by OUR VERSION | ||
| would compare that tree's constant against a row describing a release it is | ||
| not (Codex, PR #19). The generated `ClientVersion.kt` is the tree's own | ||
| answer; absent it — a tree that has not run `generateBuildFlavor` — fall | ||
| back to ours rather than inventing one. | ||
| """ | ||
| gen = client_tree / "shared/build/generated/flavor/commonMain/kotlin/ai/ciris/mobile/shared/models/ClientVersion.kt" | ||
| if gen.is_file(): | ||
| m = re.search(r'CLIENT_VERSION:\s*String\s*=\s*"([^"]+)"', gen.read_text(encoding="utf-8")) | ||
| if m: | ||
| return m.group(1) | ||
| return fallback | ||
|
|
||
|
|
||
| def check_kotlin_floor(client_tree: Path, rows: list, version: str) -> list[str]: | ||
| """ | ||
| `MIN_NODE_VERSION` in Kotlin must equal this release's `node_min`. | ||
|
|
||
| THE SAME FACT IS WRITTEN TWICE. The matrix is where the floor is reasoned | ||
| about, one row per release, append-only; the Kotlin constant is where the | ||
| version banner can read it. The first version of that constant was a | ||
| DIFFERENT NUMBER — the server's client-floor from CIRISServer#497, which | ||
| answers the opposite question — and the client would have nagged on nodes | ||
| this file calls supported. | ||
|
|
||
| CHECKED HERE, NOT IN THE CLIENT'S TEST SUITE. `client/` builds standalone | ||
| with `-PclientVersion` and this tree is vendored into two other repos, none | ||
| of which are required to have `compat/` above them: a Kotlin test that walks | ||
| up looking for this file fails the whole `:shared:desktopTest` task there, | ||
| for a reason that has nothing to do with the client (Codex, PR #19). The | ||
| matrix is the thing being compared against, so the comparison belongs beside | ||
| the matrix, where the file is guaranteed to exist. | ||
|
|
||
| Parses the row as JSON rather than scanning text after a match: a row that | ||
| ever placed `node_min` before `client_version` would send a text scan into | ||
| the NEXT release's floor, and if that value happened to match the constant | ||
| the check would pass while drifting — a gate silently failing to fail. | ||
| """ | ||
| problems: list[str] = [] | ||
| kt = client_tree / CLIENT_MODE_REL | ||
| if not kt.is_file(): | ||
| return [f"{kt} is missing — the floor constant cannot be checked"] | ||
| # Block comments too. The line-anchored exclusion catches `//` and a `*` | ||
| # continuation line, but an old declaration parked inside `/* ... */` need | ||
| # not start its line with anything (Codex, PR #19). Strip them, then match. | ||
| source = re.sub(r"/\*.*?\*/", "", kt.read_text(encoding="utf-8"), flags=re.S) | ||
| m = MIN_NODE_RE.search(source) | ||
| if not m: | ||
| # A parser that finds nothing where the construct plainly exists must | ||
| # fail loudly (AGENTS.md, Gate Rules). | ||
| return [f"parsed no MIN_NODE_VERSION from {kt}"] | ||
| # The caller's loop already reports a non-object row or a missing | ||
| # node_min. Reaching past that to index it turns an actionable failure list | ||
| # into a traceback, for both CI and the imported readiness gate. | ||
| graded = _client_version_of(client_tree, version) | ||
| row = next( | ||
| (r for r in rows | ||
| if isinstance(r, dict) and r.get("client_version") == graded), | ||
| None, | ||
|
Comment on lines
+185
to
+188
There was a problem hiding this comment.
When AGENTS.md reference: AGENTS.md:L49-L50 Useful? React with 👍 / 👎. |
||
| ) | ||
| declared = row.get("node_min") if row else None | ||
| if not isinstance(declared, str): | ||
| return [] | ||
| if m.group(1) != declared: | ||
| problems.append( | ||
| f"MIN_NODE_VERSION is {m.group(1)!r} but the {version} row's " | ||
| f"node_min is {declared!r} — same fact, two copies. The " | ||
| f"matrix is where it is reasoned about; change it there and follow " | ||
| f"in {kt}." | ||
| ) | ||
| return problems | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lowering the default floor to 0.5.168 makes the existing
ClientModeTest.version_mismatch_fires_on_a_node_below_the_floorassertionassertTrue(isVersionMismatch("0.5.175", "0.5.176"))return false, because 0.5.175 is now above the floor. The inspected.github/workflows/build.ymlruns:shared:desktopTest, which includes this common test, so the required desktop test job will fail until the test uses a node version below 0.5.168 (and updates its stale explanation).Useful? React with 👍 / 👎.