Collapse review_action to what it now is: a per-project viewer name

CHANGELOG.md

@@ -106,6 +106,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   but `voro show`'s event log — and no PR or configured viewer to fall back on,
   on the fresh install where that matters most. A rework's block is unchanged
   but for its heading, and shows nothing while the rework is still in flight.
+- The per-project review action is now what it does: a viewer name. Since the
+  review keys split — `pr` always GitHub, `open` always a local viewer — the
+  setting decided only which `[viewers.<name>]` table a project's local diffs
+  open in, so `projects.review_action` becomes `projects.viewer` and holds that
+  name or nothing. Existing databases convert in place: `viewer:<name>` keeps
+  its name, and `auto`, `pr`, and a bare `viewer` — three spellings of "name no
+  viewer" — all become the default viewer. `voro project action <p>
+  <auto|pr|viewer[:NAME]>` is now `voro project viewer <p> [NAME]`, naming no
+  viewer to fall back to the default, and the projects screen's `v` picker
+  offers the default and each named viewer instead of two entries that did
+  nothing distinguishable.
 - The cockpit key line advertises `d/D dispatch` only on a `ready` or `stalled`
   row, where dispatch can actually act, rather than on any selection — which
   also makes room for the new `a/A message` slot within the line's ten.

README.md

@@ -142,7 +142,7 @@ lands in `review` where `voro open` or `voro pr` puts the diff in front of you.
 
 To extend or override the built-in agents and viewers, layer a
 `~/.config/voro/voro.toml` on top (`voro agent init` writes a skeleton). The
-dispatch semantics, the review action, and the `voro.toml` format are covered in
+dispatch semantics, the per-project viewer, and the `voro.toml` format are covered in
 [`docs/DESIGN.md`](docs/DESIGN.md) §8; the `CLAUDE.md`/`AGENTS.md` return-path
 snippet and the Claude Code hooks configuration are in
 [`docs/agent-integration.md`](docs/agent-integration.md).

crates/voro-core/migrations/0018_project_viewer.sql

@@ -0,0 +1,15 @@
+-- 0018: collapse the review action to what it names — a viewer.
+--
+-- The review keys are static (§8): `pr` is always GitHub, `open` always a local
+-- viewer. All the setting still decides is which [viewers.<name>] table a
+-- project's local diffs open in, so the column is that name: 'viewer:<name>'
+-- keeps <name>, while 'auto', 'pr', and bare 'viewer' all meant "name no
+-- viewer, use the default", which is NULL.
+
+UPDATE projects SET review_action = NULL
+WHERE review_action IN ('auto', 'pr', 'viewer');
+
+UPDATE projects SET review_action = substr(review_action, length('viewer:') + 1)
+WHERE review_action LIKE 'viewer:%';
+
+ALTER TABLE projects RENAME COLUMN review_action TO viewer;

crates/voro-core/src/agent.rs

@@ -188,14 +188,13 @@ const STARTER_HEADER: &str = r#"# Voro configuration (~/.config/voro/voro.toml).
 #   * set `default_agent` — used for tasks with no --agent override. When unset,
 #     Voro picks the first built-in found on PATH (claude, then codex).
 #   * set up viewers — [viewers.<name>] tables define how a task's diff is
-#     shown locally when `voro pr`/`voro open` resolve to the viewer medium
-#     (DESIGN.md §8). A viewer cmd may carry `{path}` (the task's worktree, or
-#     the project checkout when it has none), `{branch}` (the task's branch, or
-#     empty), and `{base}` (the checkout's default branch); `{base}...{branch}`
-#     spells a diff range. `default_viewer` names the one used when a project
-#     does not pick a viewer itself (`voro project action <p> viewer:<name>`); a
-#     single anonymous [viewer] table is the older, still-valid spelling of
-#     the default.
+#     shown locally by `voro open` (DESIGN.md §8). A viewer cmd may carry
+#     `{path}` (the task's worktree, or the project checkout when it has none),
+#     `{branch}` (the task's branch, or empty), and `{base}` (the checkout's
+#     default branch); `{base}...{branch}` spells a diff range.
+#     `default_viewer` names the one used when a project does not pick a viewer
+#     itself (`voro project viewer <p> <name>`); a single anonymous [viewer]
+#     table is the older, still-valid spelling of the default.
 #   * price the queue — `max_running` caps how many dispatches ride at once
 #     (default 5; at the cap the queue offers no more), and a [costs] table
 #     divides each row's score by what its action asks of you, so a cheap
@@ -783,8 +782,8 @@ pub struct AgentsConfig {
     /// The anonymous `[viewer]` table — the pre-names single viewer, still
     /// honoured as a default when no `default_viewer` is set.
     viewer: Option<ViewerTemplate>,
-    /// The named `[viewers.<name>]` tables a project's review action can
-    /// pick from (DESIGN.md §8/§11a).
+    /// The named `[viewers.<name>]` tables a project can pick from
+    /// (DESIGN.md §8/§11a).
     viewers: BTreeMap<String, ViewerTemplate>,
     /// The user-set `default_viewer`, naming a `[viewers.*]` entry.
     default_viewer: Option<String>,
@@ -991,8 +990,8 @@ impl AgentsConfig {
         })
     }
 
-    /// The names of the `[viewers.*]` tables, sorted, for the TUI's
-    /// review-action picker and `viewer list`.
+    /// The names of the `[viewers.*]` tables, sorted, for the TUI's viewer
+    /// picker and `viewer list`.
     pub fn viewer_names(&self) -> Vec<String> {
         self.viewers.keys().cloned().collect()
     }

crates/voro-core/src/config_edit.rs

@@ -15,7 +15,7 @@ use toml_edit::{DocumentMut, Item, Table, value};
 
 use crate::agent::VIEWER_PATH_PLACEHOLDER;
 use crate::error::{Error, Result};
-use crate::model::{Project, ReviewAction};
+use crate::model::Project;
 
 /// Add a `[viewers.<name>]` table with the given command, refusing an empty
 /// name/command or a name that collides with an existing viewer. Existing
@@ -103,24 +103,22 @@ pub fn missing_path_placeholder(cmd: &str) -> bool {
     !cmd.contains(VIEWER_PATH_PLACEHOLDER)
 }
 
-/// The projects whose review action names this viewer explicitly
-/// (`viewer:<name>`), so deleting it can be refused with them named (DESIGN.md
-/// §5). A project on `viewer` (the unnamed default) is not counted — it follows
+/// The projects that name this viewer, so deleting it can be refused with them
+/// named (DESIGN.md §5). A project naming no viewer is not counted — it follows
 /// whatever the default resolves to rather than pinning this name.
 pub fn projects_referencing_viewer<'a>(projects: &'a [Project], name: &str) -> Vec<&'a Project> {
     projects
         .iter()
-        .filter(|p| matches!(&p.review_action, ReviewAction::Viewer(Some(n)) if n == name))
+        .filter(|p| p.viewer.as_deref() == Some(name))
         .collect()
 }
 
 fn validate_viewer(name: &str, cmd: &str) -> Result<()> {
     if name.is_empty() {
         return Err(invalid("viewer name is required".into()));
     }
-    // A name with whitespace or a colon cannot be referenced as `viewer:<name>`
-    // by a project's review action, so refuse it rather than create an
-    // unreachable viewer.
+    // A name is typed as one word — on the command line and as the bare TOML
+    // key of its `[viewers.<name>]` table — so refuse one that cannot be.
     if name.chars().any(|c| c.is_whitespace() || c == ':') {
         return Err(invalid(format!(
             "viewer name '{name}' cannot contain spaces or ':'"

crates/voro-core/src/lib.rs

@@ -27,7 +27,7 @@ pub use error::{Error, Result};
 pub use import::{GithubIssue, already_imported, issue_new_task, issue_task_body};
 pub use model::{
     Dep, DepKind, DepRef, Doc, Event, NextAction, Priority, Project, RefineOutcome, Repo,
-    ReviewAction, RunningRow, Session, SessionOutcome, Task, TaskState, location_is_url,
+    RunningRow, Session, SessionOutcome, Task, TaskState, location_is_url,
 };
 pub use pr::{Mergeability, PrPlan, PrRef, format_review_feedback, parse_mergeable, plan_pr};
 pub use review::{

crates/voro-core/src/model.rs

@@ -308,92 +308,17 @@ impl fmt::Display for RefineOutcome {
     }
 }
 
-/// Which viewer a project's local diffs open in (DESIGN.md §8/§11a). The two
-/// review keys are static — `g`/`pr` are always the GitHub PR flow, `o`/`open`
-/// always a local viewer — so this no longer chooses between media; it names
-/// the `voro.toml` viewer `o`/`open` resolve for this project. `Auto` and `Pr`
-/// survive as stored values that name no viewer, leaving the default one.
-/// Stored on the project (`projects.review_action`).
-#[derive(Debug, Clone, PartialEq, Eq, Default)]
-pub enum ReviewAction {
-    /// No viewer named — the default viewer. Stored as NULL, the unconfigured
-    /// default.
-    #[default]
-    Auto,
-    /// No viewer named either, kept so a project pinned to the GitHub flow
-    /// before the keys split still reads and writes.
-    Pr,
-    /// A local viewer from `voro.toml`: the named `[viewers.<name>]` when one
-    /// is given, otherwise the default viewer.
-    Viewer(Option<String>),
-}
-
-impl ReviewAction {
-    /// Parse the stored/CLI form: `auto`, `pr`, `viewer`, or `viewer:<name>`.
-    pub fn parse(s: &str) -> Result<ReviewAction> {
-        match s {
-            "auto" => Ok(ReviewAction::Auto),
-            "pr" => Ok(ReviewAction::Pr),
-            "viewer" => Ok(ReviewAction::Viewer(None)),
-            other => match other.strip_prefix("viewer:") {
-                Some(name) if !name.trim().is_empty() => {
-                    Ok(ReviewAction::Viewer(Some(name.trim().to_string())))
-                }
-                _ => Err(Error::Invalid(format!(
-                    "unknown review action '{s}' — expected auto, pr, viewer, or viewer:<name>"
-                ))),
-            },
-        }
-    }
-
-    /// The `voro.toml` viewer this project's local diffs open in, or `None` for
-    /// the default viewer.
-    pub fn viewer(&self) -> Option<&str> {
-        match self {
-            ReviewAction::Viewer(Some(name)) => Some(name),
-            _ => None,
-        }
-    }
-}
-
-impl fmt::Display for ReviewAction {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            ReviewAction::Auto => f.pad("auto"),
-            ReviewAction::Pr => f.pad("pr"),
-            ReviewAction::Viewer(None) => f.pad("viewer"),
-            ReviewAction::Viewer(Some(name)) => f.pad(&format!("viewer:{name}")),
-        }
-    }
-}
-
-impl FromSql for ReviewAction {
-    fn column_result(value: ValueRef<'_>) -> FromSqlResult<Self> {
-        match value {
-            ValueRef::Null => Ok(ReviewAction::Auto),
-            _ => ReviewAction::parse(value.as_str()?).map_err(|e| FromSqlError::Other(Box::new(e))),
-        }
-    }
-}
-
-impl ToSql for ReviewAction {
-    /// `Auto` writes NULL — absence of configuration — so the column stays
-    /// empty until the operator pins a medium.
-    fn to_sql(&self) -> rusqlite::Result<ToSqlOutput<'_>> {
-        match self {
-            ReviewAction::Auto => Ok(rusqlite::types::Null.into()),
-            other => Ok(other.to_string().into()),
-        }
-    }
-}
-
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Project {
     pub id: i64,
     pub name: String,
     pub weight: i64,
-    /// How `pr` shows this project's review diffs (DESIGN.md §8/§11a).
-    pub review_action: ReviewAction,
+    /// The `voro.toml` viewer this project's local diffs open in (DESIGN.md
+    /// §8/§11a): a `[viewers.<name>]` name, or `None` for the default viewer.
+    /// The review keys are static — `g`/`pr` are always the GitHub PR flow,
+    /// `o`/`open` always a local viewer — so this picks no medium, only the
+    /// viewer `o`/`open` resolve for this project.
+    pub viewer: Option<String>,
     /// Retired (DESIGN.md §5): the project and all its tasks leave the cockpit
     /// — queue, stats, running strip — until unarchived. Tasks freeze in
     /// whatever state they hold; only the projects screen still shows the
@@ -786,34 +711,4 @@ mod tests {
         assert_eq!(format!("{:10}", NextAction::ReviewPr), "review PR ");
         assert_eq!(format!("{:10}", NextAction::Redispatch), "redispatch");
     }
-
-    #[test]
-    fn review_action_parses_and_displays_every_form() {
-        for (text, action) in [
-            ("auto", ReviewAction::Auto),
-            ("pr", ReviewAction::Pr),
-            ("viewer", ReviewAction::Viewer(None)),
-            ("viewer:zed", ReviewAction::Viewer(Some("zed".into()))),
-        ] {
-            assert_eq!(ReviewAction::parse(text).unwrap(), action, "{text}");
-            assert_eq!(action.to_string(), text);
-        }
-        assert!(ReviewAction::parse("github").is_err());
-        assert!(ReviewAction::parse("viewer:").is_err());
-        assert!(ReviewAction::parse("viewer:  ").is_err());
-    }
-
-    /// The narrowed role (DESIGN.md §8): the action names the viewer `o`/`open`
-    /// resolve, and only the `viewer:<name>` form names one — the two legacy
-    /// forms leave the default viewer rather than choosing a medium.
-    #[test]
-    fn review_action_names_the_projects_viewer() {
-        assert_eq!(
-            ReviewAction::Viewer(Some("zed".into())).viewer(),
-            Some("zed")
-        );
-        assert_eq!(ReviewAction::Viewer(None).viewer(), None);
-        assert_eq!(ReviewAction::Auto.viewer(), None);
-        assert_eq!(ReviewAction::Pr.viewer(), None);
-    }
 }