docs: update workspace prompt files to reflect current state - #113
Conversation
- EVOLUTION.md: mark architecture doc and sequence diagram items done, update maturity caveat to reflect closed co-evolution loop, mark config validation and checkpoint save/restore tests done - TODO.md: mark 'Adopt workspace prompt file conventions' done, update P3 (15→16) and total (61→62) tracking counts
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughModel and tokenizer loading now disable remote code execution. Tests cover successful initialization, unavailable Transformers support, and models that require remote code. Documentation updates record the security fix, Phase 3 status, completion dates, and tracking counts. ChangesSecure model loading
Project tracking
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant initialize_model
participant AutoTokenizer
participant AutoModel
participant ErrorLogger
initialize_model->>AutoTokenizer: from_pretrained(trust_remote_code=False)
AutoTokenizer-->>initialize_model: tokenizer or loading error
initialize_model->>AutoModel: from_pretrained(trust_remote_code=False)
AutoModel-->>initialize_model: model
initialize_model->>ErrorLogger: log remote-code loading error
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@TODO.md`:
- Around line 310-311: Reconcile the P3 tracking row with the Ollama
retry/backoff item still marked incomplete: verify the underlying item and mark
it complete if the fix is present, or remove “Ollama provider retry/backoff fix”
from the completion note if it remains open.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| | 🟢 P3 | 24 | 16 | Makefile, pre-commit, Docker, ADRs, ADR refresh, CHANGELOG complete; +11 hygiene items from 2026-07-22 review; Ollama provider retry/backoff fix; local_models TTL cache; workspace prompt file conventions | | ||
| | **Total** | **89** | **62** | | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the P3 completion note with the still-open retry/backoff item.
Line [262] still marks the Ollama retry/backoff issue as incomplete, while this row claims the fix is complete. Either verify and mark the underlying item done, or remove the completion claim from the tracking row.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@TODO.md` around lines 310 - 311, Reconcile the P3 tracking row with the
Ollama retry/backoff item still marked incomplete: verify the underlying item
and mark it complete if the fix is present, or remove “Ollama provider
retry/backoff fix” from the completion note if it remains open.
… counts - EVOLUTION.md: fix config-validation test date from 2026-07-24 to 2026-07-30 (actual commit 85e92c2 was 2026-07-30, not 2026-07-24) - TODO.md: fix P1 done count from 14 to 13 (13 actual checked boxes) - TODO.md: fix P2 done count from 21 to 24 (24 actual checked boxes) - TODO.md: update total from 62 to 64 to match corrected per-priority counts
Review feedback verificationI verified both review points against the actual repo state before applying any changes. Point 1: TODO.md summary table countsNot a bug — the PR corrects pre-existing table errors. I counted the actual
Point 2: Maturity caveat claimsAll claims verified as factually accurate.
The reviewer is correct that these claims can't be verified from the diff alone, but they are factually true against the current codebase. The caveat accurately reflects the state of the co-evolution loop. |
Verification of review feedbackI verified each point against the actual Point 1: P1 "Done" count drops 14→13The reviewer is wrong. On git show origin/main:TODO.md | sed -n "/## 🟠 P1/,/## 🟡 P2/p" | grep -c "\- \[x\]"
# → 13The table was overcounting by 1 before this PR. No P1 checkbox was changed because none needed to be — the PR corrects the table from the wrong value (14) to the actual count (13). This is a pre-existing arithmetic error being fixed, not a silent revert. Point 2: EVOLUTION.md flips 4 checkboxes but TODO.md not syncedThe reviewer is wrong. All 4 items are already
The PR brings EVOLUTION.md into sync with TODO.md, not out of sync. The reviewer was working from the diff alone and could not see the full TODO.md body. Point 3: P2 Done 21→24 (+3) vs 4 newly-completed itemsThe reviewer is wrong. On git show origin/main:TODO.md | sed -n "/## 🟡 P2/,/## 🟢 P3/p" | grep -c "\- \[x\]"
# → 24The +3 delta is correcting the table from 21 (wrong) to 24 (actual count). The 4 EVOLUTION.md items being marked were already checked in TODO.md — they are not "newly completed" and do not affect the count. The notes field is unchanged between old and new; only the number is corrected. Point 4: PRs #73, #89 and items 1–6Verified and accurate:
The maturity caveat update correctly reflects the current state of merged work. SummaryAll four concerns stem from the reviewer not having access to the full TODO.md body and not realizing the tracking table was already inaccurate. The PR is fixing pre-existing table arithmetic errors, not introducing new ones. No changes are needed to address these points — the diff is correct as-is. |
* fix: default trust_remote_code to False in model_fine_tuner Changed both AutoTokenizer.from_pretrained and AutoModelForCausalLM.from_pretrained calls from trust_remote_code=True to trust_remote_code=False. The known HF base models (DeepSeek, Qwen, CodeLlama, Mistral) are standard architectures that do not require remote code execution. Removed the # nosec B615 suppression comments since the security flag is now safe by default. The _resolve_hf_base_model() fallback still uses model_name verbatim for unknown families, but without trust_remote_code=True it can no longer execute arbitrary code from an untrusted repo. Added 3 regression tests: source inspection for the safe default, and behavior check when TRANSFORMERS_AVAILABLE is False. Closes TODO.md item: model_fine_tuner.py:160-178 * fix: restore nosec B615 for revision pinning B615 flags missing revision= parameter in from_pretrained(), not trust_remote_code. The PR removed these suppressions along with the trust_remote_code=True fix, but revision pinning is intentionally left to the caller. Restore # nosec B615 on both loading calls and update the regression test accordingly. * fix: address review feedback on trust_remote_code PR - Fix TODO.md changelog: nosec B615 comments were relocated, not removed; B615 flags missing revision=, not trust_remote_code - Move tokenizer nosec B615 back to opening from_pretrained( line so bandit matches it to the correct line number - Replace fragile inspect.getsource test with mock-based test that asserts trust_remote_code=False on both from_pretrained call kwargs - Fix test_initialize_returns_false_without_transformers to actually patch TRANSFORMERS_AVAILABLE=False via monkeypatch - Update PR body to correct inaccurate claim about removing nosec comments * test: add failure-path test for trust_remote_code=False incompatibility Addresses review feedback on PR #114: the existing tests only asserted the kwarg value on mocked from_pretrained calls but never exercised the failure path where a model that needs custom code raises on load. The new test mocks from_pretrained to raise OSError (what transformers actually raises when the auto-map can't resolve the architecture without custom code) and verifies that: - initialize_model() returns False - is_initialized stays False - Model load is not attempted (tokenizer fails first) - The error is logged at ERROR level for operator diagnosis
# Conflicts: # TODO.md
1 participant
What
Updates the workspace prompt files (
AGENT.md,EVOLUTION.md,SAFETY.md) toreflect the current state of the project and marks the "Adopt workspace prompt file
conventions" TODO item as done.
Changes
(done 2026-07-23), config validation test, and checkpoint save/restore test as
complete. Updated the maturity caveat to reflect that the co-evolution loop is now
closed (items 1–6 merged).
- [ ]to- [x]. Updated P3 tracking count (15→16) and total (61→62).Why
The workspace prompt files were created in a prior run but EVOLUTION.md still listed
completed items as open and had a stale maturity caveat from before the co-evolution
loop was closed. Keeping these files accurate makes the workspace self-documenting
and avoids misleading contributors about project state.
Verification
ruff format --check .— ✅ 400 files already formattedruff check evoseal/ tests/— ✅ All checks passedAddresses: TODO.md "Adopt workspace prompt file conventions" (P3)
Summary by CodeRabbit
Security
Bug Fixes
Documentation