Skip to content

feat(repo): dokimi:typecheck grader and repo scan --typecheck for Python fences - #337

Open
napetrov wants to merge 47 commits into
intel:mainfrom
napetrov:ce-typecheck
Open

napetrov wants to merge 47 commits into
intel:mainfrom
napetrov:ce-typecheck

Conversation

@napetrov

@napetrov napetrov commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two additions that check guidance against code, both opt-in.

dokimi:typecheck (built-in grader, workspace files)
  grade: mypy (pinned in the image) over the listed files
    pass: no error in the listed files; missing listed file fails

repo scan --typecheck [mypy|basedpyright] [--python-env PY]   (L0)
  each python / py / python3 fence in the guidance files
    unparsable or too deep to parse  -> unresolved (not checked)
    no --python-env                  -> unresolved (unknown)
    tool reports an error            -> fail (stale-API candidate)
    otherwise                        -> ok

Files: repos/graders/typecheck.py (new), repos/l0/typecheck.py (new), repos/l0/__init__.py, commands/repo.py, repos/manifest.py, repos/graders/{__init__,main,review}.py, schemas/repo_readiness.v1.json (optional typecheck block), docs/repo-evaluation-contract.md (§4.5, §8.2), changelog fragment, deptry exemption, CLI allowlist and snapshot.

Since-cutoff: the contract already splits results by base commit date against the model's cutoff, so a stale-API fail found here can be checked against that split.

The notes from these checks can feed patch:<file> arms (contract §3), so a guidance fix found by the fence check can be measured as its own arm.

Evidence

  • Before: repo scan has no Python-fence check; a stale call in an AGENTS.md fence is not reported.
    After: repo scan <tree> --typecheck --python-env <venv> reports the fence (test test_mypy_against_an_interpreter_passes_the_good_fence_and_fails_the_stale_one).
  • make ci-local on d96d7db8: 0 failed, 30 passed, 7 skipped; full suite 12064 passed, 7 skipped; rc 0.
  • Diff coverage ≥80% (97% on the feature commits).
  • Growth ratchet: cmd_repo_scan complexity 14 → 7 (text output moved to _print_scan_text); its baseline entry is removed.
  • Self-review (pr-review local, L2, passes: 2): blocking 0. Marker on 057ad88a.

Merge Danger

Door: two-way. The grader arm and the CLI flag are new; reverting removes them with no data migration. The schema change is an optional field (oneOf null/object), so existing reports stay valid.

Reviewer points

  • pyproject.toml: DEP004 = ["mypy"]. repos/graders/typecheck.py imports mypy inside the grading image, where it is installed for the grader; the wheel does not ship it and the host never imports it (the L0 check runs it as a subprocess). The reason comment is in the file; a reviewer may prefer a different mechanism.
  • basedpyright is host-only: it must be on PATH and is recorded with the version it reports. dokimi does not pin it (contract §8.2). --python-env without --typecheck is refused.
  • Known limitation: --typecheck takes an optional value (nargs="?"), so a source placed right after it is read as the value and the command exits 2 with invalid choice. Put the source first: repo scan . --typecheck.
  • CI: one run on a6a15234 had a timing failure, tests/test_repo_environment.py::test_unbalanced_quote_in_hook_completes_quickly (1.33 s against a 1 s limit). It passes alone and was not touched; the next run on d96d7db8 was clean (above).
  • Changelog fragment written by hand; scriv is not installed on the host.
  • Self-review: two L2 passes. Commits after pass 2 (d96d7db8, 057ad88a) were checked by targeted probes and tests, not by a third pass.

Blast Radius: low. Affects only repo scan --typecheck and dokimi:typecheck; repos that do not use either see no change. A tool-run failure (not a type error) is a usage error (exit 2), not a silent pass.

Behaviour change

  • dokimi:typecheck refuses silenced mypy: a # type: ignore or # mypy: in a listed file fails the grade, including one that was already there (no exemption for pre-existing comments).
  • A .pyi anywhere in the workspace is refused as a suppression, since it would replace the module it names.

Both are deliberate (maintainer decision). A repository that relies on either sees a failing grade.

🤖 Generated with Claude Code

dokimi:typecheck passes when the task's listed workspace files have no mypy
error, under the image's pinned mypy version. repo scan --typecheck checks each
Python fence of the guidance against --python-env: a failing fence is a
stale-API candidate, an unparsable fence or a scan without an interpreter is
unresolved. The report gains an optional typecheck block.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Self-review of the typecheck grader and repo scan --typecheck: a missing mypy
(exit 1, no report) no longer passes every fence; truncated tool output is an
error; a fence-less scan runs no tool; a relative --python-env is made absolute;
the grader runs mypy from a neutral directory so a workspace mypy.ini cannot
suppress a listed file; the error parser accepts paths with a colon and
line-less errors.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…error is the fence line

ast.parse can raise RecursionError or MemoryError on a deeply nested fence;
that aborted repo scan with a traceback. Such a fence is now unresolved like
any other unparsable one. mypy writes line -1 for an error with no line; that
was added to the fence line, pointing one line above the fence. It now maps
to the fence line.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…cases

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Copilot AI balanced review requested due to automatic review settings November 1, 2026 02:08
@napetrov
napetrov requested a review from a team as a code owner November 1, 2026 02:08
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

CLI interface change detected

This PR changes dokimi's user-facing CLI surface (commands and/or options):

~ `dokimi repo scan` parameters changed:
    + added option `--python-env`
    + added option `--typecheck`
    + added option `--typecheck-tool`

Updated on every push; the cli-interface-change label is on while the diff is non-empty.

@github-actions github-actions Bot added the cli-interface-change Changes dokimi's user-facing CLI surface label Nov 1, 2026
@exai-bot

exai-bot commented Nov 1, 2026 •

Copy link
Copy Markdown

OpenCodeReview: Commit f7cb3c5 was not reviewed. This PR has reached its review limit (6 reviews).

To review this commit: re-request review from exai-bot in the Reviewers list.

Last review (7f5bdde): 3 high, 1 medium, 5 low

Open threads from the bot: 10

Comment thread dokimi/dokimi/commands/repo.py Outdated
"the report, its claims replacing the framework's on the same file, line and text. "
"A file that cannot be read or is not JSON exits 2; a schema-invalid document is "
"reported and makes the command exit 1")
scan_p.add_argument("--typecheck", nargs="?", const="mypy", default=None, choices=("mypy", "basedpyright"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: python cli.py repo scan --typecheck . exits 2 because nargs="?" consumes . as the checker name instead of leaving it as the required source. The bare flag therefore works only in some argument orders; the new test covers only source-first usage. Make the bare flag unambiguous (for example, use a boolean --typecheck with a separate checker-selection option) and test it both before and after the source.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cb07efb. --typecheck is now a plain flag; --typecheck-tool {mypy,basedpyright} picks the tool (default mypy) and needs --typecheck. So repo scan --typecheck . reads . as the source. CLI snapshot, flag allowlist and contract §8.2 follow.


if mypy.version.__version__ != task["version"]:
raise ValueError(f"the image's mypy is {mypy.version.__version__}, the task pins {task['version']}")
workspace = Path(run_dir or "/eval/run") / "workspace"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: resolve the workspace to an absolute path before changing directories. The script accepts a relative run directory, e.g. main.py grade typecheck tasks/typed run. With an existing run/workspace/pkg/api.py, the existence check succeeds in the original directory, but api.run subsequently resolves run/workspace/pkg/api.py under the empty temporary directory and exits 2. The entry point therefore writes pass: null for an otherwise valid run. Add a regression test using a relative run_dir.

Suggestion:

Suggested change
workspace = Path(run_dir or "/eval/run") / "workspace"
workspace = (Path(run_dir or "/eval/run") / "workspace").resolve()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05255f9. The workspace is resolved to an absolute path before anything runs, and the grader no longer changes directory, so a relative run directory works for mypy. Covered in tests/test_repo_typecheck_grader.py.

Comment on lines +89 to +90
stdout, stderr, status = api.run(["--no-incremental", f"--cache-dir={cache}",
*[str(workspace / p) for p in present]])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: changing to an empty directory removes the workspace root from mypy's import search path. For a workspace containing pkg/api.py with from pkg.other import broken and pkg/other.py, but no pkg/__init__.py (a valid namespace package), mypy searches from the listed file's directory rather than the workspace root and reports import-not-found on pkg/api.py. This falsely fails valid code; the current import test only covers regular packages. Explicitly include the workspace root in mypy's search path while retaining config isolation, and add a namespace-package regression test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05255f9. No chdir any more; the neutral config sets mypy_path to the workspace root, so pkg.other resolves from a namespace package. Covered in tests/test_repo_typecheck_grader.py.

Comment on lines +95 to +96
if status not in (0, 1): # 2: mypy itself failed (a bad config, a crash)
raise RuntimeError(f"mypy exited {status}: {stderr[-300:]}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: mypy also returns status 2 for blocking source errors, not just tool failures. For example, a listed pkg/api.py containing def broken(: emits an [syntax] error on stdout and exits 2; this branch turns it into pass: null with no measured errors instead of the required pass: false. Distinguish source diagnostics from invocation/internal failures and add a syntax-error regression test (including a blocking error in an imported module).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05255f9. Exit 2 is a diagnostic when mypy printed an : error: line; only a 2 with no diagnostic is treated as a tool failure. A syntax error in a listed file now fails the grade with its message. Covered in tests/test_repo_typecheck_grader.py.

Comment on lines +104 to +107
def oracle(task: dict[str, Any], task_dir: Path) -> str:
"""No reference answer: the grade reads the workspace only."""
del task, task_dir
return ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · high
Must-fix: this no-op oracle makes every typecheck task fail repo tasks check. selfcheck._check_step prepares identical fresh workspaces for null and oracle checks, and grading.run_entry selects the built-in oracle even when the repository declares its own. Since grading ignores answer.txt, both checks necessarily produce the same result: a broken base fails the oracle check, while a clean base fails the null check. Provide a way to apply a reference workspace repair (or delegate the oracle to the repository) and cover the null/oracle self-check end to end.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05255f9. oracle copies the task's reference/ directory over the workspace, so the reference repair is what gets graded (contract §4.5). Tasks without reference/ keep the no-op oracle and cannot pass the self-check until they add one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer decision: reference/ is optional. With it, the oracle copies it and its repair is what gets graded. Without it, the listed files are graded as they are, and the self-check runs the null step only (the untouched files must fail). Code, §4.3/§4.5 and the changelog match, in 75f338f.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Path(tmp, f"fence_{i}.py").write_text(code + "\n", encoding="utf-8")
names.append(f"fence_{i}.py")
if check.tool == "mypy":
argv = [sys.executable, "-m", "mypy", "--python-executable", str(check.python),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: --python-executable selects the interpreter used to discover installed packages, but does not set mypy's target Python version; that still defaults to the host interpreter. For example, with dokimi running on Python 3.12 and --python-env pointing to Python 3.11, a fence containing from itertools import batched can be marked ok, although that import fails in the repository environment. Query the repository interpreter's version and pass it as --python-version, with a regression test using different host/target versions. The earlier ast.parse(code) also uses the host grammar, so valid newer-target syntax (such as type Alias = int on a 3.12 target with a 3.11 host) is incorrectly left unresolved; align that validation with the target as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eb86685. Fences are parsed with ast.parse(feature_version=...) for the repository interpreter's version, read with -I, and mypy gets --python-version X.Y from the same value.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +119 to +121
argv = [sys.executable, "-m", "mypy", "--python-executable", str(check.python),
"--follow-imports=silent", "--no-incremental", "--cache-dir", os.path.join(tmp, "cache"),
"--output", "json", *names]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: running in a temporary directory does not disable mypy's user-level configuration discovery. For example, a host ~/.mypy.ini containing [mypy]\nignore_errors = True makes the stale json.dumps(1).nosuch() fence report ok; user settings such as ignore_missing_imports also change the promised verdicts. Pass an explicit empty/controlled --config-file so fence results do not inherit unrelated host policy, and add a regression test with a user config that suppresses errors.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eb86685. mypy runs with --config-file pointing at an empty [mypy] file, so host and subject configs (~/.mypy.ini, ~/.config/mypy/config, pyproject) are not read.

Comment on lines +120 to +121
"--follow-imports=silent", "--no-incremental", "--cache-dir", os.path.join(tmp, "cache"),
"--output", "json", *names]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: mypy skips bodies of unannotated functions by default, so a Python fence containing import json\ndef example():\n json.dumps(1).nosuch() is reported as ok, even though the same stale call at module scope fails. Guidance examples commonly use unannotated functions. Add --check-untyped-defs to this invocation and a regression test with the stale call inside an unannotated function so those bodies are actually checked.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eb86685. --check-untyped-defs is on, so the bodies of unannotated functions are checked and the stale call in your example now fails.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical flaws allow module shadowing to execute checkout code and host configuration to suppress diagnostics, while broken oracle checks and error handling prevent reliable grading.

5 open issues
What changed in this PR

5 new findings.

This PR adds opt-in type checking to repository evaluation: a workspace-file grader and a Python-fence check for guidance documents.

Changes:

  • Registers dokimi:typecheck with task validation and grading metrics.
  • Adds repo scan --typecheck and --python-env, with fence claims and report metadata.
  • Documents both features and adds tests and CLI reference updates.
File Description
dokimi/​tests/​test_repo_typecheck_grader.py Tests grader results, validation, and entry-point integration.
dokimi/​tests/​test_repo_l0_typecheck.py Tests fence checks, CLI behavior, and tool failures.
dokimi/​tests/​data/​cli_flags_allowlist.txt Allows the two new CLI options.
dokimi/​scripts/​growth_baseline.json Removes the refactored scan command’s complexity exemption.
dokimi/​pyproject.toml Exempts the grader’s runtime mypy import from dependency checks.
dokimi/​dokimi/​schemas/​repo_readiness.v1.json Adds optional typecheck metadata.
dokimi/​dokimi/​repos/​manifest.py Validates typecheck task fields.
dokimi/​dokimi/​repos/​l0/​typecheck.py Extracts Python fences and runs type checkers.
dokimi/​dokimi/​repos/​l0/​__init__.py Integrates fence checks into readiness reports.
dokimi/​dokimi/​repos/​graders/​typecheck.py Implements the workspace-file mypy grader.
dokimi/​dokimi/​repos/​graders/​review.py Accepts the shared run-directory argument.
dokimi/​dokimi/​repos/​graders/​main.py Dispatches the new grader and supplies its run directory.
dokimi/​dokimi/​repos/​graders/​__init__.py Registers the grader and task validator.
dokimi/​dokimi/​commands/​repo.py Adds scan options, validation, and text reporting.
dokimi/​docs/​repo-evaluation-contract.md Defines grader and fence-check behavior.
dokimi/​docs/​reference/​cli-interface.json Records the new CLI options.
dokimi/​changelog.d/​20261031_120000_napetrov_repo-typecheck.md Announces both opt-in features.

Review effort: Balanced

Comment on lines +95 to +97
if status not in (0, 1): # 2: mypy itself failed (a bad config, a crash)
raise RuntimeError(f"mypy exited {status}: {stderr[-300:]}")
errors = _errors(stdout, set(present), str(workspace) + os.sep)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05255f9. A blocking diagnostic (a syntax error, a duplicate module) is a normal result: pass false with n_errors set, not pass: null. Only a tool failure (another exit code, or a 2 with no diagnostic) raises.

Comment on lines +104 to +107
def oracle(task: dict[str, Any], task_dir: Path) -> str:
"""No reference answer: the grade reads the workspace only."""
del task, task_dir
return ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05255f9. With reference/ present, the oracle run grades the reference repair and the null run grades the broken workspace, so the two checks differ. Covered in tests/test_repo_typecheck_grader.py.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer decision: reference/ is optional. With it, the oracle copies it and its repair is what gets graded. Without it, the listed files are graded as they are, and the self-check runs the null step only (the untouched files must fail). Code, §4.3/§4.5 and the changelog match, in 75f338f.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated

def tool_version(check: Check) -> str:
"""The version the tool reports, e.g. ``2.4.0``."""
argv = [sys.executable, "-m", "mypy", "--version"] if check.tool == "mypy" else [check.tool, "--version"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eb86685. Both the version probe and the mypy run use python -I with cwd outside the checkout, so a mypy.py in the repository cannot shadow the installed mypy. Covered by the L0 typecheck tests.

Comment on lines +118 to +121
if check.tool == "mypy":
argv = [sys.executable, "-m", "mypy", "--python-executable", str(check.python),
"--follow-imports=silent", "--no-incremental", "--cache-dir", os.path.join(tmp, "cache"),
"--output", "json", *names]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eb86685. mypy is given --config-file pointing at an empty file, so ~/.config/mypy/config and ~/.mypy.ini are not read. Covered by the L0 typecheck tests.


if mypy.version.__version__ != task["version"]:
raise ValueError(f"the image's mypy is {mypy.version.__version__}, the task pins {task['version']}")
workspace = Path(run_dir or "/eval/run") / "workspace"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05255f9. The workspace is resolved before use, so mypy gets absolute paths for the files and for mypy_path, and nothing changes directory. Same fix as the relative run directory thread above.

@codecov-commenter

codecov-commenter commented Nov 1, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.48819% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.47%. Comparing base (1cc9663) to head (f7cb3c5).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
dokimi/dokimi/repos/l0/typecheck.py 90.99% 11 Missing and 8 partials ⚠️
dokimi/dokimi/repos/graders/typecheck.py 96.87% 3 Missing and 3 partials ⚠️
dokimi/dokimi/commands/repo_scan.py 93.61% 2 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #337      +/-   ##
==========================================
+ Coverage   94.37%   94.47%   +0.09%     
==========================================
  Files         312      319       +7     
  Lines       40068    40951     +883     
  Branches     6801     6956     +155     
==========================================
+ Hits        37816    38688     +872     
- Misses       1436     1440       +4     
- Partials      816      823       +7     
Files with missing lines Coverage Δ
dokimi/dokimi/commands/repo.py 96.78% <100.00%> (+0.56%) ⬆️
dokimi/dokimi/repos/graders/__init__.py 100.00% <100.00%> (ø)
dokimi/dokimi/repos/graders/main.py 82.08% <100.00%> (+10.87%) ⬆️
dokimi/dokimi/repos/graders/review.py 100.00% <100.00%> (ø)
dokimi/dokimi/repos/l0/__init__.py 100.00% <100.00%> (ø)
dokimi/dokimi/repos/manifest.py 94.48% <100.00%> (+0.14%) ⬆️
dokimi/dokimi/repos/selfcheck.py 96.58% <100.00%> (+0.62%) ⬆️
dokimi/dokimi/commands/repo_scan.py 93.61% <93.61%> (ø)
dokimi/dokimi/repos/graders/typecheck.py 96.87% <96.87%> (ø)
dokimi/dokimi/repos/l0/typecheck.py 90.99% <90.99%> (ø)

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mypy runs with the workspace absolute, under an explicit neutral config
whose mypy_path is the workspace root, and without changing directory,
so a relative run directory and a namespace package both resolve.
A blocking error (exit 2 with a diagnostic) is a result, not a tool
failure; when mypy stops before the listed files, that error fails the
grade. A task's reference/ directory is copied over the workspace by
oracle, so a reference repair is what the self-check grades.

The typecheck tests need mypy on the test interpreter: it is added to
requirements-test.txt (the Tests (py3.11) and (py3.14) legs failed).

Thread: PRRT_kwDOVBg7g86yWj1M
Thread: PRRT_kwDOVBg7g86yWj1Q
Thread: PRRT_kwDOVBg7g86yWj1T
Thread: PRRT_kwDOVBg7g86yWj1V
Thread: PRRT_kwDOVBg7g86yWj9q
Thread: PRRT_kwDOVBg7g86yWj90
Thread: PRRT_kwDOVBg7g86yWj-A
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Fences are parsed with the grammar of the repository interpreter's
version, and mypy runs with that --python-version, an explicit empty
config (no subject or host mypy settings), --check-untyped-defs, and
-I, so a module in the checkout cannot stand in for mypy in the version
probe or the run.

Thread: PRRT_kwDOVBg7g86yWj1X
Thread: PRRT_kwDOVBg7g86yWj1Y
Thread: PRRT_kwDOVBg7g86yWj1Z
Thread: PRRT_kwDOVBg7g86yWj94
Thread: PRRT_kwDOVBg7g86yWj97
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…e tool

A bare --typecheck with nargs="?" read the source after it as the
checker name, so `repo scan --typecheck .` exited 2. --typecheck is now
a flag and --typecheck-tool {mypy,basedpyright} selects the tool (default
mypy); either needs --typecheck. The CLI snapshot, the flag allowlist,
the contract and the changelog fragment follow.

Thread: PRRT_kwDOVBg7g86yWj1K
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Comment thread dokimi/dokimi/commands/repo.py Outdated
Comment on lines +140 to +142
if args.python_env is not None and not os.access(args.python_env, os.X_OK):
print(f"✗ --python-env {args.python_env}: not an executable interpreter", file=sys.stderr)
sys.exit(EXIT_USAGE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: executable permission does not establish that this is a Python interpreter. On POSIX, scanning guidance containing a Python fence with --typecheck --python-env /bin/true passes this check; the version probe exits 0 with empty stdout, and python_version() raises ValueError while unpacking the version. cmd_repo_scan catches only TypecheckError, so this invalid CLI input produces a traceback instead of the intended usage diagnostic and exit 2. Validate the probe's output and convert malformed/non-Python version responses to TypecheckError; add a CLI regression test for an executable that is not Python.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not reproduced on 6a9c800. --typecheck --python-env /bin/true exits 2 with --typecheck mypy: /bin/true did not report its version: '' and no traceback: the version probe refuses it before any fence runs. Regression test: test_a_true_python_env_is_a_usage_error_not_a_traceback.

Comment on lines +93 to +94
stdout, stderr, status = api.run(["--no-incremental", f"--cache-dir={cache}", f"--config-file={config}",
*[str(workspace / p) for p in present]])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: mypy_path makes namespace-package imports discoverable, but does not make mypy assign the same module names to explicitly listed files. With no pkg/__init__.py, list both pkg/api.py and pkg/other.py, where api.py imports pkg.other: mypy names the explicit source other and the imported source pkg.other, then stops with “Source file found twice under different module names”, failing an otherwise clean workspace. Add --explicit-package-bases alongside the workspace mypy_path, and extend the namespace-package regression test to list both files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9dfe9b2. Listed namespace modules are named as their importer names them: --explicit-package-bases, with mypy_path = the workspace, plus the source root of each regular package (so a src/ layout still resolves). Test test_a_namespace_package_resolves_from_the_workspace_root now lists both files and fails without the flag.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
runnable = []
for f, fence, code in found:
try:
ast.parse(code, feature_version=target)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: feature_version cannot teach the host's AST parser syntax introduced after the host Python version. Dokimi supports running on Python 3.11; if --python-env is Python 3.12, a valid fence such as type Alias = int\nx: Alias = 'wrong' is rejected by the 3.11 parser and marked unresolved, so its real type error is never checked. Parse using the selected repository interpreter, or explicitly reject unsupported newer target versions instead of silently treating valid repository syntax as invalid Python. Add coverage with a newer target than the host.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9dfe9b2. The fence parse runs in the repository interpreter (-I, type_comments=True, no feature_version), so the repository's grammar decides, not the host's. Test test_a_fence_newer_than_the_repository_grammar_is_a_syntax_error (runs on interpreters older than 3.12).

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +144 to +145
if done.timed_out or done.returncode not in (0, 1): # 1: the tool found errors; anything else is its failure
raise TypecheckError(f"{check.tool} exited {done.returncode}: {done.stderr[-300:]}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: mypy also exits 2 for blocking source diagnostics, which the initial ast.parse does not necessarily catch. For example, x = 1 # type: ??? passes this AST pre-check (type comments are disabled), but mypy rejects the type comment and exits 2. This branch then aborts the entire scan with a usage error instead of reporting a fence result. Distinguish exit 2 with source diagnostics from a checker failure, as the new grader already does, and avoid reporting other fences as ok if the blocking error prevented their checking. Add an L0 regression test for this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not reproduced with mypy 2.4.0: x = 1 # type: ??? exits 1 with code syntax, so the fence is a fail. Exit 2 (blocking errors that name a fence, e.g. a duplicate module) is handled in 9dfe9b2: fences mypy did not reach are unresolved, not ok. Pinned by test_a_blocking_type_comment_is_a_failing_fence_not_a_tool_failure (6a9c800).

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated

def _run_tool(argv: list[str], cwd: str):
try:
return process.run(argv, timeout=TIMEOUT, env=process.Env(allow=None), cwd=cwd)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: the inherited environment still lets host MYPYPATH override the repository's installed dependencies. -I only ignores Python's own environment variables, and an empty mypy config does not disable MYPYPATH. For example, with MYPYPATH=/tmp/stubs and /tmp/stubs/unavailable.pyi defining def foo() -> int: ..., a fence containing import unavailable; unavailable.foo() can report ok even when the repository interpreter has no such package. Strip MYPYPATH from the tool environment (or explicitly clear it) and add a regression test so verdicts depend on the selected repository environment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already fixed earlier: the tool environment is an allow-list (_TOOL_ENV in repos/l0/typecheck.py) that does not pass MYPYPATH. Test test_a_host_mypypath_cannot_supply_a_module_the_pinned_environment_lacks (tests/test_repo_l0_typecheck.py).

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
requirements-test.txt is an input of requirements.lock.txt, and the pytest
job installs with --require-hashes from that lock only. The mypy pin added
in the previous commit did not reach it, so the typecheck tests failed to
collect ("No module named 'mypy'") on the py3.11 and py3.14 Tests jobs.

The five blocks (mypy, mypy-extensions, pathspec, librt, ast-serialize) are
copied from requirements-ci.lock.txt, which pins the same versions and
hashes. The lock was merged, not recompiled with uv, so no other pin moves.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…hon-env is a usage error

A tool run inherited the whole host environment, so a host MYPYPATH added
modules to mypy's search path and a fence could pass on a module the
pinned environment lacks. The run now gets PATH, locale and temp variables
only.

A --python-env that prints no version (e.g. /bin/true) raised an uncaught
ValueError, and repo scan printed a traceback. It is now the usage error
the other --python-env failures already are.

Both are covered by a red-then-green test in test_repo_l0_typecheck.py.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
"--follow-imports=silent", "--no-incremental", "--cache-dir", os.path.join(tmp, "cache"),
"--output", "json", *names]
else:
argv = [check.tool, "--pythonpath", str(check.python), "--outputjson", *names]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: basedpyright can discover configuration in ancestors of this temporary working directory. For example, start a fresh scan with TMPDIR pointing to a checkout containing pyrightconfig.json with {"reportAttributeAccessIssue":"none"}: the json.dumps(1).nosuch() fence can be reported as ok instead of fail. Ancestor extraPaths settings can likewise supply modules absent from the repository environment. Unlike the mypy branch, this invocation does not isolate configuration. Write a neutral basedpyright config inside tmp, pass it explicitly with --project, and add a regression test with a suppressing ancestor config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9dfe9b2. basedpyright runs with --project pointing at an empty pyrightconfig.json in the temp dir, so no config above the run applies. Test test_basedpyright_is_given_an_empty_project_so_no_config_above_the_run_applies captures argv. basedpyright is not installed here, so the real-tool path is not exercised.

… unresolved

- grader: --explicit-package-bases, with the source root of each regular
  package on mypy_path, so a listed namespace module and a src/ layout
  both resolve as they are imported (pkg.other, pkg.x)
- L0: fence syntax is parsed by the repository interpreter, with
  type_comments and no feature_version approximation
- L0: mypy exit 2 is a result only when it names a fence; fences the run
  did not reach are unresolved, not ok
- basedpyright runs with an empty --project, so no config above the run
  applies

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
mypy 2.4.0 reports the # type: ??? comment as a syntax error (exit 1), so
the fence is a fail; the test pins that the parse passes it through to mypy.
Not a regression test: it passes before and after the grader and L0 changes.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The comment block was four lines; check_comment_blocks fails a block of
three or more lines that the PR touches.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
has its files copied over the workspace first, so the reference repair is what gets graded."""
reference = task_dir / REFERENCE_DIR
if reference.is_dir() and run_dir is not None:
shutil.copytree(reference, Path(run_dir) / "workspace", dirs_exist_ok=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security · high
Must-fix: copytree(..., dirs_exist_ok=True) follows existing destination symlinks. For example, if the base checkout contains workspace/pkg/api.py -> ../../task.json and the task supplies reference/pkg/api.py, running the oracle overwrites run_dir/task.json with Python source; the subsequent grade then cannot load its task. A symlinked destination directory has the same escape behavior. The oracle self-check uses a fresh checkout, but repository symlinks are preserved, and the entire run directory is mounted writable. Reject symlinked destination components (including the workspace root), or safely replace them without following them, before copying. Add tests for both file and directory links to an outside sentinel.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9a253f0 (symlink check before any copy) and 4a24291 (empty directories too).
The oracle refuses a destination reached through a symlink: the workspace root, a directory on the way, or the file itself.
It raises ValueError and copies nothing. Tests cover a file link, a directory link and a root link; each leaves the outside sentinel as it was (red on the old oracle).

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +171 to +173
for i, code in enumerate(codes):
Path(tmp, f"fence_{i}.py").write_text(code + "\n", encoding="utf-8")
names.append(f"fence_{i}.py")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: all generated snippets share an import directory, so they can supply modules missing from the repository environment. For example, with a first fence value: int = 1 and a second fence import fence_0\nprint(fence_0.value), mypy resolves the import to the generated first fence and reports no error even when the environment has no fence_0 module. This violates §8.2's missing-module failure contract and makes verdicts depend on other guidance snippets. Isolate generated fences from each other's import resolution for both tool paths, and add a regression asserting that this import fails.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2745c47. Fences are now written as fence-.py: a hyphen cannot be imported, so import fence_0 in a later fence no longer resolves to another fence and is a fail.
Both mypy and basedpyright get the new names. Regression: test_a_fence_cannot_import_another_fence_as_a_module (red before).

mypy's main raises sys.setrecursionlimit for the whole process, so a
later RecursionError-based check (journeys' too-deep registry) passed
on py3.11 CI.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Comment on lines +113 to +114
stdout, stderr, status = api.run(["--no-incremental", f"--cache-dir={cache}", f"--config-file={config}",
"--explicit-package-bases", *[str(workspace / p) for p in present]])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: api.run() still reads MYPYPATH, which takes precedence over the configured mypy_path; Python's -I does not suppress it. This variable can reach the grader through the image's environment or image.env (EvalContext.grade_env preserves those settings). For example, with MYPYPATH=/eval/run/workspace/stubs, a stubs/unavailable.pyi containing def value() -> int: ... makes a listed file importing unavailable.value pass even though the runtime dependency is absent. The neutral config therefore does not fully control import resolution, and agent-written stubs can affect the verdict. Remove MYPYPATH around the mypy invocation, restoring it afterward for in-process callers, and add a regression test for an externally supplied stub directory.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9a253f0. MYPYPATH is popped around api.run and restored in the finally, so an external stub directory cannot satisfy a missing import.
test_an_external_mypypath_cannot_supply_a_module_the_image_lacks is red on the old module and checks the variable is restored.

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three bugs, three risks and two nits. The grader design is sound, and the earlier review rounds fixed most of the isolation problems. Three isolation gaps remain, and each changes a verdict or writes outside the workspace.

What I ran, on a Linux review host at 5d8b05d:

  • make ci-local: 28 passed, 2 failed, 7 skipped. The full suite is 12035 passed, 61 skipped; changed-line coverage is 95%. The two failures are the gitleaks install and the commit scan, whose download got a 504 from the proxy; no step that reads the PR's files failed. The changelog step passes.
  • Reproductions for the three bugs, with the type-checker run for real and no model or network involved.
  • A mutation run over the two new test files (partial, about 40 mutants): the survivors that matter are inline.
  • All 14 commits carry a Signed-off-by.

Other findings:

  1. The contract still tells grader authors the standard library and Python 3.8 are enough. docs/repo-evaluation-contract.md:586 (outside this diff) says built-in graders use "the standard library only and the image's python3 (3.8 or newer)", but dokimi:typecheck imports mypy, and the pinned mypy 2.4.0 declares Requires-Python: >=3.10. An author on a python:3.9 image who follows that sentence gets a grader failure at grade time. The new paragraph at lines 636-641 mentions the mypy requirement, but the sentence above it stays wrong. Suggestion: say "3.8 or newer, and 3.10 or newer plus the pinned mypy for dokimi:typecheck". The same section's "data only" (line 575) and "leaves the workspace alone" (the oracle description) also no longer hold for a task with a reference/ directory.
Bot points I checked that no longer apply at this head (19)
  • exai-bot's point on --typecheck swallowing the source does not apply: --typecheck is a plain flag at this head and --typecheck-tool picks the tool.
  • exai-bot's point on a relative run directory does not apply: grade resolves the workspace to an absolute path before mypy runs.
  • exai-bot's point on the namespace-package search path does not apply: the neutral config sets mypy_path to the workspace and each package root.
  • exai-bot's point on exit 2 for a syntax error does not apply: grade treats a 2 with an : error: line as a diagnostic and fails the grade.
  • exai-bot's point on the no-op oracle does not apply at this head: oracle copies the task's reference/ directory over the workspace (its destination handling is a separate problem, inline).
  • exai-bot's point on the target Python version does not apply: mypy gets --python-version from the repository interpreter and the fences are parsed by that interpreter.
  • exai-bot's point on user-level mypy config does not apply: --config-file points at an empty file in the temporary directory.
  • exai-bot's point on unannotated function bodies does not apply: --check-untyped-defs is passed.
  • Copilot's point on blocking source diagnostics in the grader does not apply: a 2 that printed an error line is graded as a failing diagnostic.
  • Copilot's point on the oracle self-check does not apply at this head: with a reference/ directory the oracle run and the null run grade different workspaces.
  • Copilot's point on module shadowing in the version probe does not apply: the probe runs python -I from the system temporary directory.
  • Copilot's point on host mypy configuration in the fence check does not apply: mypy gets an explicit empty --config-file.
  • Copilot's point on a relative run directory does not apply: the workspace path is resolved before mypy gets it.
  • exai-bot's point on --python-env /bin/true does not apply: the version probe rejects an output that is not two integers with a TypecheckError, so the command exits 2 without a traceback.
  • exai-bot's point on listed namespace modules does not apply: --explicit-package-bases is passed with the workspace in mypy_path.
  • exai-bot's point on the system Python's grammar parsing a newer fence does not apply: the fences are parsed by the repository interpreter itself.
  • exai-bot's point on a blocking exit 2 aborting the scan does not apply: _run returns the fence errors with blocked set, and fences mypy did not reach are unresolved.
  • exai-bot's point on MYPYPATH in the L0 fence check does not apply: the tool environment is the allow-list _TOOL_ENV, which leaves MYPYPATH out (the grader path is different, see the inline comment).
  • exai-bot's point on basedpyright configuration does not apply: the run passes --project with an empty pyrightconfig.json.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Resolved
Comment thread dokimi/dokimi/repos/l0/typecheck.py Resolved
Comment thread dokimi/dokimi/repos/l0/typecheck.py Resolved
Comment thread dokimi/docs/repo-evaluation-contract.md Resolved
Comment thread dokimi/dokimi/repos/l0/__init__.py Resolved
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…graded

is_file() and mypy both follow symbolic links, so a listed file that
the agent linked to the hidden reference repair was graded on that
repair. A link on the way to a listed file, or a linked workspace
root, now makes the path missing before any check or mypy run.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Comment on lines +129 to +130
missing = [p for p in paths
if _linked_on_the_way(linked_root, linked_root / p) or not (workspace / p).is_file()]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security · high
Must-fix: the symlink check covers only paths, but mypy also reads imported modules and package initializers. An agent can leave the listed pkg/api.py as a regular file that imports add from pkg.repair, and make the unlisted pkg/repair.py a symlink to /eval/content/tasks/&lt;task-id>/reference/pkg/api.py. That target is unavailable to the agent but mounted during grading; mypy follows it and can produce a passing grade using the hidden repair. Reject workspace symlinks that mypy could follow (including unlisted .py/.pyi modules and __init__ files), or check a sanitized workspace without access to hidden task data. Add a regression with a regular listed file importing an unlisted symlink to an external repair.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9f0e36: a link whose target lies outside the workspace now refuses the grade as a grader error, listed or not, instead of reading the link as missing. Regression test: test_a_listed_file_importing_an_unlisted_link_to_a_hidden_repair_is_refused.
Refusing rather than marking missing is a design call, open for the maintainer.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +190 to +191
Path(project).write_text("{}\n", encoding="utf-8")
argv = [check.tool, "--project", project, "--pythonpath", str(check.python), "--outputjson", *names]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: the basedpyright branch ignores target. --pythonpath selects the interpreter used to discover import paths, but does not set basedpyright's target Python version; with {} it uses its default version. For a Python 3.11 repository, a fence containing from typing import override can therefore pass even though that API is unavailable in its interpreter. Set pythonVersion in the generated project (or pass --pythonversion) from target, and add equivalent target-version coverage for basedpyright.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9f0e36: basedpyright's explicit project file now sets pythonVersion from the repository interpreter's version, not only the import paths.
Regression test: test_basedpyright_is_told_the_repository_interpreters_version.

…basedpyright's version

A link whose target lies outside the workspace makes mypy read content this run cannot see,
so the grade is refused as a grader error. basedpyright gets the repository interpreter's
version as pythonVersion in its project file, since --pythonpath only sets import paths.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Comment thread dokimi/docs/repo-evaluation-contract.md Outdated
`dokimi:typecheck` checks the listed files only. Its pass is "no error on a
listed file" (an error in a module a listed file imports belongs to that module,
not to the listed file), and a listed file that is missing fails. A symbolic link in the workspace whose target
lies outside it refuses the whole grade (a grader error, not a pass), listed or not: mypy follows an import through a

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation · medium
Must-fix: this promises a grader error for any external symlink, but grade() scans links only when present is nonempty. For example, when the sole listed file is a symlink outside the workspace, grading returns pass: false with n_missing: 1, rather than raising; the existing test_a_listed_path_reached_through_a_link_is_missing_not_graded explicitly requires that behavior. Qualify the contract to say external links refuse grading when mypy would run, and explain that listed paths reached through links count as missing. Otherwise consumers cannot reliably distinguish a failed task from a grader failure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 67bbc3a: the contract now says the refusal applies when mypy runs (some listed file present). A listed path reached through a link is missing, so with every listed file missing the grade fails with n_missing. Code unchanged.

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at c9f0e36. Links to a hidden repair are now closed, both in listed paths and in imported modules. The open problems are in the line comments, one of them blocking.

Agreed with @antonwolfy: repo-evaluation-contract.md lines 604, 615 and 628 are still unchanged.

Agreed with @exai-bot: §4.5 says any link outside the workspace voids the grade, but a listed path through a link is counted as missing (and a test fixes that behaviour).

What I ran, at c9f0e36:

  • The PR's tests: 106 passed, 2 skipped. I did not rerun make ci-local --offline on this head.
  • My repro that imports a module through a link to a file outside the workspace now stops with ValueError: pkg/repair.py is a symbolic link to a file outside the workspace.

Checked, no issues

  • Links in imported modules: the grader now refuses a link to a file outside the workspace instead of grading it. It fails rather than counting a pass, which I think is the right choice.
  • basedpyright target version: the Python version is now passed to basedpyright as well.

Comment on lines +150 to +152
stdout, stderr, status = api.run(["--no-incremental", "--no-color-output", f"--cache-dir={cache}",
f"--config-file={config}", "--explicit-package-bases",
*[str(workspace / p) for p in present]])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: An agent can pass dokimi:typecheck by silencing mypy in the workspace instead of fixing the code.

A listed a.py with x: int = 'a' grades pass: False. With # type: ignore or # mypy: ignore-errors as its first line, or # type: ignore on the line, it grades pass: True. A workspace json.pyi with def __getattr__(name: str) -> Any: ... hides json.nosuch() the same way, because the workspace is first on mypy_path.

Suggestion: count suppressions in the listed files and new .pyi files on the search path as a metric, and fail on any the null workspace did not have; or document the gap in §4.5.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 12ff812. A # type: ignore or # mypy: comment in a listed file fails the grade (n_suppressions), and so does any .pyi in the workspace. Tests cover the examples from the thread and a stub that replaces json.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: 12ff812 catches the comments and the json.pyi stub, but a .py does the same: a workspace json.py with def __getattr__(name: str) -> Any: ... makes json.nosuch() grade pass: True. And refusing every .pyi means a repository that ships its own stubs (ext.pyi for a C extension) can never pass, even with the reference.

Suggestion: refuse a workspace module, .py or .pyi, whose top name is a standard-library module (sys.stdlib_module_names) or one the image's site-packages provides.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f7cb3c5: a workspace .py or package named as a site-packages module is refused too (stdlib names were already refused in 132a88a).
Every .pyi stays refused by design: a stub replaces the module it names, so it can hide errors (see Behaviour change).

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +90 to +98
if not errors:
scan.add(f, fence.line, "python_fence", _label(code), "ok", f"{check.tool}: no error")
continue
block["fail"] += 1
line, message = errors[0]
more = f" (+{len(errors) - 1} more)" if len(errors) > 1 else ""
where = f"line {line}: " if line else "" # 0: the tool gave no line, so the claim is the fence itself
scan.add(f, fence.line + line, "python_fence", _label(code), "fail",
f"{check.tool}: {where}{message}{more}"[:MAX_WHY])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fence that is a fragment of a longer example is reported as a fail stale-API candidate, though no API is stale.

A fence client.decode("{}") after a fence that defines client gives FAIL ... Name "client" is not defined. A fence await asyncio.sleep(1) gives "await" outside function. With --fail-on-fail, ordinary snippets fail the gate.

Suggestion: make a fence whose only errors are missing context (name-defined, top-level-await, return/await outside a function; reportUndefinedVariable for basedpyright) unresolved with that reason.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 12ff812. A fence whose only errors are missing context (an undefined name, top-level await or return) is unresolved with that reason. A fence with a real error still fails.

# run directory or PYTHON* variables can stand in for mypy or for the standard library.
config = os.path.join(tmp, "mypy.ini")
Path(config).write_text("[mypy]\n", encoding="utf-8")
version = [] if target is None else ["--python-version", f"{target[0]}.{target[1]}"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repo scan --typecheck exits 2 for a repository on Python 3.9 or older, because the pinned mypy refuses that --python-version.

mypy 2.4.0 prints "Python 3.9 is not supported (must be 3.10 or higher)" and exits 2. The scan then stops with mypy exited 2 with no fence report. §8.2 does not mention the limit.

Suggestion: when the target is older than the tool supports, mark the fences unresolved with that reason, as for a missing --python-env.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 12ff812. For a repository older than mypy's floor (3.10) the fences are unresolved with that reason and mypy is not run, so there is no exit 2.

Comment on lines +101 to +108
def _oracle_applies(task: Task) -> bool:
"""False for a built-in ``dokimi:typecheck`` task with no ``reference/``: its oracle has no repair to apply
(the grade reads the workspace as it is), so the self-check runs the null step only, and the untouched
listed files must fail the gate."""
if builtin_name(task.raw.get("grader")) != "typecheck":
return True
return (task.dir / REFERENCE_DIR).is_dir()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A typecheck task without reference/ passes tasks check when a listed path does not exist, so no run can ever pass it.

Only the null step runs, and any failing grade satisfies it. With paths: ["src/pkg/modle.py"] the null grade fails with n_missing: 1, and the task is reported ok.

Suggestion: for a typecheck task with no oracle step, report a problem when the null grade has missing files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 12ff812. A typecheck task with no reference now gets a problem when its null grade reports a listed path missing.

Comment on lines +150 to +152
stdout, stderr, status = api.run(["--no-incremental", "--no-color-output", f"--cache-dir={cache}",
f"--config-file={config}", "--explicit-package-bases",
*[str(workspace / p) for p in present]])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy prints names relative to the current directory, so with another cwd a type error grades as a pass.

Listed a.py with x: int = 'a': cwd = the workspace gives pass: False; cwd = the run directory or / gives pass: True (mypy prints workspace/a.py). It works now only because the container's workdir is the workspace. On Windows, 6 of the new grader tests fail the same way.

Suggestion: pass --show-absolute-path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 12ff812. mypy runs with --show-absolute-path, so the grade is the same from any working directory (test runs from the run directory).

if not isinstance(raw.get("version"), str) or not raw.get("version"):
problems.append('version must be the pinned mypy version, e.g. "2.4.0"')
paths = raw.get("paths")
if not isinstance(paths, list) or not paths or not all(_safe_path(p) for p in paths):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two listed names for the same file make mypy stop with "Duplicate module", so the task always fails.

paths: ["a.py", "./a.py"] with a clean a.py grades pass: False, a.py:0: Duplicate module named "a".

Suggestion: in task_problems, reject paths that are equal after posixpath.normpath.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 12ff812. task_problems rejects listed paths that are equal after normpath.

Comment thread dokimi/dokimi/commands/repo.py Outdated
"reported and makes the command exit 1")
scan_p.add_argument("--typecheck", action="store_true", default=False,
help="Type-check each Python fence of the guidance (§8.2): one claim per fence, fail when the "
"tool reports an error. The tool is mypy (the host's pinned copy) unless --typecheck-tool names "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help and §8.2 say the fence check uses "the host's pinned copy" of mypy, but mypy is pinned only in requirements-test.txt and is mypy>=1.8 in the dev extra.

After a plain pip install dokimi the scan exits 2 with "No module named mypy". With the dev extra, two hosts can run different mypy versions on the same fences.

Suggestion: add an extra such as typecheck = ["mypy==2.4.0"] and name it in the help and §8.2.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changed in 12ff812, needs your call: add a typecheck extra pinning mypy==2.4.0 (pyproject plus the uv.lock change it needs), or keep requirements-test.txt as the pin and reword the help and section 8.2? Which do you want?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer decision: keep requirements-test.txt as the pin; help and §8.2 reworded in ac47ae9.

Comment thread dokimi/dokimi/commands/repo.py Outdated
Comment on lines +140 to +144
if args.python_env is not None and not os.access(args.python_env, os.X_OK):
print(f"✗ --python-env {args.python_env}: not an executable interpreter", file=sys.stderr)
sys.exit(EXIT_USAGE)
if tool != "mypy" and shutil.which(tool) is None:
print(f"✗ --typecheck {tool}: not on PATH", file=sys.stderr)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The errors still name the old valued flag, and --python-env accepts a directory.

✗ --typecheck basedpyright: not on PATH (line 144) and ✗ --typecheck mypy: ... (line 163) read like the removed --typecheck <tool>. os.access(path, X_OK) is true for a directory, so --python-env .venv passes and fails later.

Suggestion: ✗ --typecheck-tool {tool}: ..., and os.path.isfile(...) and os.access(...).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 12ff812. The errors name --typecheck-tool, and --python-env must be a regular file.

The contract said a link out of the workspace always refuses the grade. The
grader refuses only when mypy runs, that is when a listed file is present; a
listed path reached through a link is missing, so with every listed file
missing the grade fails with n_missing and no grader error.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…0 targets

- A `# type: ignore` or `# mypy:` comment in a listed file, or a .pyi in
  the workspace, fails the grade (n_suppressions); the grader cannot pass
  by silencing mypy.
- mypy runs with --show-absolute-path, so a type error is graded from any
  working directory; paths equal after normpath are a task problem.
- repo scan: a fence that is only missing context is unresolved; a
  repository older than mypy's floor (3.10) is unresolved, mypy not run.
- --python-env must be a file; the CLI names --typecheck-tool in errors.
- tasks check: a typecheck task with no reference and a missing listed path
  is a problem (its null grade is the only step).

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
found = []
for p in listed:
text = (workspace / p).read_text(encoding="utf-8", errors="replace")
found += [f"{p}:{n}" for n, line in enumerate(text.splitlines(), 1) if _SILENCE.search(line)]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: this scans raw source text, so a clean listed file containing message = "# type: ignore" is counted as suppressed and grades pass: False, even though mypy sees no suppression comment. Docstrings showing # mypy: examples fail for the same reason. The contract restricts actual comments that silence mypy, not marker text inside strings. Inspect Python COMMENT tokens (preserving their line numbers) instead, and add regressions for string literals and multiline docstrings alongside genuine suppression comments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 47a95a6. The suppression check reads comment tokens, so a marker in a string or docstring no longer fails a clean file; a genuine comment still does, on its line.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +36 to +37
_FRAGMENT = re.compile(r'^(?:Name "[^"]+"|"[^"]+") is not defined(?: \[reportUndefinedVariable\])?$'
r'|^"(?:await|return|yield)" outside function$')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: the outside-function alternative only matches mypy's wording. For example, a fence containing import asyncio\nawait asyncio.sleep(1) produces basedpyright's "await" allowed only within async function, which does not match this regex (and diagnostic rules, when present, are appended by _basedpyright_errors). _claims consequently marks this missing-context fragment as fail rather than unresolved, contrary to §8.2, and --fail-on-fail can exit 1 solely because basedpyright was selected. Recognize basedpyright's corresponding diagnostics, including their optional rule suffixes, and add basedpyright coverage for top-level await/return/yield fragments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 47a95a6. basedpyright's top-level await/return/yield messages are fragments now (its own wording, with an optional rule suffix); covered by a basedpyright test.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
out = []
for code in json.load(sys.stdin):
try:
ast.parse(code, type_comments=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: the older-interpreter fallback still fails for Python 3.7. With --python-env /path/to/python3.7 and any Python fence, this raises TypeError because ast.parse gained the type_comments parameter in Python 3.8. _syntax_errors runs before _older_than_the_tool, so the scan exits 2 instead of reporting the fences as unresolved for an unsupported mypy target. Omit this keyword on older interpreters (or handle the unsupported target before parsing), and add a regression covering this parser API boundary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 47a95a6. The fence parser omits type_comments where ast.parse does not take it (Python 3.7), so the version check runs instead of a crash; covered by a regression test.

The help and contract §8.2 called mypy "the host's pinned copy", but
pip install dokimi does not install it: the pin is requirements-test.txt
(mypy==2.4.0), and a missing mypy exits 2.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…7 parse

- A listed file is suppressed by a `# type: ignore` or `# mypy:` comment, read
  with tokenize: a marker in a string or docstring no longer fails a clean file.
  A file the tokenizer cannot read is still scanned line by line.
- basedpyright words top-level await/return/yield its own way (with an optional
  rule suffix): those fences are unresolved, as mypy's are.
- The fence parser omits type_comments on Python 3.7 (the keyword is 3.8+).
- Contract 4.5: dokimi:typecheck imports mypy (3.10+) and oracle copies reference/.
- The Check.tool comment names mypy's real pin (requirements-test.txt).

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
if ch != ":":
continue
tail = _TAIL.match(text[k:])
name = posixpath.normpath(text[:k])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
Must-fix: native Windows separators survive posixpath.normpath, so listed-file errors can be discarded. For example, _errors(r'C:\run\workspace\pkg\api.py:2: error: boom', {'pkg/api.py'}, 'C:\\run\\workspace\\') returns []: after stripping the root, the candidate is pkg\\api.py, not pkg/api.py. With mypy's normal error exit (1), grade() can consequently return pass=True. The same normalization occurs in the listed is None branch. Normalize native diagnostic paths to POSIX separators before matching (without changing literal backslashes in POSIX filenames), and add a Windows-path regression. This also leaves the earlier cwd/Windows diagnostic fix incomplete for direct Windows grading/tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6340ce: the native separator is converted before the listed names are matched, so a Windows diagnostic path is still this file's error. Regression test added.

Comment thread dokimi/dokimi/repos/selfcheck.py Outdated
"""For a typecheck task with no reference the null grade is the only step: a listed path it reports missing
fails that step, so no run could pass the task."""
doc, _ = read_grade_json(rd / "grade.json")
missing = doc.get("missing") if isinstance(doc, dict) else None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · high
Must-fix: the missing-path fix does not hold for actual grader output. graders/main.py puts lists under details, so a task with no reference and paths: ["src/pkg/modle.py"] writes "details": {"missing": ["src/pkg/modle.py"]}. This top-level lookup returns None, leaving the task reported OK, as in the earlier thread. Read details.missing instead, and exercise this check with main.grade() output; the current regression fixture writes a top-level missing that the entry point never emits.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6340ce: _missing_listed reads details.missing, where the grader writes lists. The fixture has the grader's shape.

…ts own parse script

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Comment thread dokimi/dokimi/commands/repo.py Outdated
try:
payload = scan_repo(tree, manifest, static, typecheck=check)
except TypecheckError as exc:
print(f"✗ --typecheck-tool {check.tool}: {exc}", file=sys.stderr)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maintainability · low
A TypecheckError raised by the repository interpreter (python_version: 'did not report its version', or 'did not parse the fences') is reported as --typecheck-tool &lt;tool>: ..., which names the wrong option. Say which step failed (the interpreter or the tool), or label the error by its source.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6340ce: the error is labelled repo scan --typecheck, and its text names the failing step (tool or interpreter). Test added.

return None
name = m.group("file")
return posixpath.normpath(name[len(root):] if name.startswith(root) else name), m
text = line[len(root):] if line.startswith(root) else line

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · high
Still open from the PR thread: on a Windows host mypy prints backslash paths (C:\run\workspace\pkg\api.py). root is built with os.sep, so the stripped name is pkg\api.py, and posixpath.normpath does not convert separators. name in listed then fails for pkg/api.py, the error is dropped, and a file with a type error grades pass: True. Repro: _errors(r'C:\run\workspace\pkg\api.py:2: error: boom', {'pkg/api.py'}, 'C:\\run\\workspace\\') returns []. Normalise the separators (e.g. .replace(os.sep, '/')) on both the mypy name and the root, or compare with pathlib.PurePath equality, and add a test with a backslash line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same fix as the thread above, in a6340ce: _name converts the native separator on the mypy name; a POSIX backslash stays literal (test added).

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +78 to +79
scan.add(f, fence.line, "python_fence", _label(code), "unresolved",
f"not valid Python ({why}): not type-checked")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation · low
With no --python-env, the fences are parsed by dokimi's own interpreter, which may be older than the repository's grammar (e.g. a 3.12+ type alias on a 3.11 host). Such a valid fence is labelled "not valid Python", which is inaccurate. Use a wording that says it did not parse under the parsing interpreter, or name that interpreter in the reason.

Suggestion:

Suggested change
scan.add(f, fence.line, "python_fence", _label(code), "unresolved",
f"not valid Python ({why}): not type-checked")
scan.add(f, fence.line, "python_fence", _label(code), "unresolved",
f"not parsed by {check.python or 'dokimi'} ({why}): not type-checked")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6340ce: the reason names the interpreter that parsed the fence (the repository's, or dokimi's without --python-env).

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
Comment on lines +121 to +122
fails += 1
line, message = errors[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · medium
A fence whose import cannot resolve in the pinned environment (an optional extra, a dev-only dependency, or the import fence_0 case) is reported as fail, labelled a stale-API candidate. The failure is in the environment, not in the guidance, so it inflates the fail count the since-cutoff split relies on. The test test_a_fence_cannot_import_another_fence_as_a_module pins this as fail. Suggest treating mypy import-not-found (and the basedpyright equivalent) as unresolved with a 'module not in the repository environment' reason, and not counting it as fail.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should a module the pinned environment lacks stay fail (contract §8.2 says so, and a test pins it) or become unresolved? Not changed in a6340ce; your call.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
f"{tool}: {errors[0][1]} (a fragment of a longer example: not type-checked)"[:MAX_WHY])
continue
fails += 1
line, message = errors[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maintainability · low
When a fence has both fragment errors and real errors, errors[0] may be the fragment message (for example an undefined name from an earlier fence). The claim's why then names a harmless error while the real one is only counted in '(+N more)'. Prefer the first non-fragment error for the message.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6340ce: the claim names the first error that is not a fragment; the count still covers all errors. Test added.

Comment thread dokimi/dokimi/repos/selfcheck.py Outdated
"""For a typecheck task with no reference the null grade is the only step: a listed path it reports missing
fails that step, so no run could pass the task."""
doc, _ = read_grade_json(rd / "grade.json")
missing = doc.get("missing") if isinstance(doc, dict) else None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug · high
Must-fix: missing is a list, so graders/main.py:split_result moves it under details, not top level. doc.get("missing") returns None for real grader output, _missing_listed returns [], and a no-reference task with a nonexistent listed path is still reported OK. Read doc["details"]["missing"] (with a dict check on details). Add a test that feeds real main.grade output, not a hand-built top-level dict.

Suggestion:

Suggested change
missing = doc.get("missing") if isinstance(doc, dict) else None
details = doc.get("details") if isinstance(doc, dict) else None
missing = details.get("missing") if isinstance(details, dict) else None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same fix as the thread above, in a6340ce: the lookup reads details.missing.

Comment thread dokimi/pyproject.toml
Comment on lines +145 to +146
# repos/graders/typecheck.py imports mypy inside the grading image, where it is installed
# for the grader; the wheel does not ship it. The host never imports it (the L0 check runs it as a tool).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style · low
This new comment block is three # lines, and AGENTS.md says a block of three or more # lines fails scripts/check_comment_blocks.py once the PR adds or edits a line in it. Shorten it to two lines, e.g. # mypy is imported by repos/graders/typecheck.py inside the grading image only; the wheel and the host do not need it.

Suggestion:

Suggested change
# repos/graders/typecheck.py imports mypy inside the grading image, where it is installed
# for the grader; the wheel does not ship it. The host never imports it (the L0 check runs it as a tool).
# repos/graders/typecheck.py imports mypy only inside the grading image; the host runs it as a tool (L0).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change: the block is two lines at a6340ce (pyproject.toml 145-146), and scripts/check_comment_blocks.py --base 41b3d8df --head HEAD reports no block over two lines.

raw = {"grader": "dokimi:typecheck", "tool": "mypy", "version": PINNED, "paths": ["src/pkg/modle.py"]}
rd = tmp_path / "rd"
rd.mkdir()
(rd / "grade.json").write_text(json.dumps({"pass": False, "missing": ["src/pkg/modle.py"]}), encoding="utf-8")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test · high
Must-fix: this fixture does not match what the grader writes. main.split_result moves lists (missing) under details, so real grade.json has details.missing, not a top-level missing. The test feeds the top-level shape, so it passes while selfcheck._verdict_problems misses the real case (the open selfcheck thread). Build the fixture with main.grade(...) output, or with {"pass": False, "metrics": {"n_missing": 1}, "details": {"missing": [...]}}, and add a test that fails on the current lookup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6340ce: the fixture is built with main.split_result, so missing sits under details as the grader writes it.

…he real error

- graders: a diagnostic with the native separator (Windows) matches its POSIX listed name
- selfcheck: a no-reference missing path is read from details (the grader puts lists there)
- l0: the claim names the first real error, not a fragment error from the same fence
- repo scan --typecheck: an interpreter failure is no longer labelled --typecheck-tool
- l0: the unparsed-fence reason names the parsing interpreter

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-check at a6340ce. Nearly every point from my last round is fixed: fence fragments and pre-3.10 environments are now unresolved, the grade no longer depends on the working directory, duplicate paths are rejected, and the self-check catches a missing listed file. One point is only partly fixed: a workspace .py can still shadow a module, and refusing every .pyi locks out repositories that ship their own stubs (reply in its thread). The branch now conflicts with main in repo-evaluation-contract.md.

What I ran, at a6340ce:

  • make ci-local --offline: 18 passed, 1 failed, 13 skipped. The failure is schema_compat: headroom.v1.json was added on main by #279 after this branch's base, so it is not from this PR.
  • The real grade() with mypy 2.4.0: a json.pyi stub and suppression comments now fail the grade; a root json.py with a module __getattr__ still passes; a clean file next to its own ext.pyi fails. A type error fails from the run directory and from /.
  • repo scan --typecheck on a scratch AGENTS.md: two fragment fences are unresolved, the stale call and the mixed fence fail, and the mixed one names its real error. With an interpreter that reports Python 3.9, all five fences are unresolved and the scan exits 0.
  • On Windows (Server 2025, Python 3.13): pytest tests/test_repo_l0_typecheck.py tests/test_repo_typecheck_grader.py: 124 passed, 2 failed, 4 skipped. The type-error-as-pass failures from my last round are gone; the 2 left are POSIX-only test setups (a : in a directory name, a 0700 mode).

Checked, no issues

  • Self-check: _missing_listed reads details.missing, where main.split_result puts the list, and runs only when there is no oracle step.
  • Contract: §4.5 now names reference/, the mypy and Python 3.10 requirement and the oracle's copy; §8.2 says how mypy is installed, as the maintainer decided.



@pytest.mark.parametrize("sep", [",", ":"])
def test_a_source_root_with_a_mypy_path_delimiter_is_a_task_error(tmp_path, sep):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Two grader tests fail on Windows for reasons the grader does not have there.

This test cannot create a src:lib directory (WinError 123), and test_the_oracle_refuses_a_linked_workspace_even_with_an_empty_reference (line 396) checks a 0700 mode that Windows does not keep. The other 124 tests of both files pass there.

Suggestion: skip these two on win32, as the POSIX-script test already is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f7cb3c5: both tests skip on win32 (the src:lib directory and the 0700 mode check), as the POSIX-script test does.

mypy stops on a blocking error (a top-level yield, break outside a loop),
so one such fence left every other fence unresolved and --fail-on-fail
passed a stale fence. The fences the blocking error did not name are
checked in another run.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
A top-level async with or async for lacks the same context as a
top-level await, yet both tools' messages for it were read as a stale
API, so --fail-on-fail failed correct async snippets.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Section 8.2 said only a name an earlier fence defines is missing context,
but every undefined name is, an omitted import included (the PR's own
fragment test pins `asyncio` with no import). Say so, and that a removed
builtin such as `unicode` is unresolved rather than a fail.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
The grader's mypy run lacked --check-untyped-defs, which the L0 check
passes, so an agent could make a failing grade pass by deleting a
function's annotations instead of fixing its body.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
@no_type_check (under any alias) makes mypy skip the function, so a
listed file with a type error graded pass with no suppression counted.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
The workspace is first on mypy_path, so a json.py replaces json exactly
as the refused json.pyi does: a __getattr__ -> Any there hid the listed
file's error and the grade passed. It now counts as a suppression.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
With no reference/ the null grade is the only step. A suppression the
workspace already has (a repository .pyi) fails every grade, so even a
run that repairs the code cannot pass, yet tasks check reported ok.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
repo-evaluation-contract.md: intel#349's structure paragraph closes 8.1, then this branch's 8.2 on Python fences.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
…_scan.py

With main merged in, commands/repo.py reached 823 lines, over the growth ratchet's 800 cap.
The two helpers this branch adds move out unchanged; repo.py imports them back.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
@xaleryb

xaleryb commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Ran: full suite, lint, types and security on the head plus the commits below, and on the same merged with main. Also a CLI and grader repro for each point.

  • A fence importing an installed package that has no py.typed (import pygments; print(pygments.__version__)) -> mypy fail "Library stubs not installed", basedpyright ok. Expected: not a fail (§8.2 fails only a module the environment lacks). --follow-untyped-imports makes the scan exit 2 when such a package has source mypy cannot parse. Proposal: treat a fence whose only errors are import-untyped as unresolved. The grader has the same false fail.
  • These still grade pass: true with n_suppressions: 0: a workspace yaml.py shadowing an installed package; def __getattr__(name) -> Any in an unlisted module; the bad code under if not TYPE_CHECKING:. Expected: false, or §4.5 names the gap.
  • A workspace .venv -> grader error (.venv/bin/python links outside). Expected: a pass.

Pushed:

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at bc9bc0e. Not ready to merge. The delta since my last review is sound on the points it set out to fix, but the fragment rule still sends missing-context fences to fail, two new guards that decide a verdict have no test, and points raised in comments are still open. Five points of mine: four inline (three risks, one nit) and one nit below.

What I ran, on a Linux review host at bc9bc0e:

  • make ci-local: 29 passed, 1 failed, 7 skipped; 13017 tests passed, 1 failed. The failure is tests/test_reference_checks.py::test_the_regex_limit_is_shared_by_every_pattern_of_one_check, a timing assertion (3163.69 - 3161.84 = 1.86 s against a 1.5 s limit) in a file this PR does not touch. The coverage-of-changed-lines step passes.
  • Reproductions with the real mypy 2.4.0 for the code findings, no model or network involved.
  • A mutation run over the L0 check and its CLI wiring (the survivors that decide a verdict or exit code are inline) and over the grader (6 mutants, all killed).
  • All 41 non-merge commits carry a Signed-off-by.

Other findings:

  1. Nit: the PR description is stale against the code. It shows repo scan --typecheck [mypy|basedpyright] and a known limitation about nargs="?" swallowing the source with invalid choice. At this head --typecheck is a plain flag and --typecheck-tool {mypy,basedpyright} picks the tool (cli-interface.json, the changelog). The make ci-local and self-review markers in the description name d96d7db8 and 057ad88a, not the head. Suggestion: update the usage block, drop the limitation, and cite the head.

Still open from comments on this PR:

  • I agree with @xaleryb on the installed package without py.typed. New evidence: mypy 2.4.0 reports import-untyped for it, so the fence is fail and --fail-on-fail exits 1, while §8.2 fails only a module the environment lacks.
  • I agree with @xaleryb on the workspace .venv. New evidence: a .venv/bin/python link to /usr/bin/python3 grades pass: null, and a .pyi under .venv/.../site-packages or .git/ fails a clean file, since the .pyi walk at graders/typecheck.py:157-158 covers the whole workspace.
  • @exai-bot's question on whether a module the pinned environment lacks stays fail or becomes unresolved waits for the author: the reply handed it to the maintainer, and it is unanswered.

Checked, no issues

  • Symlinks: a listed file, a linked parent directory and a linked workspace root are each read as missing before is_file() and before mypy runs (12 link tests pass). A link to a file outside the workspace refuses the grade.
  • Suppressions: # type: ignore, # mypy: and no_type_check are read from comment tokens, so a marker inside a string is not counted; # pyright: ignore is not a mypy silence and the error is still reported.
  • Text report move: repo_scan.py keeps the wrote line, warnings, incomplete reasons and FAIL lines of the base cmd_repo_scan, and the JSON report carries the typecheck block.
  • Repo rules: mypy==2.4.0 matches in the test requirements and both lock files with hashes, schema_compat passes, the schema change is an optional property, and no generated file is committed.
Bot points I checked that no longer apply at this head (44)
  • Copilot's and exai-bot's points on --typecheck swallowing the source, a relative run directory, the namespace-package search path, exit 2 for a syntax error, the no-op oracle, the target Python version, host mypy config, unannotated bodies, shadowing in the version probe, /bin/true, listed namespace modules, newer-grammar fences, blocking exit 2, MYPYPATH (fence check and grader), basedpyright config, copytree through links, fences importing each other, empty-reference linked root, report validation, colour output, line separators, : error: in names, ,/: in roots and the win32 skip do not apply: each is fixed at this head and was checked in my earlier reviews.
  • exai-bot's points on a listed file linked to the hidden reference, links in unlisted modules, the basedpyright target version and the contract wording on external links do not apply: the link check runs before is_file(), an outside link refuses the grade, the project file sets pythonVersion, and the contract says the refusal applies when mypy runs.
  • exai-bot's points on marker text inside strings, basedpyright's top-level await/return/yield wording, Python 3.7 type_comments, Windows separators and details.missing do not apply: comment tokens are read, the basedpyright wording is matched, the keyword is omitted on old interpreters, _name converts the native separator, and the self-check reads details.missing (the fixture is built with split_result).

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
# Fragment-only errors (a name an earlier fence defines, await/return/yield or async with/for outside a function),
# in mypy's or basedpyright's wording; any other error is real.
_FRAGMENT = re.compile(r'^(?:Name "[^"]+"|"[^"]+") is not defined(?: \[reportUndefinedVariable\])?$'
r'|^"(?:await|return|yield)" outside function$'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other missing-context fences are still graded fail, a stale-API candidate. _FRAGMENT names await, return, yield, async with/for and undefined names, but not the other statements that only make sense inside a function or loop. Real mypy 2.4.0 through the fence check:

  • yield from range(3) -> fail, "yield from" outside function
  • continue -> fail, "continue" outside loop
  • a fence that ends in break -> fail, "break" outside loop (plus Name "y" is not defined)
  • nonlocal x -> fail, nonlocal declaration not allowed at module level

All of them parse, so they reach mypy. A loop body or generator body in an AGENTS.md fence inflates the fail count that the since-cutoff split reads, and --fail-on-fail exits 1. Pyright's break wording (can be used only within a loop) is missing too.

Suggestion: add "(?:yield from|continue|break)" outside (?:function|loop) and nonlocal declaration not allowed at module level to _FRAGMENT, with the basedpyright wordings, and a real-mypy test per statement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f7cb3c5: continue, break, yield from outside a loop or function, and nonlocal at module level are fragments now.
A real-mypy test per statement; basedpyright's wording is from its messages and was not run here.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Outdated
# Fragment-only errors (a name an earlier fence defines, await/return/yield or async with/for outside a function),
# in mypy's or basedpyright's wording; any other error is real.
_FRAGMENT = re.compile(r'^(?:Name "[^"]+"|"[^"]+") is not defined(?: \[reportUndefinedVariable\])?$'
r'|^"(?:await|return|yield)" outside function$'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing pins the mypy wording of a top-level return. Mutation check: dropping return from (?:await|return|yield) here passes the full suite. Removing await or yield fails a test, but the mypy fragment tests cover only await, async with and async for; return and yield are covered only through the fake basedpyright test. If mypy changes that message, a fence with a top-level return turns from unresolved into fail and nobody notices.

Suggestion: add a real-mypy test with a fence return 1 that asserts unresolved, next to the await one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f7cb3c5: a real-mypy test pins a top-level return fence as unresolved, next to the await one.

target: tuple[int, int] | None) -> bool:
"""True, with each runnable fence unresolved, when the repository is older than the tool checks (no run)."""
floor = MIN_PYTHON.get(check.tool, (0, 0))
if target is None or target >= floor:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Python 3.10 boundary is untested, and it decides whether a repository is checked at all. Mutation check: target >= floor changed to target > floor passes the full suite (64 tests). With that mutant every repository on exactly 3.10 is reported unresolved for all fences and never checked; the only test uses 3.9.

Suggestion: add a test with an interpreter that reports 3.10 and asserts a fence is checked (ok or fail), next to the 3.9 one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f7cb3c5: a test pins Python 3.10 as checked (ok and fail), next to the 3.9 one. A > mutant fails it.

Comment thread dokimi/dokimi/repos/l0/typecheck.py Resolved
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit per nit, up to 9fcddfb, which fix the nit from my review:

  • 9fcddfb: dokimi/tests/test_repo_l0_typecheck.py:526: added a test with failing python, bash and json fences, asserting only the python one is counted and fails.

Left as is:

  • PR description: the stale description lives on GitHub, not in the repo, so there is nothing to commit.

make ci-local at 9fcddfb: 28 passed, 2 failed (host: gitleaks download), 7 skipped.

Not approving yet. Still open: the three risks from my review; @xaleryb on an installed package without py.typed is a mypy fail (import-untyped), contrary to §8.2; same false fail in the grader; @xaleryb on a workspace .venv (link to the interpreter, or .pyi under site-packages) makes the grade pass:null or a false fail; exai-bot on whether a module the pinned environment lacks stays fail or becomes unresolved; the author handed the choice back to the maintainer.

…lled-package shadows refused

- L0: a fence whose only errors are a module the repository environment lacks is unresolved, not fail;
  a real error in the same fence still fails it. The contract §8.2 says so.
- L0: break, continue, yield from outside a loop or function, and nonlocal at module level are fragments
  (mypy's wording, and basedpyright's where known), not stale calls.
- grader: a workspace .py or package named as a module the image's site-packages provides is a
  suppression, as a standard-library name already was (contract §4.5).
- tests: a real-mypy top-level return, the Python 3.10 boundary, the two grader tests that cannot run on
  Windows are skipped there.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli-interface-change Changes dokimi's user-facing CLI surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants