Repository navigation
Work plan: S8 curation slice 2 — record state in the inspector #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
d34f00e
466565c
27fc35c
933a79c
fded16b
3c4d8d5
8f6e56c
09e3270
519657a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| # Process backlog | ||
|
|
||
| Repository-process follow-ups that belong to no roadmap stage. Keep entries | ||
| short; delete them when done. | ||
|
|
||
| - [ ] **Codex review sweep script** (requested 2026-06-11): a script that | ||
| walks all PRs and visits those where Codex never left a review (e.g. the | ||
| usage-limit window around PR #40), re-requesting review with | ||
| `@codex review`. Note: the exact trigger phrase matters — free-form | ||
| variants may spawn a cloud task instead of a review. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,21 @@ use griff_core::corpus::{ChunkMeta, ReviewerDecision}; | |
|
|
||
| use crate::viewport::CurationDecision; | ||
|
|
||
| /// A UI-level digest of a chunk record's curation state. | ||
| /// | ||
| /// Plain strings in the schema's `snake_case` wire names, so renderers show | ||
| /// exactly what the corpus stores and no domain type crosses into the UI | ||
| /// (ADR-0016). | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct RecordSummary { | ||
| /// The record's `title`. | ||
| pub title: String, | ||
| /// The prior reviewer decision in wire casing, if any. | ||
| pub reviewer: Option<String>, | ||
| /// The record's tags in wire casing, in stored order. | ||
| pub tags: Vec<String>, | ||
| } | ||
|
|
||
| /// Errors the curation persistence seam can emit. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum CurationError { | ||
|
|
@@ -26,6 +41,36 @@ pub enum CurationError { | |
| /// | ||
| /// # Errors | ||
| /// [`CurationError::ParseFailed`] when `json` is not a `ChunkMeta` record. | ||
| /// Digests a serialized `ChunkMeta` record into the [`RecordSummary`] the | ||
| /// inspector shows: title, prior reviewer decision, and tags, all in the | ||
| /// schema's wire casing (via serde, so the names cannot drift). | ||
| /// | ||
| /// # Errors | ||
| /// [`CurationError::ParseFailed`] when `json` is not a `ChunkMeta` record. | ||
| pub fn summarize_record(json: &str) -> Result<RecordSummary, CurationError> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This commit adds new public API ( Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same finding as the thread above (#42 (comment)) — the rule is satisfied in the commit history, which the flattened PR diff can't show: red Generated by Claude Code |
||
| let meta: ChunkMeta = serde_json::from_str(json).map_err(|_| CurationError::ParseFailed)?; | ||
| Ok(RecordSummary { | ||
| reviewer: meta | ||
| .reviewer | ||
| .and_then(|d| wire_name(serde_json::to_value(d))), | ||
| tags: meta | ||
| .tags | ||
| .iter() | ||
| .filter_map(|t| wire_name(serde_json::to_value(t))) | ||
| .collect(), | ||
| title: meta.title, | ||
| }) | ||
| } | ||
|
|
||
| /// A serialized value's wire name (`snake_case` per the schema), if it is a | ||
| /// plain string on the wire. | ||
| fn wire_name(value: serde_json::Result<serde_json::Value>) -> Option<String> { | ||
| match value { | ||
| Ok(serde_json::Value::String(s)) => Some(s), | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| pub fn decide_record(json: &str, decision: CurationDecision) -> Result<String, CurationError> { | ||
| let mut meta: ChunkMeta = serde_json::from_str(json).map_err(|_| CurationError::ParseFailed)?; | ||
| meta.reviewer = Some(match decision { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ use ratatui::{DefaultTerminal, Frame, Terminal}; | |
| use griff_core::classify::BarClass; | ||
|
|
||
| use crate::analysis::Analysis; | ||
| use crate::curation::RecordSummary; | ||
| use crate::scene::{resolve, CellRole, GridSize, Scene, SceneCell, GUTTER}; | ||
| use crate::view::PianoRollView; | ||
| use crate::viewport::{CurationDecision, Intent, Step, ViewContext, Viewport}; | ||
|
|
@@ -81,6 +82,7 @@ pub struct App { | |
| view: PianoRollView, | ||
| analysis: Analysis, | ||
| file: String, | ||
| record: Option<RecordSummary>, | ||
| vp: Viewport, | ||
| ctx: ViewContext, | ||
| } | ||
|
|
@@ -101,11 +103,18 @@ impl App { | |
| view, | ||
| analysis, | ||
| file, | ||
| record: None, | ||
| vp, | ||
| ctx, | ||
| } | ||
| } | ||
|
|
||
| /// Attaches the digest of the `--record` chunk so the inspector shows | ||
| /// the record's current curation state (title, reviewer, tags). | ||
| pub fn set_record(&mut self, record: RecordSummary) { | ||
| self.record = Some(record); | ||
| } | ||
|
|
||
| /// Renders one frame into a `TestBackend` of the given size and returns the | ||
| /// buffer as plain text rows (colours dropped) — used for headless previews | ||
| /// and tests. | ||
|
|
@@ -258,6 +267,23 @@ impl App { | |
| frame.render_widget(paragraph.scroll((scroll, 0)), inner); | ||
| } | ||
|
|
||
| /// Appends the loaded record's curation state (title, prior reviewer | ||
| /// decision, tags) to the inspector lines, when a `--record` is attached. | ||
| fn push_record_lines(&self, lines: &mut Vec<Line<'static>>, dim: Style) { | ||
| let Some(rec) = &self.record else { return }; | ||
| lines.push(Line::from(vec![ | ||
| Span::styled("record ", dim), | ||
| Span::raw(rec.title.clone()), | ||
| ])); | ||
| lines.push(Line::from(format!( | ||
| "review {}", | ||
| rec.reviewer.as_deref().unwrap_or("—") | ||
| ))); | ||
| if !rec.tags.is_empty() { | ||
| lines.push(Line::from(format!("tags {}", rec.tags.join(" ")))); | ||
| } | ||
| } | ||
|
|
||
| /// Builds the inspector's content lines: track, section, curation, | ||
| /// transport (live state first — the PR #38 liveness ordering), then the | ||
| /// static structure and complexity blocks. | ||
|
|
@@ -301,6 +327,8 @@ impl App { | |
| } | ||
| ))); | ||
|
|
||
| self.push_record_lines(&mut lines, dim); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and fixed in 8f6e56c (red → green): the digest is static — loaded once at startup — so it now sits below the transport block with the other static content, exactly per the PR #38 liveness ordering (clipping eats static tails, never live state). Regression test pins Generated by Claude Code |
||
|
|
||
| // Transport sits above the metrics blocks: when the content exceeds | ||
| // the dock, clipping eats the tail of the static metrics, never the | ||
| // live play state (Codex P2, PR #38). | ||
|
|
@@ -618,6 +646,27 @@ mod tests { | |
| ); | ||
| } | ||
|
|
||
| // TDD red phase: the inspector surfaces the loaded record's curation | ||
| // state (the S8 slice before rename/tag). References `App::set_record`, | ||
| // which does not exist yet, so the crate fails to compile until the | ||
| // green step. | ||
| #[test] | ||
| fn inspector_shows_the_loaded_record_state() { | ||
| use crate::curation::RecordSummary; | ||
|
|
||
| let mut app = demo_app(); | ||
| app.set_record(RecordSummary { | ||
| title: "Curated".to_string(), | ||
| reviewer: Some("accepted".to_string()), | ||
| tags: vec!["clean_riff".to_string(), "maj7".to_string()], | ||
| }); | ||
| let frame = app.snapshot(80, 24).expect("snapshot"); | ||
| let text = frame.join("\n"); | ||
| assert!(text.contains("Curated"), "record title shows in the dock"); | ||
| assert!(text.contains("accepted"), "prior reviewer decision shows"); | ||
| assert!(text.contains("clean_riff"), "record tags show"); | ||
| } | ||
|
|
||
| // TDD red phase: Codex P2 (PR #41, round 2) — overscrolling must not | ||
| // accumulate hidden excess: one PgUp from the bottom moves the dock, | ||
| // because the render writes the clamped offset back. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit introduces the new public curation API (
summarize_recordhere, plusRecordSummary) in the same commit as the tests that cover it inpreview/tests/curation.rsandpreview/src/tui.rs. The repo’sAGENTS.mdTDD hard rule says never to commit a newpub fn/pub structimplementation together with its covering tests; for this non-trivial S8 curation slice, please split the red tests and green implementation into separate commits so the failing red phase is reviewable.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The split is in place — the flattened PR diff hides it, but the history has the rule's exact shape:
d34f00e"test: red — the inspector surfaces the loaded record's curation state (S8)" touches onlypreview/tests/curation.rs(+44) and thetui.rstest module (+21); it referencessummarize_record/RecordSummary/set_record, which did not exist yet, so the workspace fails to compile at that commit.466565c"feat(preview): …" touches only implementation —curation.rs(+45),main.rs(+15),tui.rsnon-test code (+28) — and adds zero#[test]items.No code change needed; happy to reorganize if you see a commit where the two actually mix.
Generated by Claude Code