chore: remove completed OSS launch phase from roadmap#10
Closed
chore: remove completed OSS launch phase from roadmap#10
Conversation
abbudjoe
added a commit
that referenced
this pull request
Apr 8, 2026
**Critical Fixes (Issues #1-#4):** - Replace .expect() with .unwrap_or_default() in current_timestamp() - Replace all .unwrap() on mutex locks with .unwrap_or_else(|e| e.into_inner()) - Add comprehensive doc comments to MAX_FAILURES and COOLDOWN_SECONDS - Use saturating_add() to prevent integer overflow in record_failure() **Quality Improvements (Issues #5-#10):** - Add doc comment explaining manual Default impl - Extract is_approved() logic into testable function with 2 new tests - Make error messages strategy-specific (LocalFirst/CloudFirst strategy failed) - Add comprehensive examples to resolve_strategy() documentation - Document why auto-recovery cooldown is not unit tested - Verify crossterm dependency has no conflicts **Test Count:** - Total: 60 tests (50 nv-llm + 10 nv-cli) - Added 2 tests for approval logic (is_approved_yes_variants, is_approved_no_variants) All quality checks pass: - cargo fmt --all --check ✓ - cargo clippy --workspace --all-targets -- -D warnings ✓ - cargo test --workspace ✓ - No unwrap() in library code ✓
abbudjoe
added a commit
that referenced
this pull request
Apr 8, 2026
Critical fixes: - Replace unwrap() with expect() in create_user_input() helper (issue #1) - Add comprehensive documentation that WASM tests verify infrastructure only, not execution (issue #2) - Add TODO comments referencing PR #179 for real WASM runtime (issue #2) - Add timeout and multi_thread flavor to concurrent audit test to prevent deadlocks (issue #3) - Fix tautological assertions in edge case tests - now verify specific behavior (issue #4) High priority fixes: - Extend MockLlmProvider with 4 error types: ServiceUnavailable, RateLimitExceeded, Timeout, MalformedResponse (issue #5) - Add comprehensive doc comments to MockLlmProvider explaining matching strategy and thread-safety (issue #12) - Strengthen prompt injection test to require IntentCategory::Conversation (issue #6) - Add test_audit_hash_chain_tampering_detection test (issue #7) - Add test_skill_network_capability_denied test for runtime capability enforcement (issue #8) Medium priority fixes: - Extract all magic number encryption keys to named constants (issue #9) - Add comment in Cargo.toml explaining intentional E2E test dependencies (issue #10) - Make retry backoff timing test more robust with 80ms threshold instead of 100ms (issue #11) Low priority improvements: - Rename test_policy_allow_with_confirmation → test_policy_requires_confirmation_for_destructive_actions (issue #13) - Add task IDs to concurrent audit test events for better debugging (issue #14) - Remove unused test_storage_round_trip helper (was addressing issue #15 but simplified instead) All tests pass (28/28), clippy clean with -D warnings, formatted with rustfmt.
abbudjoe
added a commit
that referenced
this pull request
Apr 8, 2026
- Add TDD workflow requirement to testing strategy (#1) - Expand error mapping: 429, 402, model deprecation (#2) - Add Keystore key rotation/invalidation handling (#3) - Document health check endpoint limitation re model.request scope (#4) - Specify atomic migration with rollback steps (#5) - Scope out biometric gate for this phase (#6) - Clarify Kotlin-only architecture boundary for vault (#7) - Add migration idempotency test case (#8) - Propose offline health check behavior (#9) - Note doc path verification (#10) - Specify GCM tag length (128-bit) (#11)
abbudjoe
added a commit
that referenced
this pull request
Apr 8, 2026
Address all 13 items from the review comment: Blocking: - #1: Replace serde_json::Value metadata with typed ThoughtMetadata enum - #2: Edge classification uses explicit caller declaration (add_edge vs add_back_edge) with index-order sanity checks, not index arithmetic - #3: Define Generate partial failure as all-or-nothing per parent Non-blocking: - #4: ThoughtIdAllocator uses plain u64, not AtomicU64 - #5: Remove created_at ghost field (Instant not serializable) - #6: Replace raw usize with GraphNodeId wrapper throughout - #7: Fix line count estimate to ~1,900 (was ~1,100) - #8: Replace (usize, usize, bool) tuple with named EdgeSpec struct - #9: GoT + sub_goals combination returns error instead of silent ignore Nice-to-have: - #10: LLM score parsing uses regex extraction with fallback - #11: Each operation emits tracing::info_span! with node/op/cycle - #12: Single budget mechanism (session-level), removed max_total_tokens - #13: Document refine() sets last_node to final internal node Step 6 gains 4 new test cases (12-15) covering partial failure, parameter conflict rejection, refine wiring, and score parsing.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Phase complete. All items shipped.