You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds the customer-journey catalog (J1–J15) and a criterion-level evidence trace on top of the existing pytest --outcomes plugin. Tracking: #95 (children #96–#100). This PR does not redesign the CLI, and it does not commit to any future command layout.
How a test session decides each criterion's verdict:
pytest --outcomes
marked test runs -> record each argv the `dokimi` root parser accepted
session end
catalog: J1–J15 defined, UC/AC each under the right parent, no duplicates, no dangling ids
registry: AC exists, refs/anchors resolve, surfaces/invokes resolve via build_parser()
enforce: true ->
unclaimed id = error, failed = fail, all skipped = inconclusive
cli_scenario passes only if a captured argv reached the command with the declared flags
enforce: false / untracked -> listed as unverified, never gated
write journeys-result.json + .md (commit, run id, stubs, gaps, unverified)
What is enforced: 24 criteria across J2, J3, J4, J7, J10, J11, J14 and J15, covering the initial scenarios:
J10: the pinned-question contract is reused. Different question-set hashes warn; one arm name carrying two prompt digests is refused; cache behaviour is made explicit.
J4: the static scan and the reach measurement are separate criteria. Task impact is partial and not enforced.
J7: the repo tasks check null/oracle/isolation self-check, with no claim of relevance or coverage.
J2/J14: novelty spend cap, cost estimate, CLI authoring path, and refusal of a stale inventory.
J3: resume and the egress refusal.
J15 (turn a problem solved in chat into a skill, added in 68e79ce): the skill-creator arm uses exactly the named creator. Drafting tasks from a transcript and keeping the source task apart are proposed; the three-arm comparison (no skill, user's skill, skill-creator's) is partial.
What is not enforced: the other 37 documented criteria. They show up in the report as unverified, even when their tests pass. These include AC-J04-004 (a refusal is classified not_loaded) and AC-J07-005 (a capped run prints PASS (inconclusive)), which have stated gaps.
Existing tests: most of the change to them is @pytest.mark.outcome ids. Beyond ids: test_docs_code_refs.py and test_docs_links.py read the new user-journeys docs; test_judge_reliability.py, test_model_compare.py and test_novelty_journey.py have assertions rewritten or a test replaced; test_outcomes_plugin.py resets REGISTRY; test_repo_runner.py gains a new test. OUTCOME-PINNED-QUESTIONS keeps its semantics.
CI: passes --journeys-report and uploads the report with the coverage artifact.
Repository rules: no ADRs and no status files. Design rationale stays in the issues.
No changelog fragment: nothing under dokimi/dokimi/ changed.
Inspected commit: f76a76020b987bf51c56dfd1a98830a3639ece84 (the intel/main tip when the branch was cut).
Evidence
Before: no journey docs. The outcomes check could not tell a test that runs a CLI command from one that only mentions it in a string.
Negative checks in tests/test_journeys_trace.py, each as an inner pytester session or a direct check:
command named only in a string -> exit 2, verdict error
argv the parser rejects -> exit 2, verdict error
enforced evidence all skipped -> exit 3, inconclusive
unclaimed / failed enforced evidence -> error / fail
unknown command, unknown flag, non-leaf -> error
broken path/anchor, undefined AC, dangling id, AC under the wrong J -> error
unenforced criterion, test not written -> not_run, session still passes
make ci-local: every row passed except test, including mypy, ruff, schema-compat, mkdocs build and package-smoke.
Known flake: the test row failed twice on timing-sensitive process-kill tests (test_reach_timeout_kills_the_harness_process_tree, test_a_timed_out_scenario_keeps_its_trace_up_to_the_kill, test_example_runner_processes). A different set failed each time. None of them is marked or touched by this PR. The shared host had load ~259 on 224 cores with ~200 xdist workers. The same tests pass 3/3 in isolation, and the full suite passes at -n 32.
Not run: live-agent or paid evaluations, and Docker. The external-catalog-smoke job and semgrep were skipped locally.
Merge Danger
Door: two-way
Docs, tests and one CI flag; there are no package changes. Reverting removes the trace.
Blast Radius: test-suite
A future PR that renames a command, removes a marked test or drops a criterion will fail pytest --outcomes until journeys.yaml is updated in the same PR. That failure is intended.
Running only a subset of tests with --outcomes already errors on unclaimed evidence. This PR adds the journey ids to that.
Open questions, not decided in this PR: who declares a whole journey complete, and who owns each journey (#95).
Add scripts/audit_test_quality.py, a static scan of every test_* function
that flags type_only, mock_echo, set_then_read, constant_mirror, no_assert
and duplicate tests (trivial_size is reported, not gated), and
tests/test_audit_test_quality.py, which holds the findings to a baseline
that only shrinks. The baseline starts empty.
The 30 tests flagged on main are fixed: 27 now assert the value, call
argument or output they exist for, two are deleted as covered elsewhere,
and one must-not-raise test is exempted with its reason.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Document the fourteen agreed journeys (J1-J14) under docs/user-journeys/:
catalog with use cases (UC-Jnn-nnn) and acceptance criteria (AC-Jnn-nnn),
flow map, and the traceability rules. Novelty is a J14 use case and a
diagnostic in J2/J5/J13, not a journey of its own.
Trace criteria in tests/outcomes/journeys.yaml, checked by the existing
pytest --outcomes plugin: ids and parents resolve, refs and anchors
resolve, CLI surfaces resolve against build_parser(), and an enforced
criterion passes only on claimed, passing evidence. A cli_scenario must
have parsed its declared command: the plugin records the argv the root
parser accepted while a marked test ran, so a command named in a string
does not count. 25 implemented criteria are enforced; every other
documented criterion is reported as unverified and blocks nothing.
The session writes journeys-result.json and .md (commit, run id, stubs,
gaps, unverified criteria); CI uploads them with the coverage artifact.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.29%. Comparing base (61e0355) to head (787c8da). ⚠️ Report is 1 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
- Reject evidence entries with missing or empty IDs during validation, preventing silent discarding when no outcome contract exists
- Record CLI invocations only when parse_known_args has no leftover arguments, preventing false positives when parse_args later rejects unknown flags
- Add regression tests for both issues
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Address four precision gaps in the test auditor:
1. Constructor tracking now invalidates after object mutations (cfg.retries=9)
so set_then_read tests detect broken reset implementations.
2. Tuple comparisons like assert (mock.call_count, out) == (1, 42) are
recognized as behavioral (not mock-only) since they check values too.
3. Weak direct asserts plus asserting helpers no longer flag as type_only,
since the helper detects behavior.
4. Duplicate detection now includes parameter defaults so test_a(x=1) and
test_b(x=2) with the same body are not flagged as duplicates.
Each fix includes a regression test that fails on the old logic.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Evidence matching and verification accounting can report criteria as passing without demonstrating their stated behavior.
No open issues 🎉
4 previously missed issues
In code that hasn't changed since last review
Normalize parsed options and respect the argument terminator
dokimi/tests/outcomes/journeys.py:140
resolve() treats raw dash-prefixed tokens as options rather than matching what argparse recognized. For example, repo scan -- --fail-on-fail parses with source='--fail-on-fail' and fail_on_fail=False, but satisfies repo scan --fail-on-fail evidence. Conversely, the accepted abbreviation --fail-on-f enables the option but fails this check. Respect the -- terminator and normalize recognized options before matching, with regression cases for both inputs.
Count unenforced criteria as unverified
dokimi/tests/outcomes/journeys.py:294
The PR and #95 require unenforced criteria to remain unverified. However, an enforce: false entry with passing evidence keeps verdict='pass', disappears from unverified, and increases the journey's passed count. Disabling enforcement while keeping the evidence can therefore still show the criterion as verified. Include unenforced criteria in the unverified accounting, align the counts and report wording, and preserve observed evidence results separately. Add a case with unenforced, passing evidence.
Refusal classification is not asserted by the test
dokimi/tests/outcomes/journeys.yaml:109
AC-J04-004 promises that refusals are never classified as not reached, but the marked refusal fixture produces a successful, token-free answer. dokimi/repos/reach.py:265–266 classifies that answer as not_loaded; test_reach_flags_a_refusal only checks the prose warning. This entry therefore reports passing evidence for a stronger behavior than the test demonstrates. Narrow the criterion to the warning behavior, or mark it partial and unverified with an explicit gap until refusal classification is implemented and asserted.
Exclude skipped-test invocations from evidence
dokimi/tests/outcomes/plugin.py:69
Invocations from skipped tests are merged into the evidence. If two tests share an evidence id, a passing test that never invokes the command plus a test that parses it and then calls pytest.skip() produces a passing CLI scenario. Collect invocations only from passing reports, and add a mixed pass/skip regression case.
The reason will be displayed to describe this comment to others. Learn more.
Review at fc52ce1. The catalog, the parser checks and the docs build hold up, and the earlier missing-id and unknown-argument fixes work. The problems are in what an enforced pass actually proves: three criteria claim more than their marked test asserts, and the CLI check is weaker than the docs say. I'd fix the inline risks before merge.
make ci-local: 19 passed, 0 failed, 4 skipped (docker, semgrep, test-latest, deploy). The test row: 5226 passed, 1 skipped. journeys: pass, 25 enforced criteria, 31 of 56 unverified. mkdocs strict build and zizmor are clean.
Reproductions for the code findings, with the LLM transport mocked, including a real pytest --outcomes session on a probe file, serial and with -n2 (same result).
A mutation run on journeys.py and plugin.py: 71 mutants, 25 of which pass test_journeys_trace.py plus the two docs tests. Several are equivalent; the real gaps are in item 1 below.
Findings without a single anchor:
The verdict logic has surviving mutants. Each of these passes all 88 tests: (a) if not entry.get("enforce") at journeys.py:276 replaced by if True; (b) gated at journeys.py:292 built from all rows instead of enforced ones, which breaks the "unenforced fails nothing" promise; (c) recorded_manual/live_agent evidence returning pass instead of not_run (journeys.py:238); (d) not all(seen.values()) changed to not any(...) at journeys.py:244; (e) swapping inconclusive and fail in RANK (journeys.py:42, plugin.py:40), because no test combines the two; (f) the report.when == "setup" and not report.passed clause at plugin.py:62 set to False, so a fixture error in one claimed test is ignored when another test passes. Suggestion: one parametrized pytester case each; for (e), one failed and one all-skipped enforced criterion expecting fail and exit 1.
I agree with @copilot-pull-request-reviewer on option matching. New evidence: repo tasks check src --ta x is accepted by argparse as --task, but resolve() records --ta, so AC-J07-003 came out error. On their AC-J04-004 point: the refusal test (test_repo_reach.py:425) asserts only prose > 8 and prose text, never a scenario class.
… pin more auditor branches
A helper counts as a behavioral check only if it has a non-shape assert. Adds
fixtures for helper shape/value, every patching spelling and the len() spellings;
the anthropic judge init test now passes temperature and seed; the persona clamp
test says it observes the log line.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… for J07-005, J14-002, J10-005
An enforced criterion's surfaces must each be declared by a cli_scenario invoke
(repo scan --fail-on-fail, reach --budget and tasks check --warm now have tests that
pass them). The row verdict is decided by session evidence only. --journeys-report
must be a .json path and its directory is created. The J07-005 test reaches the cap,
the J14-002 test runs --estimate-only with and without --include-resources, the
J10-005 test asserts the hashes. J11 is listed as written out in full.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
… and bare vs method calls
Implement the three deferred fixes:
1. Mutation tracking now recognizes AugAssign (c.n += 1), AnnAssign with value
(c.n: int = 5), tuple targets (c.n, y = 5, 6), and setattr() calls. Each pattern
invalidates the constructor echo check (set_then_read flag).
2. Mock echo detection now handles and/or expressions by judging each operand
separately, and correctly rejects tuple comparisons with any non-mock elements
(e.g., (send.call_count, out) == (1, 42) is not mock-only).
3. The no_assert suppression now only recognizes bare function calls with hint
words (verify_result, check_*), not method calls (v.validate).
Added regression tests for all three patterns.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
1. Normalize CLI flags to their canonical form (action.dest) to handle
aliases correctly: -o and --output are now treated as the same flag,
avoiding false failures when tests use short options and the registry
declares long options (or vice versa).
2. Make argparse monkey-patch thread-safe using contextvars.ContextVar,
preventing cross-test contamination when marked tests invoke threads
that also call parse_args(), which would append to this test's calls
list and create flaky journey verdicts.
3. Extend evidence ID matching to recognize both single and double quotes
in test markers: f'"{i}"' and f"'{i}'" are now both recognized,
avoiding false failures for correctly-spelled IDs in tests using
alternative marker syntaxes.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Setting enforce: false on a criterion while retaining passing evidence removes it from unverified and still counts it as passed. Both the PR description and traceability.md:133–134 say unenforced criteria remain unverified. Resolve this reporting mismatch: either include disabled criteria in unverified and adjust the counts and wording, or explicitly document that verification is independent of enforcement.
The reason will be displayed to describe this comment to others. Learn more.
Review at b6a78a7. The seven earlier threads are fixed, and the new tests for them kill the mutants I tried. The new capture and flag-canonicalization code in b6a78a7 is the weak part: it drops calls from worker threads, and no test touches it. Two inline risks to fix before merge.
make ci-local: 19 passed, 0 failed, 4 skipped (docker, semgrep, test-latest, deploy). The test row: 5234 passed, 1 skipped. journeys: pass, 25 enforced criteria, 31 of 56 unverified. mkdocs strict build is clean.
Reproductions for the code findings, with the LLM transport mocked, including a real pytest --outcomes session on a probe file.
Mutants against the updated evidence tests: ignoring --include-resources in the estimate (novelty.py:72) fails test_the_estimate_prices_the_bundled_files_when_they_are_included. Passing warm=False to run_check fails test_tasks_check_warm_runs_null_and_oracle_with_network. Forcing the continue branch to PASS, ignoring the cap, or reporting stopped_by as pool each fail test_sequential_reports_the_cap_as_inconclusive.
I agree with @copilot-pull-request-reviewer on the -- terminator, argparse abbreviations, skipped-test invocations and the stale AC-J11-002 gap. All three still hold at this head. The abbreviation case still comes out error for repo tasks check src --ta x.
Findings without a single anchor:
AGENTS.md:35-37 and dokimi/CONTRIBUTING.md:88-95 still describe pytest --outcomes as checking only tests/outcomes/*.json. Neither mentions journeys.yaml, --journeys-report or journeys-result.json. A contributor who removes a marked cjm-* test follows the guide to outcomes-result.json, which says pass, while CI fails on the journey trace. Nit: add one sentence to each.
Checked, no issues
Surface coverage:static_problems now requires a cli_scenario invoke for each surface of an enforced criterion. test_an_enforced_surface_no_scenario_invokes_is_a_problem covers it, and each invokes entry still needs one argv with all its flags.
Roll-up: a passing unit entry next to a recorded_manual entry keeps the row at pass, and the journey's passed count is 1.
Report path:--journeys-report=report.md is a usage error and sub/dir/j.json is created. Exit code 4 on the bad suffix.
Docs: the 25 enforced and 31 of 56 unverified counts match a full -m outcome run. The four pages are in nav: and docs/README.md, and the links and code-ref tests pass. No changelog fragment is needed, since nothing under dokimi/dokimi/ changed.
…ia unverified
Review of intel#102 at b6a78a7:
- Options are resolved per parser the way argparse reads them: nothing
after "--" is an option, an accepted abbreviation (--ta) counts as its
option, and the identity is the action's first option string rather than
its dest, so --x and --no-x sharing a dest stay two options. A surface
containing "--" is a registry problem.
- Capture goes back to a module global set for the running marked test,
so argvs parsed on worker threads are recorded again. Invocations from a
skipped or failed test no longer count.
- --journeys-report must end in .json; this is checked in
pytest_configure, before the run.
- A missing or unreadable catalog or registry is a problem in the report,
not an exception in pytest_sessionfinish.
- Verified means enforced and passing: an unenforced criterion with
passing tests stays in "unverified" and adds nothing to a journey's
passed count.
- AC-J04-004 and AC-J07-005 are partial and unenforced, with gaps: a
refusal is classified not_loaded, and a capped undecided run prints
PASS (inconclusive) with exit 0. The evidence id becomes
cjm-judge-cap-inconclusive and also backs AC-J11-002, whose gap is
corrected.
- Tests for each rule that a mutation could break (11 mutants, all
killed), and docs: traceability.md, AGENTS.md, CONTRIBUTING.md.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Dropping `parser is target` from _covers, the root-prog check or the
sys.argv fallback in capture, or the misplaced/duplicate use-case
checks in parse_catalog left every test green. One case per guard.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Ran: full suite with --outcomes on the head, and on the head merged into current main, plus ruff, mypy and bandit. Mutants were run against the trace and judge code.
baseline compare --strict with a -7.00 average delta -> exit 0 -> expected non-zero, as the solid regression --> fail arrow (flow-map.md), J12 "Today" and AC-J12-001 say. --strict only refuses a model mismatch. Either mark the J12 gate and AC-J12-001/002 proposed (and add a delivery: proposed registry entry with the gap), or implement it.
flow-map.md cap node says "partial result labelled" -> novelty analyze writes nothing at the cap -> expected wording to match.
AGENTS.md: removing the markers of cjm-repo-reach-harness-failure / cjm-judge-cap-inconclusive -> session exit 0 -> the sentence should be limited to enforce: true criteria, as CONTRIBUTING.md says.
PR body: "only added ids to existing tests" does not hold for three test files.
Pushed:
a37d275 restores the tie-judge FAIL guard the AC-J07-005 rewrite dropped.
399d6a3 reports a duplicate key in journeys.yaml (it silently un-enforced a criterion).
0f47fa6 turns an unbalanced quote or a non-UTF-8 file into a problem: exit 2 and a rewritten report, not a crash.
ef3b93e adds tests for the untested _covers, prog, sys.argv and use-case guards.
The reason will be displayed to describe this comment to others. Learn more.
Review at 12cb8e0. The five threads from f0f06fd are fixed, and the tests added for them kill the mutants I named. The new registry guards have two edge cases that hang or crash the session, and three enforced criteria still pass when the code behind them is broken. Five things are worth fixing before merge, all inline.
make ci-local: 19 passed, 0 failed, 4 skipped (docker, semgrep, test-latest, docs deploy). Changed-line coverage, mypy, ruff, schema-compat, zizmor and the mkdocs strict build pass.
Reproductions for the code findings, with no model or network.
Mutants of the product code behind the enforced criteria, run against the marked tests only (-m outcome) and then against the whole test file; mutants of plugin.py and journeys.py run against test_journeys_trace.py and test_outcomes_plugin.py.
I agree with @xaleryb on baseline compare --strict and AC-J12-001. New evidence: baseline.py:193-202 holds the only --strict exits, both on a model mismatch; nothing there compares a score delta.
Checked, no issues
Earlier threads: reverting the setup-skip filter to not report.failed fails both session cases (skipif and the skipping fixture). cap=None at commands/novelty.py:868 fails test_every_role_shares_one_cap and test_the_cap_gets_through_a_stage_that_absorbs_failed_calls under -m outcome. rd.is_dir() and not force at repos/runner.py:148 fails test_score_only_task_and_missing_cells.
New guards: the prog, sys.argv and _covers guards and the UnicodeDecodeError handling are killed by the new tests; the same goes for the _normalize and _unsplittable mutants I tried. NULs in refs and surfaces, null and integer criterion keys, aliased and merged mappings, and a multi-document stream produce problems and a report.
Docs: the commands and flags the journey pages name resolve in build_parser(). The cap node in flow-map.md now matches novelty analyze, which writes nothing at the cap. The 24 enforced and 37 of 61 unverified counts match journeys.md and journeys.yaml.
…tation and marker checks
A non-empty tuple assert never fails, so assert (out == 5, 'msg') is no check.
assert_* attributes count as always-true only for uncalled mock verify methods.
Constructor echo tracking also counts keyword values, container arguments,
subscript stores and for/with rebinds. The exemption marker needs a whitespace
separated reason. The analyzer API test pins the dot-only code line.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…; test the enforced gates
A YAML anchor that holds itself made the registry walk loop forever, and a
NUL byte in catalog: raised outside the report. Both are now problems in the
report. The enforced criteria AC-J03-004, AC-J04-001 and AC-J07-003 had tests
that a mutant of the gate passed; they now have marked tests for the batch
refusal, the fail-on-fail exit and the isolation probe script itself.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Review at 4a60fb2. Four of the five threads from 12cb8e0 are fixed and the new tests kill the mutants I tried. The NUL fix is incomplete, and a malformed registry can still crash the session in two more ways. One risk and one nit are inline.
make ci-local: 19 passed, 0 failed, 4 skipped (docker, semgrep, test-latest, docs deploy). Changed-line coverage, mypy, ruff, schema-compat, zizmor and the mkdocs strict build pass.
test_repo_l0.py, test_repo_run.py and test_repo_runner.py: 97 passed. The three new tests passed 5 runs out of 5 and under -n2.
Reproductions for the code findings, with no model or network.
19 mutants of the code behind the three new tests, and 5 mutants of the self-reference guard.
Still open from my earlier review:
dokimi/tests/outcomes/journeys.py:432: a NUL in a directory part of catalog: still crashes check(). schema: 1\ncatalog: "a\0b/c"\ncriteria: {}\n passes the new except in parse_catalog, then catalog_path.parent.glob("*.md") raises ValueError: scandir: embedded null character in path (glob.py:428) and no report is written. The new test uses "a\0b", whose parent is clean. Suggestion: move the glob into the try, or read the pages inside the same guard, and add "a\0b/c" to the test.
Checked, no issues
Self-reference guard: a shared anchor, a <<: *x merge key and [*x, *x] return False; self, indirect and nested cycles return True. After a cycle check() returns verdict error, markdown(report) renders, and _repeated_keys is skipped. Dropping the state == 1 check, or making _refers_to_itself always False, fails the new test.
AC-J04-001:if args.fail_on_fail: alone, an inverted condition, a changed exit code and dropping the incomplete half each fail test_fail_on_fail_follows_the_result_not_the_flag.
AC-J03-004:tasks[:1], tasks[-1:] and refusing only an all-egress batch each fail the new batch test. The batch holds fix_add, review_add and fix_pip, with fix_pip last, and any started cell would leave a directory besides _eval.
AC-J07-003:exit 0 first, and no bad=1 in the visible, guidance and task-name branches, each fail the probe test.
Counts:journeys.yaml has 24 enforce: true entries and the catalog has 61 criteria, so 37 are unverified, as the description says.
_passed_names read list, tuple and set arguments but not dict values,
so mutate({"k": c}) and mutate(**{"k": c}) left the constructor echo
check standing. Add regression fixtures for both spellings, and pin the
remaining always-true mock verify names plus an empty tuple assert.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
baseline compare --strict exits 0 on a -7.00 regression: not changed in this push. Either option (mark J12 gate and AC-J12-001/002 proposed with a delivery: proposed registry entry, or implement the non-zero exit) is an author decision, so it is left open.
flow-map.md cap node: fixed in 12cb8e0, now "Stopped at cap, no result written".
AGENTS.md cjm-* sentence: fixed in 12cb8e0, limited to enforce: true criteria as CONTRIBUTING.md says.
PR body "only added ids" claim: the body now says most of the change is ids and lists the other test edits.
Your pushed commits a37d275, 399d6a3, 0f47fa6, ef3b93e are kept as they are. The affected modules (tests/outcomes, test_journeys_trace.py, test_judge_reliability.py, test_repo_runner.py, test_model_compare.py, test_novelty_*, docs tests) pass at 266537d: 406 passed.
…blem
yaml.safe_load raises RecursionError on deep nesting, which the load
except did not cover, and Path.exists raises OSError on a name the file
system refuses. Both ended the journeys check with a traceback and no
report. The first is now a cannot-read problem, the second a missing
path. The isolation probe gains a case with a task name inside another
file name (kills grep -F for -Fx), and AC-J07-003 names what the shim
does not exercise.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…docs
The four docs/user-journeys pages are linked from docs/README.md and are
product docs, so they belong in the static benchmark corpus. Raise the
ceiling in the same commit, as the comment above it asks.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Option a applied in 616d813 (pushed to napetrov/dokimi cjm-journeys):
tests/outcomes/journeys.yaml: new J12 block, AC-J12-001 and AC-J12-002 are delivery: proposed (not enforced) with gaps saying the strict compare fails only on an unverified or mismatched model, and no threshold or blocking policy exists.
docs/user-journeys/journeys.md (J12 "Today"): states that baseline compare --strict exits 0 on a regression, and that failing on a regression is proposed.
docs/user-journeys/flow-map.md: the regression arrow now goes to a report node (exit 0 today); the regression->fail edge is dashed and labelled proposed.
No code change, so no regression test. Tests: journey, docs-link and code-ref tests pass; ruff clean; --outcomes registry reports no problems.
baseline compare --strict exits 0 on a regression; only an unverified or
mismatched model fails it. Register AC-J12-001 and AC-J12-002 as
delivery: proposed with gaps, and say in journeys.md and flow-map.md that
failing on a regression is proposed, not current.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Merging main brought in two must-not-crash tests that only assert the payload
is not None and a gate test that asserts nothing, so the gate test
test_no_new_test_that_cannot_fail fails at the head. Validate the payload
against its schema; check the clean tree's budget line and that no warning prints.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
From Python 3.13, catalog: "a\0b/c" makes the glob of the catalog's directory
raise ValueError after parse_catalog was guarded, so check() wrote no report.
Read the pages inside the same guard.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
…twice
An alias composes to the anchored node itself, so the walk in _repeated_keys
(added in 399d6a3) followed every alias: exponential time in nested aliases,
which safe_load reads instantly, and one problem line per alias.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Re-ran at 616d813: the full suite, ruff, mypy and bandit, plus the earlier baseline compare --strict and cjm-* marker inputs. The J12 text and registry now match the behaviour, and the AGENTS.md and cap-node fixes hold.
Tests: test_no_new_test_that_cannot_fail fails on three tests merged from main (test_deeply_nested_yaml_does_not_crash, test_odd_manifests_do_not_crash, test_gate_passes_a_clean_tree). Expected: green.
catalog: "a\0b/c" on Python 3.13+: the catalog-directory glob raises ValueError and no report is written. Expected: a problem and a report.
The reason will be displayed to describe this comment to others. Learn more.
Review at aaebb49. The NUL and deep-nesting fixes hold, and the J12 text now matches baseline compare --strict. One thread from 4a60fb2 is still open: the isolation test flakes when two runs share a host. One nit inline.
make ci-local: 25 passed, 1 failed, 4 skipped. The test row had 6559 passed and 1 failed: test_unbalanced_quote_in_hook_completes_quickly took 2.13 s against a 1 s limit while other jobs ran in parallel. It passes alone in 0.8 s, and the test is not touched by this PR. Changed-line coverage, mypy, ruff, schema-compat, zizmor, gitleaks and the mkdocs build pass.
pytest --outcomes -m outcome on a copy: journeys: pass, 24 enforced criteria, 37 of 61 unverified.
Reproductions for the findings, with no model or network.
9 mutants of the four recent journeys.py fixes (glob inside the guard, RecursionError, OSError in _ref_problem, the seen set): 8 killed by test_journeys_trace.py, 1 survives (dedupe by source line instead of node identity). Inputs that I tried against check() give a problem and a report: NUL in refs and in the catalog directory, a catalog path that is a directory, a 5000-character name, 500-level block nesting, an undecodable page in the catalog directory, and a 40-deep chain of aliases.
9 mutants of the isolation probe script, run against the whole tests/test_repo*.py set.
Still open from my earlier review:
dokimi/tests/test_repo_runner.py:368: the probe test still writes a fixed dokimi-tasks file in the system temp directory (selfcheck.py:50) and leaves it behind. Eight copies of the test run at once on one host fail 13 times out of 48, each at test_repo_runner.py:408 (named.returncode is 0, not 1): one run overwrites the task list another is reading, and the probe reports a clean image. Suggestion: have the script write the list to $(mktemp) and remove it at the end. That touches dokimi/dokimi/repos/selfcheck.py, so it needs a changelog fragment. The gaps line added for AC-J07-003 now covers the find root, the prune and the source mount.
Checked, no issues
Thread fixes: the catalog-directory NUL ("a\0b/c") is a cannot read problem with a report, and reverting the glob move fails the new parametrized test. The deep-nesting and long-ref cases behave the same way.
Quality gate: the three tests changed in 57d82f0 fail when the code they cover is broken (for example, removing the RecursionError catch in checks.py fails test_deeply_nested_yaml_does_not_crash). test_audit_test_quality.py passes: 99 tests.
Docs and rules: the four journey pages are in nav:, the mkdocs strict build passes, the docs link and code-ref tests pass, .gitignore covers the reports, and dokimi/dokimi/ has no change from this PR, so no changelog fragment is needed.
Say that baseline compare --strict adds a non-zero exit only for an unverified or differing model, in the J12 text and the AC-J12-001 gaps line
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I pushed 787c8da, which fixes the nit from my review:
dokimi/docs/user-journeys/journeys.md:663 and dokimi/tests/outcomes/journeys.yaml:356: both now say baseline compare --strict adds a non-zero exit only for an unverified or differing model; a score regression still exits 0.
Left as is:
dokimi/dokimi/repos/selfcheck.py:50: moving the task list to a mktemp file changes the probe script's behaviour and needs a new changelog fragment, so it stays as it is.
make ci-local at 787c8da: 26 passed, 0 failed, 4 skipped.
main carries an older copy of scripts/audit_test_quality.py and its test from intel#102;
the branch's reviewed version is kept (main had nothing the branch lacks).
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
antonwolfy
left a comment
Adds the customer-journey catalog (J1–J15) and a criterion-level evidence trace on top of the existing
pytest --outcomesplugin. Tracking: #95 (children #96–#100). This PR does not redesign the CLI, and it does not commit to any future command layout.Summary
How a test session decides each criterion's verdict:
partialand not enforced.repo tasks checknull/oracle/isolation self-check, with no claim of relevance or coverage.not_loaded) and AC-J07-005 (a capped run printsPASS (inconclusive)), which have stated gaps.@pytest.mark.outcomeids. Beyond ids:test_docs_code_refs.pyandtest_docs_links.pyread the newuser-journeysdocs;test_judge_reliability.py,test_model_compare.pyandtest_novelty_journey.pyhave assertions rewritten or a test replaced;test_outcomes_plugin.pyresetsREGISTRY;test_repo_runner.pygains a new test.OUTCOME-PINNED-QUESTIONSkeeps its semantics.--journeys-reportand uploads the report with thecoverageartifact.dokimi/dokimi/changed.Inspected commit:
f76a76020b987bf51c56dfd1a98830a3639ece84(theintel/maintip when the branch was cut).Evidence
pytest -n 32 --outcomes --journeys-report=journeys-result.json→5249 passed, 2 skipped,outcome OUTCOME-PINNED-QUESTIONS: pass,journeys: pass, 24 enforced criteria, 37 of 61 unverified(at 68e79ce).Negative checks in
tests/test_journeys_trace.py, each as an inner pytester session or a direct check:make ci-local: every row passed excepttest, including mypy, ruff, schema-compat, mkdocs build and package-smoke.testrow failed twice on timing-sensitive process-kill tests (test_reach_timeout_kills_the_harness_process_tree,test_a_timed_out_scenario_keeps_its_trace_up_to_the_kill,test_example_runner_processes). A different set failed each time. None of them is marked or touched by this PR. The shared host had load ~259 on 224 cores with ~200 xdist workers. The same tests pass 3/3 in isolation, and the full suite passes at-n 32.external-catalog-smokejob and semgrep were skipped locally.Merge Danger
Door: two-way
Docs, tests and one CI flag; there are no package changes. Reverting removes the trace.
Blast Radius: test-suite
pytest --outcomesuntiljourneys.yamlis updated in the same PR. That failure is intended.--outcomesalready errors on unclaimed evidence. This PR adds the journey ids to that.Open questions, not decided in this PR: who declares a whole journey complete, and who owns each journey (#95).
🤖 Generated with Claude Code