diff --git a/docs/config.md b/docs/config.md index 64e4480..797150b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -198,7 +198,7 @@ Rules: | `pipeline.render.format` | string | No | `markdown` (only supported value) | | `pipeline.render.title` | string | No | empty (falls back to `session.title` when set) | | `pipeline.render.include_timestamps` | bool | No | `true` | -| `pipeline.render.include_segment_ids` | bool | No | `false` | +| `pipeline.render.include_segment_ids` | bool | No | `true` | | `pipeline.render.include_metadata` | bool | No | `false` | | `pipeline.scriptorium.binary` | string | No | `scriptorium` | | `pipeline.scriptorium.config_path` | string | No | empty | diff --git a/internal/adapters/seriatim/subprocess.go b/internal/adapters/seriatim/subprocess.go index 835e3a7..124db7e 100644 --- a/internal/adapters/seriatim/subprocess.go +++ b/internal/adapters/seriatim/subprocess.go @@ -577,9 +577,9 @@ func buildRenderArgs(req RenderRequest, format string) []string { "--input-file", req.InputTranscriptPath, "--output-file", req.OutputRenderedPath, "--format", format, - "--include-timestamps", strconv.FormatBool(req.IncludeTimestamps), - "--include-segment-ids", strconv.FormatBool(req.IncludeSegmentIDs), - "--include-metadata", strconv.FormatBool(req.IncludeMetadata), + "--include-timestamps=" + strconv.FormatBool(req.IncludeTimestamps), + "--include-segment-ids=" + strconv.FormatBool(req.IncludeSegmentIDs), + "--include-metadata=" + strconv.FormatBool(req.IncludeMetadata), } if strings.TrimSpace(req.Title) != "" { args = append(args, "--title", req.Title) diff --git a/internal/adapters/seriatim/subprocess_test.go b/internal/adapters/seriatim/subprocess_test.go index d684db7..42e7af8 100644 --- a/internal/adapters/seriatim/subprocess_test.go +++ b/internal/adapters/seriatim/subprocess_test.go @@ -628,9 +628,9 @@ func TestSubprocessRunnerRenderSuccessInvocationAndProvenance(t *testing.T) { "--input-file", req.InputTranscriptPath, "--output-file", req.OutputRenderedPath, "--format", req.Format, - "--include-timestamps", "true", - "--include-segment-ids", "false", - "--include-metadata", "true", + "--include-timestamps=true", + "--include-segment-ids=true", + "--include-metadata=false", "--title", req.Title, } if strings.Join(rec.Args, "\n") != strings.Join(wantArgs, "\n") { @@ -946,8 +946,8 @@ func renderReqForTest(t *testing.T) RenderRequest { Format: "markdown", Title: "Session 42", IncludeTimestamps: true, - IncludeSegmentIDs: false, - IncludeMetadata: true, + IncludeSegmentIDs: true, + IncludeMetadata: false, GeneratedConfigPath: filepath.Join(dir, "seriatim.render.generated.yml"), StdoutLogPath: filepath.Join(dir, "seriatim.render.stdout.log"), StderrLogPath: filepath.Join(dir, "seriatim.render.stderr.log"), diff --git a/internal/config/config.go b/internal/config/config.go index 991bddc..38bcbb1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -216,7 +216,7 @@ type RenderConfig struct { Format string `yaml:"format"` Title string `yaml:"title"` IncludeTimestamps *bool `yaml:"include_timestamps"` - IncludeSegmentIDs bool `yaml:"include_segment_ids"` + IncludeSegmentIDs *bool `yaml:"include_segment_ids"` IncludeMetadata bool `yaml:"include_metadata"` } diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 0898fda..2b6892a 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -44,7 +44,7 @@ const ( DefaultRenderFormat = "markdown" DefaultRenderTitle = "" DefaultRenderTimestamps = true - DefaultRenderSegmentIDs = false + DefaultRenderSegmentIDs = true DefaultRenderMetadata = false DefaultNormalizeOutputPath = artifactmodel.TranscriptPathFinal diff --git a/internal/config/load.go b/internal/config/load.go index 6c17ef4..23c6944 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -527,6 +527,9 @@ func applyRenderDefaults(cfg **RenderConfig) { if (*cfg).IncludeTimestamps == nil { (*cfg).IncludeTimestamps = boolPtr(DefaultRenderTimestamps) } + if (*cfg).IncludeSegmentIDs == nil { + (*cfg).IncludeSegmentIDs = boolPtr(DefaultRenderSegmentIDs) + } } func applyNormalizeDefaults(cfg *NormalizeConfig) { diff --git a/internal/config/render_test.go b/internal/config/render_test.go index 3deca70..5215bda 100644 --- a/internal/config/render_test.go +++ b/internal/config/render_test.go @@ -30,8 +30,8 @@ func TestRenderLoadAndValidate(t *testing.T) { if cfg.Pipeline.Render.IncludeTimestamps == nil || !*cfg.Pipeline.Render.IncludeTimestamps { t.Fatalf("render.include_timestamps = %#v, want true", cfg.Pipeline.Render.IncludeTimestamps) } - if cfg.Pipeline.Render.IncludeSegmentIDs { - t.Fatalf("render.include_segment_ids = true, want false") + if cfg.Pipeline.Render.IncludeSegmentIDs == nil || !*cfg.Pipeline.Render.IncludeSegmentIDs { + t.Fatalf("render.include_segment_ids = %#v, want true", cfg.Pipeline.Render.IncludeSegmentIDs) } if cfg.Pipeline.Render.IncludeMetadata { t.Fatalf("render.include_metadata = true, want false") @@ -59,14 +59,26 @@ func TestRenderLoadAndValidate(t *testing.T) { if cfg.Pipeline.Render.IncludeTimestamps == nil || *cfg.Pipeline.Render.IncludeTimestamps { t.Fatalf("render.include_timestamps = %#v, want false", cfg.Pipeline.Render.IncludeTimestamps) } - if !cfg.Pipeline.Render.IncludeSegmentIDs { - t.Fatalf("render.include_segment_ids = false, want true") + if cfg.Pipeline.Render.IncludeSegmentIDs == nil || !*cfg.Pipeline.Render.IncludeSegmentIDs { + t.Fatalf("render.include_segment_ids = %#v, want true", cfg.Pipeline.Render.IncludeSegmentIDs) } if !cfg.Pipeline.Render.IncludeMetadata { t.Fatalf("render.include_metadata = false, want true") } }, }, + { + name: "explicit segment ids false overrides default", + renderYAML: `render: + include_segment_ids: false +`, + assert: func(t *testing.T, cfg *Config) { + t.Helper() + if cfg.Pipeline.Render.IncludeSegmentIDs == nil || *cfg.Pipeline.Render.IncludeSegmentIDs { + t.Fatalf("render.include_segment_ids = %#v, want false", cfg.Pipeline.Render.IncludeSegmentIDs) + } + }, + }, { name: "invalid render format fails", renderYAML: `render: diff --git a/internal/config/validate.go b/internal/config/validate.go index fb3df42..c5b5d30 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -312,6 +312,9 @@ func validateRender(cfg *RenderConfig) error { if cfg.IncludeTimestamps == nil { return fmt.Errorf("pipeline.render.include_timestamps must be set (defaults should populate this)") } + if cfg.IncludeSegmentIDs == nil { + return fmt.Errorf("pipeline.render.include_segment_ids must be set (defaults should populate this)") + } format := strings.TrimSpace(cfg.Format) if format != "markdown" { return fmt.Errorf("pipeline.render.format must be markdown") diff --git a/internal/stage/render.go b/internal/stage/render.go index c2de8af..639f02a 100644 --- a/internal/stage/render.go +++ b/internal/stage/render.go @@ -69,7 +69,10 @@ func (renderStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*St } title := resolveRenderTitle(renderCfg, env.Config.Session) includeTimestamps := renderCfg.IncludeTimestamps == nil || *renderCfg.IncludeTimestamps - includeSegmentIDs := renderCfg.IncludeSegmentIDs + includeSegmentIDs := config.DefaultRenderSegmentIDs + if renderCfg.IncludeSegmentIDs != nil { + includeSegmentIDs = *renderCfg.IncludeSegmentIDs + } includeMetadata := renderCfg.IncludeMetadata meta := map[string]any{ @@ -237,11 +240,12 @@ func renderConfigOrDefault(cfg *config.RenderConfig) *config.RenderConfig { } enabled := true includeTimestamps := true + includeSegmentIDs := config.DefaultRenderSegmentIDs return &config.RenderConfig{ Enabled: &enabled, Format: config.DefaultRenderFormat, IncludeTimestamps: &includeTimestamps, - IncludeSegmentIDs: config.DefaultRenderSegmentIDs, + IncludeSegmentIDs: &includeSegmentIDs, IncludeMetadata: config.DefaultRenderMetadata, } } diff --git a/internal/stage/render_test.go b/internal/stage/render_test.go index a2a5d8f..57d87c3 100644 --- a/internal/stage/render_test.go +++ b/internal/stage/render_test.go @@ -165,6 +165,7 @@ func setupRenderEnv(t *testing.T) (*Env, *manifest.Manifest, *seriatim.FakeRunne enabled := true includeTimestamps := true + includeSegmentIDs := false seriatimReport := false cfg := &config.Config{ PipelinePath: pipelinePath, @@ -183,7 +184,7 @@ func setupRenderEnv(t *testing.T) (*Env, *manifest.Manifest, *seriatim.FakeRunne Format: "markdown", Title: "Pipeline Title", IncludeTimestamps: &includeTimestamps, - IncludeSegmentIDs: false, + IncludeSegmentIDs: &includeSegmentIDs, IncludeMetadata: false, }, },