You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
repo scan: L0 rules for parity with oneDAL static_check (D6) - #115
Diffed the claims from oneDAL's static_check.py (dev/agent_evals on the #3818 branch) against repo scan with oneDAL's manifest and no --static, on oneDAL main (7f6d9f8). Then ported the generic gaps into the L0 adapters:
check_fence
- starlark|bzl|bazel|build -> check_starlark+ starlark|bzl|bazel|build, or python|py that reads as BUILD -> check_starlark+ (loads a label, calls a Bazel-only native rule, or calls a .bzl macro with name =)
check_starlark
+ load("//pkg:x.bzl", "a") -> starlark_load: `a` is a public top-level name of x.bzl
check_label
- @repo//:t -> unresolved+ @repo//:t -> checked against <repo>.BUILD | BUILD.<repo> | <repo>.tpl.BUILD+ (a .bzl / MODULE.bazel / WORKSPACE must name that file)+ @<module>// -> ok, this repository
_cmake
+ -S <dir> -> cmake_src: <dir>/CMakeLists.txt (fail only after a literal cd)
shell_items
+ <placeholder> -> $_ (filled in later, never looked up, same as $VAR)
The issue also named --config names checked against .bazelrc. repo scan already does that, including .bazelrc imports. Keyword arguments are checked too, but only in Starlark-tagged fences. oneDAL's example is fenced as python, and the fence fix covers it.
oneDAL main: every claim static_check.py marks fail or unresolved
gap, ported (needs cmake in the manifest's build_systems, see below)
10 paths (detail/, backend/common.hpp, ...)
unresolved
ok_suffix / ok_basename
same
repo scan resolves more
4 others (@platforms//os:linux, a sibling-repo path, two build outputs)
unresolved
unresolved
unresolved
same
Real fails on main: oneDAL 0 of 8, repo scan 0 before and 0 after. Counting claims oneDAL makes that repo scan does not (same file, line and text): 22 before, 15 after, 13 after with cmake in build_systems. Those 13 are the 4 + 2 fails above, plus 7 that both tools check but report under a different text or line: links by URL vs path, cmake_src given as the whole command vs ., and fence labels at the fence line vs their own line.
Because main has no real fails, I seeded seven stale-guidance defects into a copy of main and ran all three:
seeded defect
oneDAL
before
after
after, cmake in manifest
def dal_module renamed in dev/bazel/dal.bzl
fail (2 calls)
no claim
fail (load symbol)
fail
@mkl//:mkl_core -> mkl_kore
fail
unresolved
fail
fail
--config=host -> hots
fail
fail
fail
fail
-DONEDAL_LINK -> ONEDAL_LNK
fail
no claim
no claim
fail
examples/oneapi/cpp/CMakeLists.txt removed
unresolved
no claim
no claim
fail (2)
"@onedal//cpp/oneapi/dal:core" -> :kore in the python fence
fail
no claim
unresolved
unresolved
make onedal_c -> onedal_cc
fail
unresolved
unresolved
unresolved
caught
6 / 7
1 / 7
3 / 7
5 / 7
The last two rows are the same oneDAL rule that gives the kmeans false positive on main: a name the build files do not spell out literally is missing. oneDAL's cpp/oneapi/dal/BUILD calls macros and its makefiles name rules from variables, so the framework says unresolved, as the contract already specifies. These, the other oneDAL-only rules (//... refused, bare cc_library/cc_test, any Starlark call outside its three .bzl files, PLAT=/COMPILER= values), and bazel test/run without --config were not ported because they are oneDAL policy. oneDAL drops them along with static_check.py, and contract §8 says so. A repository that wants such rules can still add them through its own static entry point.
To get the 5/7 column, oneDAL's repo-eval.yaml needs build_systems: [bazel, make, cmake]. Its guidance has cmake commands, and today they go unchecked. That one-line change belongs to #3818.
Evidence
Before (intel/main code, new test file tests/test_repo_l0_parity.py): 13 failed, 3 passed. The 3 that pass are negative guards: a plain-Python fence, the bazel adapter off, and a load of a missing .bzl, which stays the label's claim.
python fence with a load -> {} expected m(srsc=) fail, //a:kore fail, load ok
external repository BUILD file -> @zlib//:zlb unresolved expected fail
cmake -S after a cd -> no cmake_src claim
placeholder in a label -> ("bazel_label", "//pkg/") unresolved
After: 16 passed. Full suite pytest -q -n auto: 5194 passed, 2 skipped. ruff check ., mypy, dokimi.schema_check, config_check, mkdocs build --strict, and pre-commit on the changed files all pass.
CI self-scan (benchmarks/self-scan.repo-eval.yaml, gated): 258 claims, 0 fail, the same as before. Stress run over every Markdown file in this repository with all five adapters on: no new fail, and one false fail removed (pre-commit run <hook> used to look up > as a hook id).
radon: the new functions are all CC ≤ 14. check_label (20) and _bazel (16) are unchanged, _cmake drops from 17 to 14, and Scan._chdir / resolve_path each drop by 1 (shared ancestors()).
Merge Danger
Door: two-way. Only code is involved: no schema change, no new flag, no persisted format.
Blast Radius: scan-verdicts
repo scan reports more claims: new kinds starlark_load and cmake_src, plus labels and macros from python fences. A guidance file with a stale load symbol, a wrong @repo//:t whose BUILD file is in the tree, or cmake -S into a directory without CMakeLists.txt after a cd now fails a --fail-on-fail gate where it used to pass.
Words with <placeholder> in shell commands are no longer looked up. A few unresolved or fail claims on > or cut labels disappear.
Diffing the claims oneDAL's static_check.py and `repo scan` make on oneDAL
main showed the framework missed a BUILD snippet fenced as `python`
(its macros, keyword arguments and labels), external-repository labels
whose BUILD file is in the tree, and `cmake -S`. Ported as generic rules:
- a `python` fence that reads as BUILD code is checked as Starlark
- `load()` symbols against the public names of the in-tree .bzl
- `@repo//:t` against `<repo>.BUILD`, `BUILD.<repo>`, `<repo>.tpl.BUILD`
when a .bzl, MODULE.bazel or WORKSPACE names that file
- `@<module>//` alone is this repository
- `cmake -S <dir>` needs a CMakeLists.txt there (fail only after a cd)
- a `<placeholder>` in a command is a word to fill in, not redirections
The `--config` check the issue named was already there. oneDAL's other
fails on main are its own policy or false positives (contract §8).
Part of intel#41 (step D6).
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
❌ Patch coverage is 97.68786% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.36%. Comparing base (6e695ce) to head (e86fdbe). ⚠️ Report is 3 commits behind head on main.
Contract §8 said oneDAL's policy rules stay in its static entry point.
oneDAL deletes static_check.py instead, so say the rules were not ported
because they are repository policy and that oneDAL dropped them; a
repository can still add such rules through its own static entry point.
Part of intel#41 (step D6).
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
External labels: :all and :all-targets select the package, exports_files
names and names the BUILD file uses as strings count, and a BUILD file
belongs to a repository only when a .bzl, MODULE.bazel or WORKSPACE string
names its path (a comment does not; lib_tools.BUILD is not @lib's).
load() reads names an unpacking assignment binds; other binding forms
make a missing name unresolved. cmake -S follows directory links and is
unresolved inside a submodule.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Indented Markdown fences are skipped during Starlark analysis
dokimi/dokimi/repos/l0/core.py:449
Python fences nested in Markdown lists retain their leading spaces in Fence.lines. For example, load("//:defs.bzl", "gone") matches neither BUILD-detection regex, so the entire fence is skipped, including its missing symbols and stale labels. Remove common indentation before both BUILD detection and Starlark checking, preserving the original line numbers. Add a regression test for a list-nested fence.
- exports_files: parse only positional/srcs argument, not keyword args; mark calls inside exports_files (like glob) as computed
- _bzl_public: mask strings/comments before extracting public names to avoid matching docstring examples
Fixes PRs intel#115 threads at lines 182 and 436.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The opening paren of the exports_files(...) call itself should not be counted in
exports_depth; only nested brackets/parens within the argument should increment
it. This fixes LICENSE not being recognized in exports_files(["LICENSE"]).
Fixesintel#115 CI tests for all Python versions.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e86fdbe: the label pass now walks _TOKEN strings, so single-quoted load('//:gone.bzl', ...) gives the missing-file claim in python and starlark fences; tests added.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e86fdbe: exports_files now takes its first argument as a unit and collects every string of a plain list; test with ["LICENSE", "NOTICE"] (@zlib//:NOTICE ok).
The reason will be displayed to describe this comment to others. Learn more.
Review at e8eab98. The rules are well scoped and the oneDAL comparison is useful. Two of the rules give wrong verdicts on inputs the PR newly routes into Starlark checking, and the tests for the new exports_files and BUILD-file association code let real regressions through. I would fix those before merge.
Reproductions for the code findings, each as a pytest case in the style of test_repo_l0_parity.py.
A mutation run on the new code: 105 mutants, 49 of which pass test_repo_l0.py, test_repo_l0_parity.py and test_repo_l0_cases.py.
I agree with @copilot-pull-request-reviewer's points on single-quoted loads, the exports_files comma, template names, a glob in cmake -S, indented fences and the escaped-backslash quote; I reproduced each at this head.
Checked, no issues
Placeholders:cat <file>, foo <in >out, make nosuch 2>&1 | tee <log> still report nosuch as fail, and -j<N>, --config=<name> and //a/<b> are skipped as dynamic.
load():alias = "name", multi-line calls, @case// and @@case// labels, and a symbol the .bzl itself loads were checked with the expected verdicts.
cmake -S:-S src, -Ssrc, -S ./src/, -S $SRC, a trailing bare -S, and the ancestor fallback from a nested guidance file were checked.
Repo rules: the changelog fragment name and sections follow the existing ones, the new test file carries the SPDX header, no schema file changed, and no generated artifact or lock file is committed.
Parse the first exports_files argument as a unit (lists, globs, srcs=),
treat %{...} template names as computed, mask .bzl strings and comments
from _TOKEN, check labels of every string with _is_label, narrow
reads_as_build to load() of a label or a Bazel-only rule, skip glob
arguments of cmake -S, and pin the BUILD name forms with full claim sets.
Complete the contract's conditions.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
The reason will be displayed to describe this comment to others. Learn more.
Review at e86fdbe. The five earlier threads are fixed: the Python-fence trigger is narrowed to load( of a label and Bazel-only natives, exports_files takes its first argument as a unit, the .bzl masking uses _TOKEN, the contract matches _check_external, and the new cases compare full claim sets. What is left is at the edges of the new cmake -S and template code. Two risks in the code and two test gaps are worth fixing before merge.
Reproductions for the code findings, with the LLM transport mocked and no network.
A mutation run on the new code: 119 mutants, 57 of which pass test_repo_l0.py, test_repo_l0_parity.py and test_repo_l0_cases.py (about 8 of those are equivalent or dead code).
Checked, no issues
Earlier fixes: a python fence with test_suite(, or with run( when a .bzl defines run, gets no claims. exports_files(glob(["*"])) is unresolved. SEP = "\\" followed by def m loads. "//cdn.example.com/x" gets no claim.
exports_files:visibility= after the list, a trailing comma, a comment in the list, a second call and native.exports_files behave; a reordered srcs=, a tuple and + glob(...) are unresolved.
Line mapping:fence.lines[code.count("\n", 0, m.start())] gives the right line after a multi-line string, in a CRLF file and with a form feed in the fence.
cmake -S:./.. normalisation, a symlinked cd target, cd .., cd -P, ; and && chains, a scoped subshell cd, and no leak of a cd into the next fence.
Repo rules: one changelog fragment, no new docs page, no generated artifact, no schema change.
The reason will be displayed to describe this comment to others. Learn more.
cmake -S with a placeholder the fill-in does not cover, or a non-tree path, fails after a cd.
The guard skips /, ~, - and glob characters, and dynamic(src). The new $_ rule (core.py, _FILL_IN) only matches <[A-Za-z_][\w.-]*>, so these still reach the lookup.
cd nocm then cmake -S <source dir> -B b: claim cmake_src 'dir' fail.
cd nocm then cmake -S C:/x/y -B b: claim cmake_src 'C:/x/y' fail.
The PR description says placeholders are no longer looked up, and -S <dir> is handled, so a fail at a --fail-on-fail gate for these is a new false fail. The redirect and <...> handling in shell_items runs before _FILL_IN.
Suggestion: widen _FILL_IN to any <...> span without a newline (before _REDIRECT sees it), and skip -S values with a drive prefix (^[A-Za-z]:), % or a backslash, as resolve_path already does for %. Add one case each to test_repo_l0_parity.py.
The reason will be displayed to describe this comment to others. Learn more.
A template with a bare %{...} placeholder still gives fail, though the contract says a template's %{...} gives unresolved.
Only a %{ inside a collected name string marks the file computed. A line that injects whole targets does not.
Case: tpl.tpl.BUILD = cc_library(name = "a") plus a line %{extra_targets}; a .bzl names it as template of repository tpl; guidance says `bazel build @tpl//:b`. Result: bazel_label @tpl//:b fail. Expected unresolved. A zlib.tpl.BUILD that is only %{rules} gives fail for @zlib//:zzz too.
Suggestion: treat any %{ in the file text as computed, not only in names (computed or "%{" in text), and add the bare-placeholder case next to the %{target_name} one.
The reason will be displayed to describe this comment to others. Learn more.
The reads_as_build triggers are barely pinned by tests.
Mutation check, each passes the full L0 suite (154 tests):
delete any one of load, package, glob, select, alias, module, platform, toolchain, workspace, licenses, exports_files, package_group from the exclusion set (12 mutants; only test_suite is caught);
cut _BUILD_ONLY to {cc_library};
drop re.M from _LOAD_LABEL or from the ^(\w+)\( scan, so a BUILD fence that starts with a comment or an import line is no longer recognised.
With an excluded name removed, plain Python such as glob(...) or select(...) at line start routes a python fence into Starlark checking again, which is the false-fail class from the earlier thread.
Suggestion: parametrise "python fence calling X( is plain Python" over the excluded names, add "python fence with X( reads as BUILD" for http_archive, genrule, py_test, config_setting, and a case whose first line is # BUILD before cc_library(.
The reason will be displayed to describe this comment to others. Learn more.
The glob guard and the label-string forms are tested for one case each.
Mutation check, all pass the full L0 suite:
removing ?, [ or { from "*?[{" in _cmake_source (only * is caught); cd . && cmake -S examples/{a,b} then gives a false fail;
dropping the /, ~ or - prefix guard in _cmake_source;
in the check_starlark label loop: no lstrip("rRbB") (a r"//a:kore" is not checked), no triple-quote slice ('''//a:kore'''), and fence.lines[0] instead of the string's own line.
Suggestion: parametrise this case over ?, [ab] and {a,b}; add /abs, ~/x, -Wno-dev; add a bazel_label case in r"", b"" and ''' form that also asserts the claim's line.
of those arguments (``"t_" + s``, ``"t_%s" % s``, a variable), a call at the start of a line, or in a comprehension
Spelled out: ``name = "x"`` or ``out = "x"`` (either quote), ``outs = ["x.h", ...]``, the files of a top-level
``exports_files([...])`` (every name of a plain list). Computed: any other value
of those arguments, ``exports_files(glob(...))`` and the like, (``"t_" + s``, ``"t_%s" % s``, a variable), a call at the start of a line, or in a comprehension
The reason will be displayed to describe this comment to others. Learn more.
Nit: the _build_names docstring has a stray comma and unbalanced parentheses: exports_files(glob(...))` and the like, (`"t_" + s`...). Put the exports_files(glob(...)) example inside the list of computed forms and wrap the lines.
This branch has not been deployed
No deployments
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
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.
antonwolfy
left a comment
Part of #41 (step D6).
Summary
Diffed the claims from oneDAL's
static_check.py(dev/agent_evalson the #3818 branch) againstrepo scanwith oneDAL's manifest and no--static, on oneDALmain(7f6d9f8). Then ported the generic gaps into the L0 adapters:The issue also named
--confignames checked against.bazelrc.repo scanalready does that, including.bazelrcimports. Keyword arguments are checked too, but only in Starlark-tagged fences. oneDAL's example is fenced aspython, and the fence fix covers it.oneDAL
main: every claimstatic_check.pymarksfailorunresolvedrepo scanbeforecpp/AGENTS.md:15CamelCase,snake_case;cpp/daal/AGENTS.md:12;cpp/oneapi/AGENTS.md:10(4)docs/AGENTS.md:22make html(aftercd docs)docs/Makefileexamples/AGENTS.md:21//examples/oneapi/cpp:kmeansdal_algo_example_suite(algos = [..., "kmeans", ...])makes thekmeanstest suiteexamples/AGENTS.md:21,22bazel testwithout--config(2)dev/bazel/AGENTS.md:58dal_test_suite(hdrs= / dal_deps= / framework=)(3)pythoncpp/oneapi/AGENTS.md:41,dev/AGENTS.md:47cmake -S .(2)cmakein the manifest'sbuild_systems, see below)detail/,backend/common.hpp, ...)repo scanresolves more@platforms//os:linux, a sibling-repo path, two build outputs)Real fails on
main: oneDAL 0 of 8,repo scan0 before and 0 after. Counting claims oneDAL makes thatrepo scandoes not (same file, line and text): 22 before, 15 after, 13 after withcmakeinbuild_systems. Those 13 are the 4 + 2 fails above, plus 7 that both tools check but report under a different text or line: links by URL vs path,cmake_srcgiven as the whole command vs., and fence labels at the fence line vs their own line.Because
mainhas no real fails, I seeded seven stale-guidance defects into a copy ofmainand ran all three:cmakein manifestdef dal_modulerenamed indev/bazel/dal.bzlloadsymbol)@mkl//:mkl_core->mkl_kore--config=host->hots-DONEDAL_LINK->ONEDAL_LNKexamples/oneapi/cpp/CMakeLists.txtremoved"@onedal//cpp/oneapi/dal:core"->:korein thepythonfencemake onedal_c->onedal_ccThe last two rows are the same oneDAL rule that gives the
kmeansfalse positive onmain: a name the build files do not spell out literally is missing. oneDAL'scpp/oneapi/dal/BUILDcalls macros and its makefiles name rules from variables, so the framework saysunresolved, as the contract already specifies. These, the other oneDAL-only rules (//...refused, barecc_library/cc_test, any Starlark call outside its three.bzlfiles,PLAT=/COMPILER=values), andbazel test/runwithout--configwere not ported because they are oneDAL policy. oneDAL drops them along withstatic_check.py, and contract §8 says so. A repository that wants such rules can still add them through its ownstaticentry point.To get the 5/7 column, oneDAL's
repo-eval.yamlneedsbuild_systems: [bazel, make, cmake]. Its guidance hascmakecommands, and today they go unchecked. That one-line change belongs to #3818.Evidence
intel/maincode, new test filetests/test_repo_l0_parity.py): 13 failed, 3 passed. The 3 that pass are negative guards: a plain-Python fence, thebazeladapter off, and aloadof a missing.bzl, which stays the label's claim.pytest -q -n auto: 5194 passed, 2 skipped.ruff check .,mypy,dokimi.schema_check,config_check,mkdocs build --strict, and pre-commit on the changed files all pass.benchmarks/self-scan.repo-eval.yaml, gated): 258 claims, 0 fail, the same as before. Stress run over every Markdown file in this repository with all five adapters on: no new fail, and one false fail removed (pre-commit run <hook>used to look up>as a hook id).check_label(20) and_bazel(16) are unchanged,_cmakedrops from 17 to 14, andScan._chdir/resolve_patheach drop by 1 (sharedancestors()).Merge Danger
Door: two-way. Only code is involved: no schema change, no new flag, no persisted format.
Blast Radius: scan-verdicts
repo scanreports more claims: new kindsstarlark_loadandcmake_src, plus labels and macros frompythonfences. A guidance file with a staleloadsymbol, a wrong@repo//:twhose BUILD file is in the tree, orcmake -Sinto a directory withoutCMakeLists.txtafter acdnow fails a--fail-on-failgate where it used to pass.<placeholder>in shell commands are no longer looked up. A fewunresolvedorfailclaims on>or cut labels disappear.l0/adapters.pylines it changes (around 51–58) are untouched here, and the contract edits stop before the last paragraph of §8.Checklist
pytestpassesruff,mypy, and schema checks pass.github/.licenserc.yaml)🤖 Generated with Claude Code