Skip to content

feat(novelty): analyze states the measured share of readable words on stdout - #173

Open
napetrov wants to merge 13 commits into
intel:mainfrom
napetrov:fix/skill-gap-W5
Open

napetrov wants to merge 13 commits into
intel:mainfrom
napetrov:fix/skill-gap-W5

Conversation

@napetrov

Copy link
Copy Markdown
Contributor

Part of #4.
Depends on #166: both touch commands/novelty.py, so this rebases cleanly only after #166 merges.

Summary

novelty analyze states on stdout how much of the artifact's readable words its run measured, and warns on stdout when that is half or less.

 analyze headline (stdout)
   ... degraded: N, dead tokens: X
+  coverage: 38% of the artifact's readable words were measured (entry point only)
+  WARNING: only 38% of the artifact's readable words were measured, so every rate here
+           describes that part, not the artifact; pass --include-resources to measure the bundled files
   WARNING ... (stderr, unchanged)
 novelty.v1 bundle record
+  measured_share        1 - unmeasured share on the readable basis, 4 places, null when unknowable

The threshold is the one the analyzer already used for its stderr warning, so the same runs warn as before. Only the channel changed.

Evidence

Five new tests fail against intel/main (commands/novelty.py and novelty/bundle.py reverted, the rest of the branch kept); the other 153 in the four touched test files pass there:

before: commands/novelty.py and novelty/bundle.py reverted to intel/main
  FAILED test_the_record_states_the_share_of_readable_words_this_run_read
  FAILED test_exactly_half_the_readable_words_read_is_warned_as_the_analyzer_always_has_been
  FAILED test_a_bundle_run_that_read_everything_it_could_measures_all_of_it
  FAILED tests/test_novelty_cli.py::test_the_headline_states_the_measured_share_and_warns_on_stdout_at_half_or_less
  FAILED tests/test_novelty_journey.py::test_the_stdout_headline_says_how_much_of_the_artifact_was_read
  5 failed, 153 passed   (test_novelty_bundle, _cli, _journey, _settings_parity)

after: this branch, full suite below

Local checks on the rebased head:

uv run ruff check .                              All checks passed!
uv run mypy dokimi                               Success: no issues found in 222 source files
uv run python -m dokimi.schema_check             exit 0
uv run python -m dokimi.config_check             exit 0
uv run pytest -n auto -q                         7073 passed, 22 skipped

Merge Danger

Door: two-way. The change adds a line and a record field; nothing is computed differently.

Blast Radius: low. The stderr warning and the threshold are unchanged.

  • New stdout lines in the headline. Every analyze headline gains a coverage: line, and a run at or under half gains a second WARNING: line on stdout. A parser that reads the headline by position needs updating.
  • New record field. bundle.measured_share is added to the novelty.v1 bundle record and to the schema. It is null when a bundled file did not decode, so readers must allow null.
  • Rates quoted from entry-point runs now carry the caveat where they are copied (the changelog says so). No numbers change.
  • Not in this PR: flipping the --include-resources default. The PR proposes it as a follow-up and does not make it. Turning it on would change every entry-point number and the cost of every analyze run, so it needs a decision of its own.

Checklist

  • Tests added/updated and pytest passes
  • ruff, mypy, and schema checks pass
  • Docs updated: docs/skill-novelty.md (bundle section)
  • No ad-hoc run artifacts committed
  • New code and test files carry the SPDX license header (no new code files; the changelog fragment follows the existing fragments, which carry none)

🤖 Generated with Claude Code

…yze headline

The headline printed the added and already-knew counts but not how much of the
artifact the run read, so an entry-point run of a progressive-disclosure skill
quoted rates that describe the entry point as if they described the skill.

The headline now prints the measured share of readable words on stdout, and
warns on stdout when a run read half or less of them. The novelty.v1 bundle
record stamps the share as bundle.measured_share.

The stdout warning uses the line the analyzer has always warned at (an outside
share of 0.5 or more), so the stdout and stderr warnings agree and the coverage
parity golden keeps its value. The settings parity module leaves out the new
stdout lines when it compares with main; the journey test pins them instead.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ity checks the headline

- novelty.v1 bundle: declare measured_share (number or null, 0 to 1) beside
  measurable_outside_share; the record already stamps it.
- Add the scriv fragment the package change needs: Added for the headline
  coverage line and bundle.measured_share, Changed for the stdout warning.
- docs/skill-novelty.md: "a run that read half or less ... says so on stdout"
  was ungrammatical ("an run"); split it into its own sentence.
- settings parity: the coverage and WARNING lines are no longer dropped
  unchecked. Main's golden cannot hold them, so each analyze step checks its
  stdout coverage lines against the measured_share its own novelty.json
  stamped: the percentage, and the WARNING exactly when the share is 0.5 or less.

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

codecov-commenter commented Oct 22, 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 92.30769% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.99%. Comparing base (9a93635) to head (bdb9047).

Files with missing lines Patch % Lines
dokimi/dokimi/commands/novelty.py 88.88% 1 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     #173   +/-   ##
=======================================
  Coverage   91.98%   91.99%           
=======================================
  Files         223      223           
  Lines       27743    27768   +25     
  Branches     4560     4565    +5     
=======================================
+ Hits        25520    25545   +25     
+ Misses       1574     1573    -1     
- Partials      649      650    +1     
Files with missing lines Coverage Δ
dokimi/dokimi/metrics/bundle_coverage.py 100.00% <100.00%> (ø)
dokimi/dokimi/novelty/bundle.py 95.22% <100.00%> (ø)
dokimi/dokimi/commands/novelty.py 95.56% <88.88%> (-0.28%) ⬇️

... and 1 file with indirect coverage changes

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

@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 db55ba0. The record field and the schema widening are right, and the stderr output is unchanged from the base. The stdout percentage is wrong for three shares, and the new stdout warning fires for runs that never warned before. The percentage bug and the risk inline are worth fixing before merge, as are the three points below.

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

  • make ci-local: 20 passed, 8 failed, 7 skipped. All 8 failures are exit 127 (ruff, zizmor, pre-commit, bandit, mypy, deptry, mkdocs, pip-audit missing from my venv): make dev stops at the tools install on this base, which #179 fixed on main. Static checks, mypy and the strict docs build pass on GitHub's CI at this head. The changelog check and the test jobs passed.
  • The four touched test files: 158 passed.
  • Reproductions for the code findings with synthetic bundle records, no model calls.
  • Mutation run on the new tests: the <= / < boundary, int / round, the threshold, the scope suffix and the >= / > stderr condition are all killed. Five mutants pass the full suite: always appending the --include-resources advice, dropping coverage.include_resources from the advice condition, dropping the zero-words guard, dropping round(..., 4) in measured_share, and dropping "null" from the schema type.

Findings that cannot be anchored inline:

  1. The stdout warning is not "the same runs as before" for --include-resources runs. Before the PR only an entry-point run with outside_share >= 0.5 warned (bundle.py:461). _print_coverage warns on any run with measured_share <= 0.5, bundle runs included. A --include-resources run with max_file_words=50, a 100-word entry and a 500-word reference measures 100 of 600 words. Head prints coverage: 16% ... and WARNING: only 16% .... The base printed nothing about coverage, and the warnings list is identical on both. So the changelog line "the WARNING: only N% ... line goes to stdout as well as the stderr warning" and "the same runs warn as before" (changelog.d/...measured-share.md:11-14) are false for budget-truncated bundle runs, and the PR description's "only the channel changed" too. Warning there is arguably right, but say so in the changelog, or restrict the warning to what the stderr one covers.
  2. docs/skill-novelty.md:492-496 and the schema description do not cover the new cases. The docs say a run that read half or less "says so on stdout", but not that coverage: unknown exists, that bundle.measured_share can be null, or that bundle runs now warn. The bundle description string in novelty.v1.json lists the record's fields and does not name measured_share.
  3. Test gap on the advice and the null type. The ; pass --include-resources ... suffix is asserted only where it appears. Mutation check: always appending it, dropping coverage.include_resources from its condition, and dropping or not coverage.measurable_resource_words all pass the full suite (158 passed in the four touched files). A budget-capped --include-resources run could then tell the user to pass the flag they already passed. Removing "null" from the measured_share type in the schema also passes, although the code emits None for an undecodable file. Add a CLI test for a bundle run at 30% that asserts the line has no --include-resources advice, and a schema validation of a record with measured_share: null.

Checked, no issues

  • Consistency: measured_share equals 1 - measurable_outside_share for entry-point runs, as the docs say, and (entry + measured) / (entry + measurable) for truncated bundle runs.
  • Stderr: the warnings list and stderr output are identical to the base in every scenario I ran. 1 - 0.5 is exactly 0.5, so >= has no float edge.
  • Schema: an optional property added in place; old artifacts still validate. No version bump is needed under AGENTS.md.
  • Consumers: nothing in the repo parses the headline by position. _print_headline has one caller.
  • Repo rules: the changelog fragment follows changelog.d/README.md, and the new tests are offline.

Comment thread dokimi/dokimi/commands/novelty.py Outdated
Comment thread dokimi/dokimi/commands/novelty.py Outdated
Comment thread dokimi/dokimi/commands/novelty.py Outdated
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>

# Conflicts:
#	dokimi/dokimi/novelty/bundle.py
@napetrov

Copy link
Copy Markdown
Contributor Author

Review-body items from the review at db55ba0, in 3872e1c:

  1. Stdout warning wider than before for bundle runs: kept (a budget-capped bundle run is a partial read too), and the changelog and docs now say so instead of 'the same runs warn as before'.
  2. Docs and schema: skill-novelty.md covers coverage: unknown, a null bundle.measured_share and bundle-run warnings; the novelty.v1 bundle description names measured_share.
  3. Test gap: test_a_bundle_run_capped_by_the_budget_warns_without_advising_the_flag_it_passed (no --include-resources advice on a bundle run), and test_a_record_with_an_unknowable_share_validates (null validates, a string does not).
    Also checked the merge of intel/main (coverage code moved to metrics/bundle_coverage.py): MEASURED_SHARE_WARN_AT, the measured_share property, the bundle.py threshold and record field, and the schema entry are all present; nothing from the branch was lost.

…are keeps scope and warning

0.29, 0.57 and 0.58 printed one too low (float product). An undecodable file
printed coverage: unknown with no scope and no warning. Docs, changelog and the
schema description now cover the bundle-run warning and the null share.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…e; chore(growth): raise commands/novelty.py for W5

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.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.

The three points from my earlier review are fixed: the percentage floors the stored 4-place share, the coverage: unknown line keeps its scope and warns, and the comment is accurate. One risk and three nits remain, all inline.

What I ran, on a Linux review host at 62b87f9:

  • make ci-local: 28 passed, 1 failed, 7 skipped. The failure is pip-audit (gitpython 3.2.0, CVE-2026-114145); the PR changes no dependency or lock file, so it is not from this PR. Tests, changed-line coverage, ruff, mypy, schema_compat, the docs build and the changelog check pass.
  • Reproductions with synthetic bundle records, no model calls.
  • A mutation run over _print_coverage, Coverage.measured_share, the bundle.py threshold and the schema: 26 mutants, 6 of which pass the five touched test files.

Checked, no issues

  • Test move: test_novelty_bundle_key.py holds all 22 items removed from test_novelty_bundle.py unchanged (compared with ast), plus the three new tests.
  • Percentage: round(share * 10000) // 100 is right for 29, 57, 58 and 49; the stored share is rounded once, so there is no double rounding.
  • Stderr and schema: the warnings list is unchanged; measured_share is an optional nullable property added in place, so no version bump is needed.
  • Docs claims: the changelog now names the two cases where stdout warns more widely than stderr, and the code agrees.


# A file that did not decode leaves the share unknowable, and the headline says so,
# with the scope and, while the entry point is half or less of the words, the warning.
novelty_cmd._print_headline(dict(artifact, bundle=dict(record, unreadable_files=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.

The unknown-share branch and its advice guards are only half pinned. Mutation check: each of these passes the five touched test files (165 passed):

  • replacing not coverage.include_resources with True in the coverage: unknown warning (novelty.py:850-852). An --include-resources run with an undecodable file and a small entry point would then print "the entry point is half or less of the artifact's readable words", which is false for a bundle run;
  • changing >= to > in the same condition. No test uses an entry point of exactly half the words, so stdout would stop warning where the stderr warning (bundle.py:224, >=) still fires;
  • dropping or not coverage.measurable_resource_words from rerun. An entry-point run whose bundled files are all policy-excluded would be told to pass a flag that reads nothing more;
  • dropping round(..., 4) in Coverage.measured_share. The record then stores 0.2857142857 and every test follows it.

Suggestion: add cases to this test and the parametrized one: unreadable_files=1 with include_resources=True and entry_point_words=10 asserts no WARNING; unreadable_files=1 with 50 entry words and 50 on disk asserts the warning; an entry-point run with policy_excluded_words equal to the resource words asserts no pass --include-resources; and a record with 2 entry words and 7 resource words asserts bundle["measured_share"] == 0.2222.

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.

Added in 8739308: test_an_undecodable_file_in_a_bundle_run_does_not_claim_an_entry_point_only_warning (include-resources with an undecodable file prints no WARNING), test_an_unknown_share_warns_at_exactly_half_as_the_stderr_warning_does (50 of 100 words warns), and test_the_stored_measured_share_is_rounded_to_four_places (2 entry + 7 resource words stores 0.2222). Verified each fails with the guard changed (True for the include-resources check, > for >=, no round). For the policy-excluded 'rerun' term: it cannot matter, since a known share only warns when readable words lie outside the entry point, and the unknown-share warning needs the same, so measurable_resource_words is never 0 there; I removed the dead condition instead of testing it.

Comment thread dokimi/dokimi/commands/novelty.py Outdated
Comment thread dokimi/scripts/growth_baseline.json Outdated
Comment thread dokimi/docs/skill-novelty.md Outdated
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
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 c9e5a77, which fix the three nits from my review:

  • d7db0f4: dokimi/dokimi/commands/novelty.py:861: a share above 0 and under 0.01 now prints <1%, with a test for 1 entry word in 10,000.
  • 95b8380: dokimi/scripts/growth_baseline.json:3: the rationale now reads "stdout coverage line and warning (_print_coverage); split tracked separately".
  • c9e5a77: dokimi/docs/skill-novelty.md:499: the doc says 1 - measurable_outside_share to 4 places, and the schema says measured_share is null when a file did not decode or there are no readable words.

make ci-local at c9e5a77: 28 passed, 1 failed, 7 skipped (the failure is pip-audit, as at the reviewed head).

Not approving yet. Still open: the risk from my review.

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

# Conflicts:
#	dokimi/scripts/growth_baseline.json
…asured_share

Cover the include-resources guard and the exact-half boundary of the
coverage: unknown warning, and the rounding of the stored share. Drop the
unreachable measurable_resource_words term from the rerun advice.

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

# Conflicts:
#	dokimi/scripts/growth_baseline.json

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants