Skip to content

CI: fail when the engine names a specific application - #1749

Merged
abrichr merged 25 commits into
mainfrom
claude/engine-generality-guard
Nov 4, 2026
Merged

abrichr merged 25 commits into
mainfrom
claude/engine-generality-guard

Conversation

@abrichr

@abrichr abrichr commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

The Flow lint job now rejects new application names and app-specific identifiers in the engine. Application data and the existing demo-module exceptions stay explicit in scripts/generality.toml; all other existing hits form a per-file, per-term ratchet that can only shrink. CONTRIBUTING explains where application knowledge belongs.

After #1760, the baseline falls from 425 to 358 hits, including the MockLoan server retained for installed OpenEnv use. Three unchanged identifiers moved from reward/seed.py to reward/bundle.py; their existing allowances moved with them. Two newer prose examples now use generic names. The checker, denylist and safety tests are unchanged.

Tested: 10 guard tests; Flow Ruff and formatting checks (1307 files); mypy (585 files). A separate scan of current main (9ee6030) with only the seven comment/docstring edits reports no new or stale entries. No application or lab run was needed.

Updated head: f7a173b. The two local documentation conflicts preserve every changelog entry and the guard PR's updated audit paragraph. The existing PR branch was fast-forwarded without force-pushing.

Current main (3d6b0b1) is now a parent of the branch. The diff against main is only the 15 guard/prose files (+711/-14); already-merged benchmark/results files are absent. Validation after this integration: 62 guard/reward/environment tests pass; Flow Ruff/format pass (1312 files); mypy passes (588 files). The reviewed checker, denylist and 358-hit baseline are unchanged.

Audit every place the engine (packages/flow/openadapt_flow) names a
specific application, bucketed as example, data, harness in the package,
or real special-casing: docs/GENERALITY_AUDIT.md.

Move openadapt_flow/benchmark and openadapt_flow/instrument to
packages/flow/benchmark/harness (import benchmark.harness). The wheel
packages only openadapt_flow, so it no longer ships them. The benchmark
command loads the harness from a source checkout and says so from a
wheel; instrument already failed to import from a wheel.

Reword two comments that named an app on a generic branch.
scripts/check_engine_generality.py scans openadapt_flow/ for the
application names and app-specific identifiers in scripts/generality.toml.
app_profiles/ is data and passes; OpenAdapt's demo modules may name the
demo app; a comment line may give an example under '# example:'. Hits
that predate the check are counted in generality_baseline.json, which
only shrinks: a rise fails, and so does a fall the baseline doesn't
record. Runs in the flow lint job. CONTRIBUTING documents the rule:
app knowledge lives in profiles, contracts and fixtures.
Audit every place the engine (packages/flow/openadapt_flow) names a
specific application, bucketed as example, data, harness in the package,
or real special-casing: docs/GENERALITY_AUDIT.md.

Move openadapt_flow/benchmark and openadapt_flow/instrument to
packages/flow/benchmark/harness (import benchmark.harness). The wheel
packages only openadapt_flow, so it no longer ships them. The benchmark
command loads the harness from a source checkout and says so from a
wheel; instrument already failed to import from a wheel.

Reword two comments that named an app on a generic branch.
@abrichr
abrichr force-pushed the claude/generality-audit-83a5d8 branch from 22de5f6 to 7d884c0 Compare November 3, 2026 19:04
A comment that landed on main named the lab's Patient Notes app beside
a tolerance; the guard caught it. The example marker also accepts '#:'
doc-comments.
The wheel ships only the engine (see packages/flow/docs/GENERALITY_AUDIT.md).
Moved out of openadapt_flow:

- integrations/harness2 -> benchmark/harness/harness2 (the harness2 extra
  is dropped; install [browser] beside Harness² from a checkout)
- mockloan/ and demo_driver.record_disbursement_demo ->
  benchmark/harness/mockloan/ (recorder in demo.py)
- guard_demo.py -> benchmark/harness/guard_demo.py
- agent/ledger_demo.py -> packages/agent/tests/ledger_demo.py
- mockmed/teach_scope.py -> packages/flow/tests/_teach_scope.py
- learning/synth_stream.py: StructuralDiffInducer -> learning/induce.py;
  the MockMed drift stream -> benchmark/harness/synth_stream.py

reward/seed.py is split: the bundle helpers (write_bundle, calibrate_bundle,
self_signed_certificate) are now reward/bundle.py. The MockMed seed stays,
because serve-reward --seed-mockmed needs it from a wheel.
…move-harnesses

# Conflicts:
#	packages/flow/CHANGELOG.md
#	packages/flow/docs/GENERALITY_AUDIT.md
#	packages/flow/tests/test_lending_swer.py
Base automatically changed from claude/generality-audit-83a5d8 to main November 4, 2026 01:11
…e-harnesses

# Conflicts:
#	packages/flow/CHANGELOG.md
#	packages/flow/benchmark/harness/run_benchmark.py
#	packages/flow/docs/GENERALITY_AUDIT.md
#	packages/flow/openadapt_flow/__main__.py
#	packages/flow/pyproject.toml
#	packages/flow/tests/test_lending_swer.py
#	packages/flow/tests/test_runtime_never_imports_benchmark.py
…ity-guard

# Conflicts:
#	.github/workflows/packages.yml
#	packages/flow/CHANGELOG.md
#	packages/flow/benchmark/harness/run_benchmark.py
#	packages/flow/docs/GENERALITY_AUDIT.md
#	packages/flow/openadapt_flow/__main__.py
#	packages/flow/pyproject.toml
#	packages/flow/tests/test_runtime_never_imports_benchmark.py
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent review (AI, adversarial)

Verdict: changes needed (at 10e3734)

Blocking

  1. The branch rolled back recent work on main. These were stale merge resolutions:
    • pyproject.toml dropped the /benchmark/harness/reliability_corpus.py exclusion.
    • packages.yml dropped benchmark/harness from ruff, and dropped tests/e2e/test_code_agent_default_e2e.py from the e2e list.
    • __main__.py went from sys.path.append back to insert(0, ...).
    • test_runtime_never_imports_benchmark.py lost lazy_ok.
    • pyproject.toml re-added a mypy override.
  2. Names inside snake_case or camelCase identifiers went uncounted (\b treats _ as a word character). _openemr_fixup, APP_OPENEMR, isOpenEMR and to_stedi_body all passed the check, and the baseline undercounted.

Non-blocking

  1. The # example: marker counted anywhere on a comment line.
  2. epic was case-sensitive (EPIC_BASE passed), and tabLoan\b missed tabLoanApplication.
  3. Files with other suffixes are not scanned (none hold names today).
  4. A count ratchet lets you swap one hit for another within a file.
  5. import tomllib fails on Python 3.10.

Checked and fine

  • The CI working directory and path resolution.
  • The ratchet logic.
  • The CONTRIBUTING wording is positive.
  • No prospect names, no GPL material, and both data files are in the public-artifact inventory.

Response (5e3742e and the follow-up that undoes an unrelated scripts/ reformat)

  • 1: Every file the guard has no reason to touch is restored to main's version. The diff against main is now 13 files, all guard-related.
  • 2: Name patterns now stop only at a letter or digit next to the name. camelCase is split before matching (isOpenEMR reads as is Open EMR). TextEdit and InvenTree accept a separator. New tests cover _openemr_fixup, APP_OPENEMR, isOpenEMR, to_stedi_body, EPIC_BASE and tabLoanApplication. The baseline was rebuilt from a fresh scan: 426 hits, up from 365. The added hits are names inside identifiers that were already in the engine.
  • 3: The marker has to open the comment. A test covers it.
  • 4: epic now also matches EPIC, and tabLoan now matches as a prefix.
  • 7: The script falls back to tomli, which the package already requires on Python 3.10.
  • 5 and 6: Left as they are. They are worth tightening once a hit shows up there.

Guard tests: 10 passed. check_engine_generality.py passes against the new baseline.

@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent re-review (AI) at 8a01ffe

Verdict: looks good. Nothing blocking is left.

  • Point 1: pyproject, __main__.py and the import-guard test match main. The workflow change is only the new step.
  • Point 2: is_openemr, APP_OPENEMR, isOpenEMR, OPENEMRClient and to_stedi_body now fail the check.
  • Point 3: x='OpenEMR' # example: and # OpenEMR #example: now fail.
  • Points 4 and 7: EPIC_BASE and tabLoanApplication now match, and the tomli fallback is in place.

Two new minor findings:

  • text[ _-]?edit and mock[ _-]?med match ordinary words (text_edit, mock_media).
  • Epic matches the ordinary word at the start of a sentence.

Response (88eaf68)

  • Ordinary words: name patterns now match the raw line or the camelCase-split line. That lets textedit and inventree drop their separator, and mockmed stops at a letter or digit. text_edit and mock_media pass, while TextEdit and MockMedServer still fail, as the tests show. The one ordinary-word hit (dense_surface.py, "a text edit") is out of the baseline, which is now 425.
  • Epic: left case-sensitive (Epic and EPIC). Lowercase "epic" is too common a word to deny.

Guard tests: 10 passed.

Signed-off-by: abrichr <richard.abrich@mldsai.com>
…into agent/97

Signed-off-by: abrichr <richard.abrich@mldsai.com>

# Conflicts:
#	packages/flow/CHANGELOG.md
#	packages/flow/docs/GENERALITY_AUDIT.md
Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Updated to cdf9c99 after #1760. The baseline is now 358 (from 425), including the retained MockLoan server; three unchanged moved identifiers retain their exact existing allowances. Two prose examples became generic. No checker or denylist changes.

All 10 guard tests pass; Flow Ruff/format and mypy pass. A separate current-main scan with only the prose edits also has no new or stale entries. The branch was fast-forwarded, without force-pushing. Please review the baseline refresh.

@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr

abrichr commented Nov 4, 2026 •

Copy link
Copy Markdown
Member Author

Independent adversarial review: APPROVE

From: Grok. Reviewed head cdf9c99.

The branch's merge-base (b21a86d) is behind main, so a plain git diff origin/main HEAD looks like it deletes tests. I checked the real delta with a local, uncommitted test merge of origin/main into the head. That merge had no conflicts and was aborted afterwards; nothing was pushed. Against main it changes 15 files (+711, -14):

  • the checker, its denylist and its baseline;
  • the CI step;
  • CONTRIBUTING and GENERALITY_AUDIT;
  • public-artifacts.json;
  • comment-only edits in seven engine files.

No tests are deleted or weakened. reward/ is identical to main.

What I checked, all on the merged tree:

  • python scripts/check_engine_generality.py exits 0 with "358 baseline hits left".
  • tests/test_engine_generality_check.py, tests/test_behavioral_generalization.py, tests/test_rl_env_typed.py and tests/test_reward*.py: 98 passed, 1 skipped.
  • I dropped a new file into openadapt_flow/ to see what the checker catches. Each of these fails it (exit 1):
    • a plain "http://openemr.local";
    • the camel-case isOpenEMR;
    • an llx_ table name;
    • the /apis/default/fhir path;
    • x = 1 # example: OpenEMR, because a trailing marker doesn't exempt the line.
  • A comment line that opens with # example: OpenEMR passes, as designed (scripts/check_engine_generality.py:46-47).
  • The CI step runs in packages/flow on Python 3.12 (.github/workflows/packages.yml, flow-lint job, working-directory: packages/flow and python-version: "3.12"). The missing-tomli fallback at check_engine_generality.py:33-35 would raise, so it fails closed.
  • No dangling imports of the moved modules.

Non-blocking gaps (follow-ups, not blockers):

  1. Within one file and one term, the ratchet compares counts only (check_engine_generality.py:124). I removed one baselined name from backends/playwright_backend.py and added a new NEW_HOST = "openemr.example" line to the same file, and the check still exited 0. Storing hashes of the baselined lines would close this.
  2. Only these suffixes are scanned (check_engine_generality.py:44): .py, .js, .html, .css, .json, .yaml, .yml, .toml, .txt. A .md file under openadapt_flow/ naming an app passes (probe exit 0), and so would .sh, .bat or .sql. Scanning every text file, or failing on unknown suffixes, would fail closed.
  3. Files that fail UTF-8 decoding are skipped without a message (check_engine_generality.py:94). Printing the path, or failing, would make a skip visible.
  4. File and directory names aren't checked, only file contents, so openadapt_flow/openemr_adapter.py with a clean body would pass.

All probe files were removed afterwards. The worktree is back at cdf9c99 and clean.

The coordinator's two questions:

  • Does the 426→358 baseline drop remove entries, or hide them? It removes them. The checker fails on stale entries as well as new hits (scripts/check_engine_generality.py:118-139 and 174-181), so the baseline has to equal the hits actually in the code. It passes with exactly 358. The dropped entries are:

    • the modules that left the engine: harness2/*, guard_demo.py, agent/ledger_demo.py and learning/synth_stream.py;
    • three MockLoan lines in demo_driver.py, where the recorder moved to benchmark/harness/mockloan/demo.py;
    • one textedit hit in validation/dense_surface.py.
  • Did the three transferred identifiers move with unchanged code? Yes. reward/seed.py went from 39 mockmed hits to 34, and reward/bundle.py is new with 3. The 3 in bundle.py:48-49 and :210 are:

    • SYNTHETIC_TASK_ID = "task_mockmed_encounter_note"
    • SYNTHETIC_ENVIRONMENT_ID = "environment_mockmed_synthetic"
    • certificate_id = "reward_certificate_mockmed"

    These are byte-identical to the old seed.py values. seed.py:44-45 keeps MOCKMED_TASK_ID and MOCKMED_ENVIRONMENT_ID as aliases. With the old names swapped for the new ones, the contract-building code matches the old seed.py (task and environment digests, effects, oracle, certificate policy). reward/ on the head is identical to main.

@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr
abrichr enabled auto-merge (squash) November 4, 2026 07:59
@abrichr
abrichr disabled auto-merge November 4, 2026 07:59
Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Current main is now a parent at f7a173b, via a plain merge without force-pushing. The diff against main is exactly 15 guard/prose files (+711/-14); already-merged benchmark/results files are gone. The reviewed checker, denylist and 358-hit baseline are unchanged. After integration: 62 guard/reward/environment tests pass, Flow Ruff/format pass, and mypy passes (588 files). CI is running.

@abrichr
abrichr enabled auto-merge (squash) November 4, 2026 08:06
@abrichr
abrichr merged commit 42a82da into main Nov 4, 2026
53 checks passed
@abrichr
abrichr deleted the claude/engine-generality-guard branch November 4, 2026 08:28
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.

1 participant