[docs] - Align README with layout, install path, and CI test story - #36
Conversation
- Replace Windows-only install/dev blocks with cross-platform python -m pip because Linux/macOS and Ubuntu CI are first-class, not cmd.exe-only. - Document required CI (lint, mypy, unit 3.12/3.13, fake-BERT smoke) vs optional integration gated on workflow_dispatch / push [run-integration]; the suite patches TYPE_CHECKING-only AutoTokenizer and is not expected to pass on main. - Correct stemmer copy (suffix-expansion STEM / substring FUZZY, not Porter/lemmatization) and drop seed_keywords=[] which loads THREAD_SEEDS. - Sync SPEC tree and test story; make SECURITY_AUDIT remediations portable. Co-authored-by: Chris Grady <cgfixit@users.noreply.github.com>
- Finding 4 still claimed Porter/lemmatization and a missing LICENSE after those README items were corrected; add a dated note so the review snapshot is not read as current user-facing copy. Co-authored-by: Chris Grady <cgfixit@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78860f14b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,5 +1,5 @@ | |||
| # Pinned versions — known-good, reproducible build | |||
| # Tested: Python 3.12 / Windows (R2D2sys) / CPU-only / June 2026 | |||
| # Tested: Python 3.12+ / Linux CI + Windows / CPU-only | |||
There was a problem hiding this comment.
Validate the pinned set before calling it CI-tested
The new header says this exact constraints set is tested by Linux CI on Python 3.12+, but the dependency-bearing jobs in .github/workflows/ci.yml install with pip install -e . or pip install -e ".[dev]" without -c constraints.txt; they therefore test resolver-selected versions, not these pins, and the matrix covers only 3.12 and 3.13 rather than all 3.12+ releases. Users choosing the recommended pinned installation may consequently rely on an untested combination. Either add a constraints-based CI job or narrow this claim to the environments where the pinned set was actually validated.
AGENTS.md reference: .codex/AGENTS.md:L32-L36
Useful? React with 👍 / 👎.
Documentation-only sync. Aligns README and project docs with the live package, install contract, CLI, known
accelerate/transformersissues, and the real unit-vs-integration CI story. No runtime or test-code changes (including the integrationAutoTokenizermock).Why
User-facing docs still described a Windows/
cmd.exe-only install, an incomplete test tree, Porter/lemmatization stemming, andpytestas if the integration suite were a working required gate. Source and CI say otherwise.Changes
python -m venv,python -m pip install -r requirements.txt -c constraints.txt, thenpython -m pip install -e .. Windows activate/run.batkept as extras, not the only path.python -m insight_extractor [file.txt]and theinsight-extractconsole script; outputs write to CWD.accelerateerrors; live pins already includeaccelerate>=1.3.0/constraints.txt1.14.0. ResidualTorchTensorParallelPluginerrors come from a leftover olderaccelerate, not the current pin set.pytest tests/unit/on 3.12+3.13, and a fake-BERT CLI smoke job. Optionaltests/integration/runs only onworkflow_dispatchor a push head-commit containing[run-integration]; a PR message alone does not enable it. Those files patchinsight_extractor.tokenizer.AutoTokenizer(TYPE_CHECKING-only import) and are not expected to pass onmain.test_extractor.pyandtest_tokenizer.py; LICENSE present; STEM/FUZZY matchstemmer.py;seed_keywords=[]example removed (empty list loadsTHREAD_SEEDS);MatchInfo.scoreexample corrected tostemmed.pyproject.tomlsketch labeled as such; remediations usepython -m pip.constraints.txtcomment only — Notes Linux CI + Windows; pins unchanged.docs/CODEX_SETUP_REVIEW.md— Dated addendum so finding 4 is not read as current README state.Evidence (verified against source, not the old README)
>=3.12; unit matrix 3.12 and 3.13pyproject.tomlrequires-python;ci.ymlpython-version: ["3.12", "3.13"]requirements.txt+-c constraints.txt+pip install -e .requirements.txt,constraints.txtheader,pyproject.tomlpython -m insight_extractor; scriptinsight-extract__main__.py;[project.scripts]THREAD_SEEDS= 363;REGEX_PATTERNS= 15constants.py(counted)\b\w*{kw}\w*\bstemmer.pygenerate_patternNoneseed_keywordsloadsTHREAD_SEEDSextractor.py__init__ci.ymlci-passneeds:if:isworkflow_dispatchor pushhead_commitcontains[run-integration]ci.ymllines 286–288AutoTokenizeris TYPE_CHECKING at module scope; runtime import is inside the propertytokenizer.pyinsight_extractor.tokenizer.AutoTokenizer.from_pretrainedtests/integration/test_extractor.py,test_e2e.pytest_e2estill passesenable_dynamic/enable_semantic/enable_regextests/integration/test_e2e.py(unsupported constructor flags)accelerate==1.14.0,transformers==4.53.0constraints.txtLICENSE(MIT)Skipped
CLAUDE.md/.codex// skill files — agent manuals, not user-facing project docs..codex/AGENTS.mdalready states the live accelerate pin and stale integration mocks.