Add the crosswalk schema every crosswalk already declares - #121
Conversation
Every crosswalk in this repository declares a $schema URL that returns 404, so records validate and crosswalks have nothing to validate against. A dangling $schema is worse than an absent one, because it reads as validated. The schema is derived from the four crosswalk files rather than from an idea of what a crosswalk should look like, so it accepts all four unchanged. The required set is exactly what all four already carry, and inside source and target only url is universal, since a standard carries a version and a record count while a scanner carries a vendor and a rule total. The validator adds two checks a schema cannot express: that the $schema a crosswalk declares is a file this repository ships, which is the bug that started this, and that every identifier a crosswalk cites resolves to a record, which no pattern can catch because a well-formed id for a record that does not exist looks exactly like one for a record that does. A commit field is defined and left optional. A record count with a read date cannot be re-derived by a reader, and a date does not identify a tree: twelve commits across seven days of this history all carried the same record count. Signed-off-by: Sankalp Gilda <sankalp.gilda@gmail.com>
|
This is exactly the kind of PR that's hard to fault on substance, the retroactive-commit research alone is worth more than most schema PRs bring: finding twelve commits over seven days all carrying the same 59-record count means even your own earlier claim can't be pinned after the fact, that's a real argument, not an assertion. On the actual question: Agreed on no unversioned alias, for the reason you gave, a hand-synced duplicate is the same staleness risk The FORMAT_CHECKER gap in Read the actual diff in full, not just the description, tracing the logic against every claim made above. It holds up. The One real question before merge: is One small, non-blocking gap: Merging once the |
The schema declares "format": "uri" on endpoint.url, but the dev extra installed only jsonschema, which implements date itself and registers uri only when a URI library is present. The constraint parsed and then constrained nothing: "not a uri at all" validated clean, and the ^https?:// pattern beside it was the only thing refusing a bad url. Add rfc3986-validator to the dev extra. It registers uri and uri-reference, pulls in no transitive dependencies, and avoids both rfc3987, which is GPL, and the [format-nongpl] extra, which installs thirteen distributions to reach the same two checkers. The header comment in validate_crosswalks.py claimed both date and uri were enforced, which contradicted the schema description in the same change and was wrong about uri. Say what the code does and what it depends on. Bound the coverage counters with minimum: 0, matching record_count. A negative counter passed before. minimum constrains only numbers, so the one prose value coverage is allowed to carry is unaffected. All four crosswalks still validate, all 70 records still validate, and the test suite passes.
|
No, it is not there, and the constraint has been inert the whole time. There is no requirements.txt in the repository at all; the dependency set is pyproject.toml, where the dev extra was pytest, pytest-cov and jsonschema. CI installs exactly that, with pip install -e ".[dev]". I built a clean venv the same way to see what turned up. The format checker registers date, email, idn-email, ipv4, ipv6, regex, time and uuid. No uri. That leaves format: uri on endpoint.url doing nothing, "not a uri at all" passes straight through, and the ^https?:// pattern is what has been refusing a bad url all along. You were right to ask. The field is protected either way, as you guessed: in my mutation log the refusal cites the pattern every time. format: date is the other half of the answer, and that one is live, since jsonschema implements it with no extra package. Uri was the only one of the pair with the problem. So generated and checked_against_live_site are genuinely enforced dates, and 2026-13-45 really is refused, for the reason I gave when I opened this. Pushed: rfc3986-validator added to the dev extra. I did check which package actually does the work, since jsonschema used to route uri through rfc3987, which is GPL. On 4.26, rfc3986-validator alone registers uri and uri-reference, brings no transitive dependencies of its own, and avoids the thirteen distributions [format-nongpl] drags in to reach the same two checkers. Rebuilt from the corrected pyproject the way CI builds it, "not a uri at all" now fails with 'not a uri at all' is not a 'uri', where before it passed. The whole CI sequence is green on that venv: 4 of 4 crosswalks, 70 of 70 records, the fixture check, and 280 tests. I also found a line of my own that was wrong. The header comment of validate_crosswalks.py said the date and uri formats were enforced rather than annotated. That was true of date and false of uri. It also contradicted the schema description in the same pull request, which said the opposite and happened to be the correct one, and that contradiction is why you had to ask which of the two to believe. The comment now names the package it depends on and says what happens without it. You are right about coverage too, and it is one line, so I folded it in here. minimum: 0 on the counters. I mutation-checked that one in both directions: with a real counter set to -1 the file is refused with -1 is less than the minimum of 0, and with the schema change reverted and the same mutation still sitting there it passes. So that mutation is genuinely testing this fix and not something adjacent to it. Restored afterwards, 4 of 4 green. minimum constrains only numbers, so the prose value ave-to-ast10 carries under note_on_unmapped is untouched. That also means the follow-up issue you are opening and this dependency are the same fix. Switching validate_records.py to a format checker only means anything for date-time and uri on published, last_updated and researcher_url if the uri checker is registered, which it now is. A validator that went red on data already in the tree would not be much of a fix, so I checked ahead: all 70 records pass ave-record-1.1.0 today with the format checker fully enabled. Nothing turns red. I will take the issue when you open it. One correction, on the figure you picked out. Twelve commits over seven days is wrong. Counting records/AVE-*.json at every commit reachable from main, 31 commits carry a 59-record tree, running from 2026-07-17 to 2026-07-29. On the first-parent line alone it is 16, on five separate days. I know where twelve came from now: I ran that count inside a shallow clone. Only 21 commits were reachable there, and exactly 12 of them carried 59, so the number was true of the history I could see. What I never checked was whether I could see all of it. Unshallowed it is 31 out of 88. The mistake happens to make the argument stronger, since a generated date pins a tree even less well than I said, which is not a reason to leave it standing in the PR body. The body now has the corrected figure and a note on where the old one came from. A rev-list over records/ reproduces it. On commit being required going forward and grandfathered for the existing four: agreed on that bar, and I would like to propose the sequence that ends with the exception list not existing at all. Take this PR as it stands, with commit optional, so your four pass unchanged. Then I do the backfill as its own PR. Then commit becomes required outright, which by that point costs nothing, and the schema never has to carry a two-tier rule or a list of files allowed to skip it. If you would prefer to keep the conditional shape, the if/then on status you already use for draft records ports over cleanly and I will write it that way. The backfill is seven pins over the four files. Every repository involved resolves. I checked that against a control that correctly 404s, so I was not reading a broken path as a clean one. The AVE side is already derivable from your own history: ave-to-ast10 says 2026-07-11 with 56, and the only commit that day carrying a 56-record tree is 38b6cf0; clawscan and skillspector say 2026-07-12 with 56, and the only one that day is ae9da8b; cfgaudit says 2026-07-25 with 59, inside that 2026-07-17 to 2026-07-29 run. The three tool sides need a read of each upstream to pick the right commit. That is the part that takes an afternoon. The eighth side is ave-to-ast10's target, the OWASP AST10 site, which has no repository to pin at all, so it keeps checked_against_live_site. Each file's note will state how its commit was chosen. Agreed on gaps_in_ast10 as a separate follow-up, and I am happy to take that one whenever you want it, either as ave_ids arrays or left as prose under a name that stops implying it holds identifiers. |
|
The twelve-to-thirty-one correction is the part of this I want to respond to first, separately from the technical fixes. Finding that your own earlier number came from a shallow clone showing 21 of 88 reachable commits, and being precise about why, that you checked the count but never checked whether you could see the whole history, is a genuinely rare thing to volunteer. And you're right that the correction strengthening your own argument doesn't make leaving the wrong figure standing okay. Appreciated, specifically for that reasoning, not just the fix. On the rfc3986-validator and coverage fixes: both confirmed, both mutation-tested properly (reverting the fix and confirming the same mutation passes is the right way to prove a test actually exercises the change, not something adjacent to it), and finding the GPL issue with rfc3987 before it became a dependency problem is exactly the kind of check that's easy to skip and expensive to skip. Checked the actual diff directly too, not just the description: On sequencing for Backfill research looks solid, correctly identifying that AST10's site-only target has no commit to pin and stays on checked_against_live_site is the right judgment call, not a gap. Take the gaps_in_ast10 follow-up whenever works, your call on ave_ids arrays versus a renamed prose field. Opening the FORMAT_CHECKER issue for validate_records.py now, since you've already confirmed all 70 records pass with format checking enabled, there's no reason to sit on a fix that's already been checked safe. It's yours per your own offer. And if anything else turns up while you're in there, the backfill, gaps_in_ast10, or something neither of us has spotted yet, open it as its own PR whenever. Everything in this thread so far has come from you actually looking closely rather than from anyone asking you to, no reason for that to stop being the pattern. Nothing left open. Merging. |
2 participants
The $schema URL that every crosswalk in this repository declares, https://aveproject.org/schema/crosswalk-1.0.0.schema.json, returns 404. Records validate fine, because schema/ ships ave-record-1.0.0, ave-record-1.1.0 and ave-record, but crosswalks have nothing to validate against at all. Declaring no schema at all would arguably be safer, since a dangling reference still reads as validated. The PR adds the missing file, a validator that runs it, and a CI step to keep the four crosswalks in the tree valid. I raised it in #94 and offered to write it there; here it is.
The four files you already ship are where the schema comes from. One that failed your own tree would be no use to you. Everything all four carry became required, which is $schema, source, target, generated, note, mappings and coverage, while inside source and target I required only url. A standard carries version and record_count; a scanner carries vendor, license and a rule total; requiring either shape of the other would reject a file already in the repository. Conventions I took wholesale from ave-record-1.1.0.schema.json. That means draft 2020-12, an $id of the form https://aveproject.org/schema/-.schema.json, and snake_case throughout. Every property carries a description saying what it is for instead of restating its type. The AVE-YYYY-NNNNN pattern is your record schema's own ave_id, copied across.
Every refusal below came out of mutating a real crosswalk and re-running it. A $schema that is missing or foreign gets rejected, as does a generated value that will not parse as a date, 2026-13-45 among them. The same goes for a source that has lost its url, a target that is gone entirely, and mappings that are empty or hold an empty entry. On the identifier side it catches a malformed value under ave_id, ave_ids or primary_ave_id. Coverage has to be present and has to be an object, and a count inside it cannot be a float. Finally, a record_count that is negative or stringified fails, and so does a commit that is abbreviated or uppercase.
The validator carries two checks a schema cannot express. The first is whether the $schema a crosswalk declares is a file this repository actually ships, which is the failure described at the top. The second is whether every AVE identifier cited resolves to a record under records/. The second cannot be a pattern, so the validator looks the record up.
For shape and header style, validate_crosswalks.py follows validate_records.py closely. It differs in running Draft202012Validator with FORMAT_CHECKER enabled, which makes a date format an enforced constraint. Which raises something I noticed while I was in there and will leave with you: validate_records.py builds its validator without a format checker, so date-time and uri on published, last_updated and researcher_url are annotations nothing acts on. I have deliberately not touched it in this PR. It is a one-line change and belongs in its own PR.
Some of it is deliberately loose, and I would rather say why than leave you guessing. additionalProperties stays true at the top level, on mapping entries and on coverage, because a crosswalk is a bespoke document about one pair of taxonomies. cfgaudit-to-ave.json keys its mappings on ave_id, clawscan-to-ave.json on clawscan_module, skillspector-to-ave.json on skillspector_category, and ave-to-ast10.json on ast_id. The coverage counters vary along with them, because what is countable varies. Setting additionalProperties false, as the record schema does, would mean amending the schema for every crosswalk anyone contributes, which would make the gate a tax on contributions. Typing the shared vocabulary tightly and letting the per-pair keys through avoids that. Mapping entries are the second: they require no particular key, only that they not be empty, since no key is common to all four files at that level. The third is commit, which is optional because none of the four carries one yet.
commit is the field I would most like your read on, since it is what #94 was actually about. A record_count paired with a records_read date and no commit is not something a reader can re-derive, which is why you ended up counting upstream records by hand to establish that my number was stale, and stale in both directions at that. A date does not identify a tree. I went back over your history afterwards and counted records/AVE-*.json at every commit reachable from main. 31 commits carry a 59-record tree, running from 2026-07-17 to 2026-07-29, and 16 of those sit on the first-parent line, on five separate days. So even now my own read of 2026-07-28 cannot be pinned to anything. The schema therefore defines commit as a full 40-character sha, sitting in source or target alongside url and version, with a description of what it is for. Full length, since short shas stop being unique as a repo grows. As for the name, I looked for an existing field before choosing one and there is none anywhere in the repo. checked_against_live_site in ave-to-ast10.json is the nearest thing, and it is a date, which is the ambiguity being fixed. commit is what git calls it, and it pastes into git show unaltered. Keeping it optional is what lets this PR pass your four files unchanged. Should it be required? That depends on where you want the contribution bar, which is why I left it optional. Backfilling the existing four is an afternoon and I will do it whenever you say.
Corrected 2026-08-04: that sentence first said twelve commits spread over seven days, and it was wrong. I had run the count inside a shallow clone, which reached only 21 commits, 12 of them carrying 59, and I never checked whether the history I counted was all of it. Unshallowed it is 31 out of 88. The error runs in the direction that understates the problem, since a date pins a tree even less well than I claimed.
A couple of other things turned up while I was in there. Under gaps_in_ast10, ave-to-ast10.json puts free text in the ave_id key, specifically "all 56 records" and "AVE-2026-00052, AVE-2026-00053", so a pattern applied wherever that key name appears would reject a file of your own. I scoped the pattern to mappings and gaps, where every value today is a single well-formed identifier, and left gaps_in_ast10 untyped. Widening it is a follow-up if you would rather those became ave_ids arrays. The other: no unversioned crosswalk.schema.json alias, even though ave-record.schema.json exists as a byte-identical copy of ave-record-1.1.0.schema.json. Nothing references the unversioned crosswalk URL today, and keeping a second copy in sync by hand has the same problem as an unpinned count. The record schema does carry its unversioned twin, so consistency argues the other way. Your call.
I ran it before opening this. At tree 3b759c5 the validator returns ok for ave-to-ast10.json, cfgaudit-to-ave.json, clawscan-to-ave.json and skillspector-to-ave.json: four of four, exit 0. It also passes against the AEE crosswalk I regenerated on my side, which now carries a commit, so the optional field gets exercised. The refusals I listed came from mutating cfgaudit-to-ave.json one field at a time and confirming each specific error, with the unmutated control passing throughout. I kept the run output if you want to see the individual refusals.