refactor: standardize JsonView render API with Render<T> trait and ViewRenderError

.github/skills/dev/rust-code-quality/handle-errors-in-code/skill.md

@@ -85,13 +85,46 @@ impl ProvisionError {
 }
 ```
 
+## Unwrap and Expect Policy
+
+| Context                | `.unwrap()`   | `.expect("msg")`                             | `?` / `Result` |
+| ---------------------- | ------------- | -------------------------------------------- | -------------- |
+| Production code        | ❌ Never      | ✅ Only when failure is logically impossible | ✅ Default     |
+| Tests and doc examples | ✅ Acceptable | ✅ Preferred when message adds clarity       | —              |
+
+```rust
+// ✅ Production: propagate errors with ?
+fn load_config(path: &Path) -> Result<Config, ConfigError> {
+    let content = std::fs::read_to_string(path)
+        .map_err(|e| ConfigError::FileAccess { path: path.to_path_buf(), source: e })?;
+    serde_json::from_str(&content)
+        .map_err(|e| ConfigError::InvalidJson { path: path.to_path_buf(), source: e })
+}
+
+// ✅ Production: expect() only when failure is a code invariant violation
+let pair = "key=value";
+let (k, v) = pair.split_once('=')
+    .expect("split on '=' always succeeds: the string literal contains '='");
+
+// ❌ Production: never unwrap()
+let value = some_result.unwrap();
+
+// ✅ Tests and doc examples: unwrap() is fine
+#[test]
+fn it_should_parse_valid_config() {
+    let config: Config = serde_json::from_str(VALID_JSON).unwrap();
+    assert_eq!(config.name, "test");
+}
+```
+
 ## Quick Checklist
 
 - [ ] Error type uses `thiserror::Error` derive
 - [ ] Error message includes specific context (names, paths, values)
 - [ ] Error message includes fix instructions where possible
 - [ ] Prefer `enum` over `Box<dyn Error>` or `anyhow`
 - [ ] No vague messages like "invalid input" or "error occurred"
+- [ ] No `.unwrap()` in production code (tests and doc examples are fine)
 
 ## Reference
 

docs/contributing/error-handling.md

@@ -434,64 +434,64 @@ Errors in this project are organized following Domain-Driven Design layers. Each
 
 ### Unwrap and Expect Usage
 
-The use of `unwrap()` is generally **discouraged** in this project. While it may make sense in certain contexts, we prefer alternatives that align with our principles of observability, traceability, and user-friendliness.
-
 #### General Rule
 
-**Prefer `expect()` over `unwrap()`** - Even in cases where panicking is acceptable, use `expect()` to provide meaningful context that aids debugging and aligns with our observability principles.
+| Context                | `.unwrap()`   | `.expect("msg")`                             | `?` / `Result` |
+| ---------------------- | ------------- | -------------------------------------------- | -------------- |
+| Production code        | ❌ Never      | ✅ Only when failure is logically impossible | ✅ Default     |
+| Tests and doc examples | ✅ Acceptable | ✅ Preferred when message adds clarity       | —              |
+
+In **production code**, always propagate errors with `?` or explicit `map_err`. Use `.expect()` only for operations where failure is truly logically impossible given the surrounding code's invariants. Never use `.unwrap()` — it provides no context when it panics.
+
+In **tests and doc examples**, `.unwrap()` is acceptable. Prefer `.expect("message")` when the message would meaningfully help diagnose a test failure, but it is not required.
 
 #### When Unwrap/Expect is Acceptable
 
-##### Tests
+##### Tests and Doc Examples
 
-In test code, panicking on unexpected failures is acceptable and even desired. However, **prefer `expect()` with descriptive messages** to make test failures easier to understand.
+In test code and doc examples, panicking on unexpected failures is acceptable and even desired. `.unwrap()` is fine. Prefer `.expect("message")` when the message would help diagnose a failure, but it is not required.
 
 ```rust
-// ✅ Good: Tests with expect() providing context
+// ✅ Acceptable: plain unwrap() in tests
 #[test]
 fn it_should_parse_valid_config() {
     let config_str = r#"{"name": "test", "port": 8080}"#;
-    let config: Config = serde_json::from_str(config_str)
-        .expect("Failed to parse valid test configuration - this indicates a parsing bug");
+    let config: Config = serde_json::from_str(config_str).unwrap();
 
     assert_eq!(config.name, "test");
 }
 
-// ✅ Also acceptable in tests with clear context
+// ✅ Also good: expect() adds useful context when a test fails
 #[test]
 fn it_should_create_temp_directory() {
     let temp_dir = TempDir::new()
         .expect("Failed to create temporary directory for test - check filesystem permissions");
     // ... rest of test
 }
-
-// ❌ Avoid: Unwrap without context
-#[test]
-fn it_should_parse_valid_config() {
-    let config_str = r#"{"name": "test", "port": 8080}"#;
-    let config: Config = serde_json::from_str(config_str).unwrap();  // What failed? Why?
-
-    assert_eq!(config.name, "test");
-}
 ```
 
-##### Infallible Operations
+##### Infallible Operations in Production
 
-Use `expect()` (not `unwrap()`) for operations that are logically infallible but return `Result` or `Option` due to API design.
+Use `.expect()` (never `.unwrap()`) for operations that are logically infallible — where the code's own invariants make failure impossible. The `.expect()` message must explain _why_ failure is impossible.
 
 ```rust
-// ✅ Good: expect() with clear reasoning
-let port: u16 = env::var("PORT")
-    .expect("PORT environment variable must be set during application initialization")
-    .parse()
-    .expect("PORT must be a valid u16 number - validated during configuration loading");
+// ✅ Correct: expect() because the literal always contains '='
+let pair = "key=value";
+let (k, v) = pair.split_once('=')
+    .expect("split on '=' always succeeds: the string literal contains '='");
 
-// ✅ Good: Mutex operations that should never fail
+// ✅ Correct: Mutex poisoning means a prior panic occurred — acceptable to surface that
 let data = self.state.lock()
     .expect("State mutex poisoned - indicates a panic occurred while holding the lock");
 
-// ❌ Avoid: unwrap() without explanation
+// ❌ Wrong: unwrap() in production code — never acceptable
 let port: u16 = env::var("PORT").unwrap().parse().unwrap();
+
+// ✅ Correct production alternative: propagate as a domain error
+let port: u16 = env::var("PORT")
+    .map_err(|_| ConfigError::MissingEnvVar { name: "PORT" })?
+    .parse()
+    .map_err(|_| ConfigError::InvalidPort { raw: env::var("PORT").unwrap_or_default() })?;
 ```
 
 #### When to Use Proper Error Handling Instead
@@ -550,10 +550,10 @@ let config = CONFIG.get().unwrap();
 
 #### Summary
 
-- **Default**: Use proper error handling with `Result` and specific error types
-- **Tests**: Use `expect()` with descriptive messages explaining what failed
-- **Infallible operations**: Use `expect()` with clear reasoning about why failure is impossible
-- **Never**: Use `unwrap()` without a very good reason (prefer `expect()` even in those cases)
+- **Default (production)**: Use proper error handling with `Result`, `?`, and specific error types
+- **Infallible operations (production)**: Use `.expect("reason")` — only when failure is logically impossible; the message must explain why
+- **Never (production)**: Use `.unwrap()` — it provides no context and masks errors
+- **Tests and doc examples**: `.unwrap()` is acceptable; prefer `.expect("message")` when the message adds diagnostic value
 
 This approach ensures that even panic messages provide valuable debugging context, maintaining our commitment to observability and traceability throughout the codebase.
 
@@ -886,7 +886,7 @@ When reviewing error handling code, verify:
 - [ ] **Thiserror Usage**: Are enum errors using `thiserror` with proper `#[error]` attributes?
 - [ ] **Source Preservation**: Are source errors preserved with `#[source]` for traceability?
 - [ ] **Pattern Matching**: Can callers handle different error cases appropriately?
-- [ ] **Unwrap/Expect**: Is `unwrap()` avoided in favor of `expect()` with descriptive messages?
+- [ ] **Unwrap/Expect**: Is `unwrap()` absent from production code? (Tests and doc examples may use `.unwrap()`; production code uses `?` or `.expect("reason")` for logically-impossible failures only)
 - [ ] **Consistency**: Does the error follow project conventions?
 - [ ] **Error Grouping**: Are related errors grouped with section comments?
 - [ ] **Box Wrapping**: Are large nested errors wrapped with `Box<T>` to avoid enum bloat?
@@ -915,7 +915,7 @@ Look for error handling examples in:
 2. **Use enums by default**: Only use `anyhow` when justified
 3. **Include context**: Always provide enough information for diagnosis
 4. **Make errors actionable**: Tell users how to fix the problem
-5. **Prefer `expect()` over `unwrap()`**: Provide meaningful context even when panicking
+5. **Never `unwrap()` in production**: Use `?` to propagate or `.expect("reason")` only for logically-impossible failures; `.unwrap()` is acceptable in tests
 6. **Test error paths**: Write tests for error scenarios
 7. **Document error types**: Document when and why specific errors occur
 

docs/refactors/plans/standardize-json-view-render-api.md

@@ -41,14 +41,14 @@ incompatible patterns in use, and no mechanism to prevent future drift.
 **Total Active Proposals**: 2
 **Total Postponed**: 0
 **Total Discarded**: 0
-**Completed**: 0
+**Completed**: 2
 **In Progress**: 0
-**Not Started**: 2
+**Not Started**: 0
 
 ### Phase Summary
 
-- **Phase 0 - Introduce ViewRenderError + Render Trait (High Impact, Low Effort)**: ⏳ 0/1 completed (0%)
-- **Phase 1 - Standardize Return Type (High Impact, Medium Effort)**: ⏳ 0/1 completed (0%)
+- **Phase 0 - Introduce ViewRenderError + Render Trait (High Impact, Low Effort)**: ✅ 1/1 completed (100%)
+- **Phase 1 - Standardize Return Type (High Impact, Medium Effort)**: ✅ 1/1 completed (100%)
 
 ### Discarded Proposals
 
@@ -107,13 +107,13 @@ be reviewed as one unit. Low effort since no existing call sites change yet.
 
 ### Proposal #1: Add `ViewRenderError` and `Render<T>` Trait
 
-**Status**: ⏳ Not Started
+**Status**: ✅ Completed
 **Impact**: 🟢🟢🟢 High
 **Effort**: 🔵 Low
 **Priority**: P0
 **Depends On**: None
-**Completed**: -
-**Commit**: -
+**Completed**: 2026-02-27
+**Commit**: `91798861`
 
 #### Problem
 
@@ -208,13 +208,13 @@ the trait when the error type is later updated. Defining `ViewRenderError` upfro
 
 #### Implementation Checklist
 
-- [ ] Create `src/presentation/cli/views/render.rs` with `ViewRenderError` and `Render<T>`
-- [ ] Add `thiserror` as a dependency if not already present (check `Cargo.toml`)
-- [ ] Re-export both from `src/presentation/cli/views/mod.rs`
-- [ ] Add `impl Render<...> for JsonView` to all 13 `json_view.rs` files
-- [ ] Add `impl Render<...> for TextView` to all 13 `text_view.rs` files
-- [ ] Verify all tests pass
-- [ ] Run `cargo run --bin linter all` and fix issues
+- [x] Create `src/presentation/cli/views/render.rs` with `ViewRenderError` and `Render<T>`
+- [x] Add `thiserror` as a dependency if not already present (check `Cargo.toml`)
+- [x] Re-export both from `src/presentation/cli/views/mod.rs`
+- [x] Add `impl Render<...> for JsonView` to all 13 `json_view.rs` files
+- [x] Add `impl Render<...> for TextView` to all 13 `text_view.rs` files
+- [x] Verify all tests pass
+- [x] Run `cargo run --bin linter all` and fix issues
 
 #### Testing Strategy
 
@@ -231,13 +231,13 @@ pattern.
 
 ### Proposal #2: Remove `unwrap_or_else` Fallbacks and Return `Result`
 
-**Status**: ⏳ Not Started
+**Status**: ✅ Completed
 **Impact**: 🟢🟢🟢 High
 **Effort**: 🔵🔵 Medium
 **Priority**: P1
 **Depends On**: Proposal #1 + [`standardize-command-view-folder-structure`](standardize-command-view-folder-structure.md) (must be applied first so file paths are stable)
-**Completed**: -
-**Commit**: -
+**Completed**: 2026-02-28
+**Commit**: `80616e44`
 
 #### Problem
 
@@ -300,12 +300,12 @@ an error.
 
 #### Implementation Checklist
 
-- [ ] Update all 11 `json_view.rs` standalone `render()` methods to the `Render<T>` trait impl
-- [ ] Update call sites in all 11 command handlers to use `?`
-- [ ] Add `From<ViewRenderError>` conversions where missing in handler error types
-- [ ] Also update `create` and `provision` (already return `Result`) to use the trait
-- [ ] Verify all tests pass
-- [ ] Run `cargo run --bin linter all` and fix issues
+- [x] Update all 11 `json_view.rs` standalone `render()` methods to the `Render<T>` trait impl
+- [x] Update call sites in all 11 command handlers to use `?`
+- [x] Add `From<ViewRenderError>` conversions where missing in handler error types
+- [x] Also update `create` and `provision` (already return `Result`) to use the trait
+- [x] Verify all tests pass
+- [x] Run `cargo run --bin linter all` and fix issues
 
 #### Testing Strategy
 
@@ -318,23 +318,23 @@ behavior (the rendered JSON string) does not change.
 ## 📈 Timeline
 
 - **Start Date**: 2026-02-27
-- **Actual Completion**: TBD
+- **Actual Completion**: 2026-02-28
 
 ## 🔍 Review Process
 
 ### Approval Criteria
 
-- [ ] Technical feasibility validated
-- [ ] Aligns with [Development Principles](../development-principles.md)
-- [ ] Implementation plan is clear and actionable
-- [ ] Priorities are correct (high-impact/low-effort first)
+- [x] Technical feasibility validated
+- [x] Aligns with [Development Principles](../development-principles.md)
+- [x] Implementation plan is clear and actionable
+- [x] Priorities are correct (high-impact/low-effort first)
 
 ### Completion Criteria
 
-- [ ] All active proposals implemented
-- [ ] All tests passing
-- [ ] All linters passing
-- [ ] Documentation updated
+- [x] All active proposals implemented
+- [x] All tests passing
+- [x] All linters passing
+- [x] Documentation updated
 - [ ] Changes merged to main branch
 
 ## 📚 Related Documentation
@@ -353,8 +353,17 @@ to be implemented (earlier PRs #351, #353) and happened to use `Result` from the
 Subsequent commands followed a slightly different pattern that evolved independently.
 The Copilot review on PR #397 was the first mention of the inconsistency.
 
+A follow-up cleanup (`1c71cd7e`) also removed the inherent `pub fn render() -> String`
+methods from all 13 `text_view.rs` files. These had been left intact after Proposals #1
+and #2 because they weren't part of the original scope. Post-completion review revealed
+that keeping them created an asymmetry: `JsonView` exposed rendering exclusively through
+the `Render<T>` trait while `TextView` still had an inherent "convenience" method that
+delegated to the trait. Applying the "least surprise" principle, the inherent methods were
+removed and their bodies inlined directly into the `Render<T>` impls, making all 26 view
+structs consistent.
+
 ---
 
 **Created**: 2026-02-27
-**Last Updated**: 2026-02-27 (revised: merged ViewRenderError into Proposal #1, removed deferred Phase 2)
-**Status**: 📋 Planning
+**Last Updated**: 2026-02-28 (follow-up: removed inherent render() from text_views, commit 1c71cd7e)
+**Status**: ✅ Completed

src/presentation/cli/controllers/configure/errors.rs

@@ -9,6 +9,7 @@ use thiserror::Error;
 use crate::application::command_handlers::configure::errors::ConfigureCommandHandlerError;
 use crate::domain::environment::name::EnvironmentNameError;
 use crate::presentation::cli::views::progress::ProgressReporterError;
+use crate::presentation::cli::views::ViewRenderError;
 
 /// Configure command specific errors
 ///
@@ -89,6 +90,12 @@ Tip: This is a critical bug - please report it with full logs using --log-output
         #[source]
         source: ProgressReporterError,
     },
+    /// Output formatting failed (JSON serialization error).
+    /// This indicates an internal error in data serialization.
+    #[error(
+        "Failed to format output: {reason}\nTip: This is a critical bug - please report it with full logs using --log-output file-and-stderr"
+    )]
+    OutputFormatting { reason: String },
 }
 
 // ============================================================================
@@ -100,6 +107,13 @@ impl From<ProgressReporterError> for ConfigureSubcommandError {
         Self::ProgressReportingFailed { source }
     }
 }
+impl From<ViewRenderError> for ConfigureSubcommandError {
+    fn from(e: ViewRenderError) -> Self {
+        Self::OutputFormatting {
+            reason: e.to_string(),
+        }
+    }
+}
 
 impl ConfigureSubcommandError {
     /// Get detailed troubleshooting guidance for this error
@@ -330,6 +344,9 @@ This is a critical internal error that should not occur during normal operation.
 This error indicates a bug in the progress reporting system.
 Please report it so we can fix it."
             }
+            Self::OutputFormatting { .. } => {
+                "Output Formatting Failed - Critical Internal Error:\n\nThis error should not occur during normal operation. It indicates a bug in the output formatting system.\n\n1. Immediate actions:\n   - Save full error output\n   - Copy log files from data/logs/\n   - Note the exact command and output format being used\n\n2. Report the issue:\n   - Create GitHub issue with full details\n   - Include: command, output format (--output-format), error output, logs\n   - Describe steps to reproduce\n\n3. Temporary workarounds:\n   - Try using different output format (text vs json)\n   - Try running command again\n\nPlease report it so we can fix it."
+            }
         }
     }
 }

src/presentation/cli/controllers/configure/handler.rs

@@ -19,6 +19,7 @@ use crate::presentation::cli::views::commands::configure::{
 };
 use crate::presentation::cli::views::progress::ProgressReporter;
 use crate::presentation::cli::views::progress::VerboseProgressListener;
+use crate::presentation::cli::views::Render;
 use crate::presentation::cli::views::UserOutput;
 use crate::shared::clock::Clock;
 
@@ -251,9 +252,7 @@ impl ConfigureCommandController {
     ///
     /// # Note
     ///
-    /// JSON serialization errors are handled inline by `JsonView::render()`,
-    /// which returns a fallback error JSON string. Therefore, this method
-    /// does not propagate serialization errors.
+    /// JSON serialization errors are propagated as `ConfigureSubcommandError::OutputFormatting`.
     #[allow(clippy::result_large_err)]
     fn display_configure_results(
         &mut self,
@@ -263,8 +262,8 @@ impl ConfigureCommandController {
         self.progress.blank_line()?;
         let details = ConfigureDetailsData::from(configured);
         let output = match output_format {
-            OutputFormat::Text => TextView::render(&details),
-            OutputFormat::Json => JsonView::render(&details),
+            OutputFormat::Text => TextView::render(&details)?,
+            OutputFormat::Json => JsonView::render(&details)?,
         };
         self.progress.result(&output)?;
         Ok(())