fix(sdd-status): verify-report parser skips pending/TODO when line has pass signal#901
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo functions in Changessddstatus verify-report parser fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/assets/kilocode/agents/sdd-archive.md (1)
35-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
sdd-archiveis missing the phase-standard persistence/result contract.Line 35 returns a minimal payload, but this template omits the mandatory
mem_savesection and the structured result fields used by the other Kilo SDD phase templates in this PR. That creates a cross-phase contract gap for archive outputs and persistence.Suggested contract-aligned patch
4. Return: status=done, summary, artifacts list + +## Engram Save (mandatory) + +After completing work, call `mem_save` with: +- title: `"sdd/{change-name}/archive-report"` +- topic_key: `"sdd/{change-name}/archive-report"` +- type: `"architecture"` +- project: `{project-name from context}` +- capture_prompt: `false` when the Engram tool schema supports it; if an older schema rejects or does not expose the field, omit it rather than failing. + +## Result Contract + +Return a structured result with these fields: +- `status`: `done` | `blocked` | `partial` +- `executive_summary`: one-sentence archive outcome +- `artifacts`: list of files written and topic_keys updated +- `next_recommended`: `none` +- `risks`: archive blockers or follow-up concerns +- `skill_resolution`: `paths-injected` if exact skill paths were provided and loaded, otherwise `none`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/assets/kilocode/agents/sdd-archive.md` around lines 35 - 36, The Return section in the sdd-archive.md template (lines 35-36) is missing critical contract elements that are required by the phase-standard persistence pattern used in other Kilo SDD phase templates. The current minimal payload of "status=done, summary, artifacts list" needs to be expanded to include the mandatory mem_save section for result persistence and the structured result fields used consistently across other SDD phase templates. Review the Return contract patterns in the other Kilo SDD phase templates referenced in this PR, identify the standard payload structure including the mem_save section and result fields, and update the sdd-archive Return statement to match that contract structure to ensure cross-phase compatibility.internal/tui/model.go (1)
3134-3148:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore Codex precedence in SDD-mode back navigation.
Lines 3134-3148 route back from
ScreenSDDModethrough Kilo/Kiro/Claude but skip Codex entirely. When Codex is enabled, Esc can bypass the immediately previous picker and jump too far back.💡 Suggested fix
if m.Screen == ScreenSDDMode { if m.Selection.Preset == model.PresetCustom { - if m.shouldShowKiloModelPickerScreen() { + if m.shouldShowCodexModelPickerScreen() { + m.setScreen(ScreenCodexModelPicker) + } else if m.shouldShowKiloModelPickerScreen() { m.setScreen(ScreenKiloModelPicker) } else if m.shouldShowKiroModelPickerScreen() { m.setScreen(ScreenKiroModelPicker) } else if m.shouldShowClaudeModelPickerScreen() { m.setScreen(ScreenClaudeModelPicker) } else { m.setScreen(ScreenDependencyTree) } return m } - if m.shouldShowKiloModelPickerScreen() { + if m.shouldShowCodexModelPickerScreen() { + m.setScreen(ScreenCodexModelPicker) + return m + } + if m.shouldShowKiloModelPickerScreen() { m.setScreen(ScreenKiloModelPicker) return m }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/model.go` around lines 3134 - 3148, The back navigation logic from ScreenSDDMode (lines 3134-3148) is missing a check for the Codex model picker, which causes Esc to bypass it when enabled. Add a shouldShowCodexModelPickerScreen() check in the if-else chain between the Claude picker check and the default ScreenDependencyTree fallback, calling setScreen(ScreenCodexModelPicker) when true, to restore Codex precedence in the navigation flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/agents.md`:
- Line 114: The parenthetical list of model aliases in the per-phase model
routing description is incomplete and may mislead readers into thinking those
five options (auto, sonnet, opus, haiku, gateway) are the only available
KiloModelAlias choices, when the codebase actually supports 24+ aliases
including OpenAI, Google, DeepSeek, Qwen, Llama, and Mistral variants. Either
remove the parenthetical entirely for clarity, add text like "including" or
"such as" to indicate it is illustrative, or create a separate reference table
or subsection that documents the complete list of supported KiloModelAlias
options to set proper reader expectations.
In `@internal/assets/kilocode/agents/sdd-archive.md`:
- Around line 18-33: The archive report template code block in the
sdd-archive.md file is missing proper markdown lint formatting. Add a blank line
before the opening code fence, specify the markdown language for the fence
(change ``` to ```markdown), and ensure a blank line exists after the closing
fence. This will resolve the fenced-block lint errors related to missing
surrounding blank lines and missing fence language specification.
In `@internal/components/kilojsonc/kilojsonc.go`:
- Around line 21-30: The Generate function accepts an unused parameter
modelAssignments, but its documentation claims this parameter controls model
routing behavior when nil or empty, while the struct-level comments state that
custom model overrides are intentionally not supported. This creates a confusing
API contract. Either remove the modelAssignments parameter from the Generate
function signature and update the caller that invokes Generate to no longer pass
this argument, or add a TODO comment explaining why the parameter exists without
being utilized to reconcile the documentation inconsistency with the actual
implementation behavior.
In `@internal/components/sdd/inject_test.go`:
- Around line 6407-6411: The code at the os.ReadFile call for orchPath silently
skips the assertion when the file is missing because the error check only
proceeds if readErr is nil. To fail fast when gentle-orchestrator.md is absent,
add a check that calls t.Fatal or similar if readErr is not nil, before
proceeding to validate the file content. This ensures the test immediately fails
if the file cannot be read, rather than silently skipping the mode: primary
validation.
In `@internal/model/kilo_model.go`:
- Around line 111-167: In the KiloModelID function's switch statement, update
the Anthropic model IDs that use retired dated snapshots. Replace the return
value for KiloModelOpus from "anthropic/claude-opus-4-20250514" to
"anthropic/claude-opus-4-8", replace KiloModelSonnet from
"anthropic/claude-sonnet-4-20250514" to "anthropic/claude-sonnet-4-6", replace
KiloModelHaiku from "anthropic/claude-haiku-4-20250514" to
"anthropic/claude-haiku-4-5", replace KiloModelSonnet4 from
"anthropic/claude-sonnet-4-20250514" to "anthropic/claude-sonnet-4-6", and
replace KiloModelOpus4 from "anthropic/claude-opus-4-20250514" to
"anthropic/claude-opus-4-8" to use the current canonical model IDs instead of
the retired dated snapshot identifiers.
In `@internal/tui/model_test.go`:
- Around line 1942-1956: The test TestModelConfig_KiloPickerNavigation currently
only validates entry into ScreenKiloModelPicker but does not verify the complete
flow when confirming a Kilo selection. After the existing assertions in
TestModelConfig_KiloPickerNavigation that verify entry with ModelConfigMode
being true, add a follow-up section that calls Update again to simulate
confirming the Kilo selection, then assert that ModelConfigMode becomes false,
the Screen transitions to ScreenSync, and PendingSyncOverrides is properly
populated with Kilo assignments. Reference similar confirmation test patterns in
the file (such as Claude, Kiro, OpenCode, or Codex tests) to ensure consistency
in the assertion structure.
In `@internal/tui/screens/kilo_model_picker.go`:
- Around line 134-137: The kilo_model_picker.go file has an inconsistency
between how aliases are cycled and how they are displayed. At the cycling
location (lines 134-137), when nextKiloAlias is called on
state.CustomAssignments[phase], missing phase entries cause inconsistent
behavior because runtime resolution falls back to "default" but the picker shows
"auto" for missing keys. At the render location (lines 204-207), missing keys
are displayed as "auto" but this doesn't match the actual runtime behavior.
Apply a "default" fallback in both places: when cycling through aliases in the
phase assignment logic, ensure that missing phases in state.CustomAssignments
are initialized with or fallback to "default" before being passed to
nextKiloAlias, and when rendering at lines 204-207, ensure the display logic
also respects "default" as the initial fallback value before displaying "auto",
so the picker's behavior matches runtime resolution.
---
Outside diff comments:
In `@internal/assets/kilocode/agents/sdd-archive.md`:
- Around line 35-36: The Return section in the sdd-archive.md template (lines
35-36) is missing critical contract elements that are required by the
phase-standard persistence pattern used in other Kilo SDD phase templates. The
current minimal payload of "status=done, summary, artifacts list" needs to be
expanded to include the mandatory mem_save section for result persistence and
the structured result fields used consistently across other SDD phase templates.
Review the Return contract patterns in the other Kilo SDD phase templates
referenced in this PR, identify the standard payload structure including the
mem_save section and result fields, and update the sdd-archive Return statement
to match that contract structure to ensure cross-phase compatibility.
In `@internal/tui/model.go`:
- Around line 3134-3148: The back navigation logic from ScreenSDDMode (lines
3134-3148) is missing a check for the Codex model picker, which causes Esc to
bypass it when enabled. Add a shouldShowCodexModelPickerScreen() check in the
if-else chain between the Claude picker check and the default
ScreenDependencyTree fallback, calling setScreen(ScreenCodexModelPicker) when
true, to restore Codex precedence in the navigation flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 26eec8bc-85e1-42e5-a6a6-092efa95fed7
📒 Files selected for processing (30)
docs/agents.mdinternal/agents/kilocode/adapter.gointernal/agents/kilocode/adapter_test.gointernal/assets/assets.gointernal/assets/kilocode/agents/sdd-apply.mdinternal/assets/kilocode/agents/sdd-archive.mdinternal/assets/kilocode/agents/sdd-design.mdinternal/assets/kilocode/agents/sdd-explore.mdinternal/assets/kilocode/agents/sdd-init.mdinternal/assets/kilocode/agents/sdd-onboard.mdinternal/assets/kilocode/agents/sdd-propose.mdinternal/assets/kilocode/agents/sdd-spec.mdinternal/assets/kilocode/agents/sdd-tasks.mdinternal/assets/kilocode/agents/sdd-verify.mdinternal/components/kilojsonc/kilojsonc.gointernal/components/sdd/inject.gointernal/components/sdd/inject_test.gointernal/model/kilo_model.gointernal/model/kilo_model_test.gointernal/model/selection.gointernal/sddstatus/status.gointernal/sddstatus/status_test.gointernal/tui/model.gointernal/tui/model_test.gointernal/tui/router.gointernal/tui/screens/kilo_model_picker.gointernal/tui/screens/kilo_model_picker_test.gointernal/tui/screens/model_config.gointernal/tui/screens/model_config_test.gointernal/tui/screens/sdd_phases.go
| - Uses the OpenCode-compatible adapter: `AGENTS.md`, `skills/`, `commands/`, and `opencode.json` live under `~/.config/kilo` | ||
| - Full SDD delegation is provided by the merged multi-agent overlay in `~/.config/kilo/opencode.json`, not by a separate native sub-agent directory | ||
| - **Native sub-agents**: gentle-ai writes 10 SDD phase agent files to `~/.kilo/agents/sdd-{phase}.md` with YAML frontmatter, following the same pattern as Cursor and Kiro | ||
| - **Per-phase model routing**: supports `KiloModelAssignments` for assigning different Kilo Gateway models to each SDD phase (auto, sonnet, opus, haiku, gateway) |
There was a problem hiding this comment.
Clarify or remove the incomplete model alias list.
The parenthetical (auto, sonnet, opus, haiku, gateway) lists only 5 aliases but the codebase defines 24+ KiloModelAlias constants (including OpenAI o1/o3, Google Gemini variants, DeepSeek, Qwen, Llama, Mistral, etc.). Readers may incorrectly assume these five are the only options for per-phase model routing. Either expand the list to show it's illustrative, remove the parenthetical entirely, or reference the full list in a separate table or subsection for clarity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/agents.md` at line 114, The parenthetical list of model aliases in the
per-phase model routing description is incomplete and may mislead readers into
thinking those five options (auto, sonnet, opus, haiku, gateway) are the only
available KiloModelAlias choices, when the codebase actually supports 24+
aliases including OpenAI, Google, DeepSeek, Qwen, Llama, and Mistral variants.
Either remove the parenthetical entirely for clarity, add text like "including"
or "such as" to indicate it is illustrative, or create a separate reference
table or subsection that documents the complete list of supported KiloModelAlias
options to set proper reader expectations.
| ``` | ||
| # Archive Report: {change-name} | ||
| ## Date: {today} | ||
| ## Status: ARCHIVED | ||
| ## Summary | ||
| [one paragraph from proposal.md] | ||
| ## Artifacts | ||
| - proposal.md | ||
| - spec.md | ||
| - design.md | ||
| - tasks.md | ||
| ## Task Completion | ||
| [count checked vs total from tasks.md] | ||
| ## Next Steps | ||
| - Review and apply if needed | ||
| ``` |
There was a problem hiding this comment.
Fix markdownlint fence issues in the archive report example block.
Line 18 triggers fenced-block lint problems (missing surrounding blank lines and missing fence language). This is a straightforward docs-lint fix.
Minimal lint-safe diff
-3. Write `openspec/changes/archive/{change-name}/archive-report.md` with:
- ```
+3. Write `openspec/changes/archive/{change-name}/archive-report.md` with:
+
+ ```markdown
# Archive Report: {change-name}
## Date: {today}
## Status: ARCHIVED
## Summary
[one paragraph from proposal.md]
## Artifacts
- proposal.md
- spec.md
- design.md
- tasks.md
## Task Completion
[count checked vs total from tasks.md]
## Next Steps
- Review and apply if needed
```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| # Archive Report: {change-name} | |
| ## Date: {today} | |
| ## Status: ARCHIVED | |
| ## Summary | |
| [one paragraph from proposal.md] | |
| ## Artifacts | |
| - proposal.md | |
| - spec.md | |
| - design.md | |
| - tasks.md | |
| ## Task Completion | |
| [count checked vs total from tasks.md] | |
| ## Next Steps | |
| - Review and apply if needed | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 18-18: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/assets/kilocode/agents/sdd-archive.md` around lines 18 - 33, The
archive report template code block in the sdd-archive.md file is missing proper
markdown lint formatting. Add a blank line before the opening code fence,
specify the markdown language for the fence (change ``` to ```markdown), and
ensure a blank line exists after the closing fence. This will resolve the
fenced-block lint errors related to missing surrounding blank lines and missing
fence language specification.
Source: Linters/SAST tools
| // Generate writes ~/.config/kilo/kilo.jsonc with Kilo Gateway provider config | ||
| // and model routing. The file is created if it does not exist, or deep-merged | ||
| // if it already exists. | ||
| // | ||
| // modelAssignments maps phase names to KiloModelAlias values. When nil or | ||
| // empty, the default balanced preset is used. | ||
| func Generate(homeDir string, modelAssignments map[string]model.KiloModelAlias) (bool, error) { | ||
| if homeDir == "" { | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Unused parameter modelAssignments with inconsistent documentation.
The function signature accepts modelAssignments and the doc comment (lines 25-26) describes its intended behavior, but the parameter is never used in the function body. Meanwhile, the struct comment at lines 14-16 states the opposite: "We only inject the $schema key... injecting custom providers or model overrides conflicts with its built-in gateway."
This creates a confusing API contract where callers pass data that is silently discarded. Either:
- Remove the unused parameter if model routing is intentionally not implemented
- Add a
// TODOor update the doc to clarify why the parameter exists but is unused
Option 1: Remove unused parameter
-func Generate(homeDir string, modelAssignments map[string]model.KiloModelAlias) (bool, error) {
+func Generate(homeDir string) (bool, error) {This requires updating the caller in inject.go line 853.
Option 2: Document why parameter exists but is unused
// modelAssignments maps phase names to KiloModelAlias values. When nil or
-// empty, the default balanced preset is used.
+// empty, the default balanced preset is used.
+//
+// NOTE: modelAssignments is currently unused. Kilo Code handles model routing
+// natively via its built-in gateway; injecting custom model config here caused
+// server errors. The parameter is retained for potential future use when Kilo
+// supports external model configuration.
func Generate(homeDir string, modelAssignments map[string]model.KiloModelAlias) (bool, error) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Generate writes ~/.config/kilo/kilo.jsonc with Kilo Gateway provider config | |
| // and model routing. The file is created if it does not exist, or deep-merged | |
| // if it already exists. | |
| // | |
| // modelAssignments maps phase names to KiloModelAlias values. When nil or | |
| // empty, the default balanced preset is used. | |
| func Generate(homeDir string, modelAssignments map[string]model.KiloModelAlias) (bool, error) { | |
| if homeDir == "" { | |
| return false, nil | |
| } | |
| // Generate writes ~/.config/kilo/kilo.jsonc with Kilo Gateway provider config | |
| // and model routing. The file is created if it does not exist, or deep-merged | |
| // if it already exists. | |
| // | |
| // modelAssignments maps phase names to KiloModelAlias values. When nil or | |
| // empty, the default balanced preset is used. | |
| // | |
| // NOTE: modelAssignments is currently unused. Kilo Code handles model routing | |
| // natively via its built-in gateway; injecting custom model config here caused | |
| // server errors. The parameter is retained for potential future use when Kilo | |
| // supports external model configuration. | |
| func Generate(homeDir string, modelAssignments map[string]model.KiloModelAlias) (bool, error) { | |
| if homeDir == "" { | |
| return false, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/components/kilojsonc/kilojsonc.go` around lines 21 - 30, The
Generate function accepts an unused parameter modelAssignments, but its
documentation claims this parameter controls model routing behavior when nil or
empty, while the struct-level comments state that custom model overrides are
intentionally not supported. This creates a confusing API contract. Either
remove the modelAssignments parameter from the Generate function signature and
update the caller that invokes Generate to no longer pass this argument, or add
a TODO comment explaining why the parameter exists without being utilized to
reconcile the documentation inconsistency with the actual implementation
behavior.
| if orchContent, readErr := os.ReadFile(orchPath); readErr == nil { | ||
| if strings.Contains(string(orchContent), "mode: primary") { | ||
| t.Fatal("gentle-orchestrator.md must not use mode: primary (Kilo v7 rejects this)") | ||
| } | ||
| } |
There was a problem hiding this comment.
Fail fast if gentle-orchestrator.md is missing.
At Line 6407, the read error is ignored, so this assertion is skipped when the file is absent. That can hide a broken injection path.
Suggested fix
- if orchContent, readErr := os.ReadFile(orchPath); readErr == nil {
- if strings.Contains(string(orchContent), "mode: primary") {
- t.Fatal("gentle-orchestrator.md must not use mode: primary (Kilo v7 rejects this)")
- }
- }
+ orchContent, readErr := os.ReadFile(orchPath)
+ if readErr != nil {
+ t.Fatalf("ReadFile(gentle-orchestrator.md) error = %v", readErr)
+ }
+ if strings.Contains(string(orchContent), "mode: primary") {
+ t.Fatal("gentle-orchestrator.md must not use mode: primary (Kilo v7 rejects this)")
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if orchContent, readErr := os.ReadFile(orchPath); readErr == nil { | |
| if strings.Contains(string(orchContent), "mode: primary") { | |
| t.Fatal("gentle-orchestrator.md must not use mode: primary (Kilo v7 rejects this)") | |
| } | |
| } | |
| orchContent, readErr := os.ReadFile(orchPath) | |
| if readErr != nil { | |
| t.Fatalf("ReadFile(gentle-orchestrator.md) error = %v", readErr) | |
| } | |
| if strings.Contains(string(orchContent), "mode: primary") { | |
| t.Fatal("gentle-orchestrator.md must not use mode: primary (Kilo v7 rejects this)") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/components/sdd/inject_test.go` around lines 6407 - 6411, The code at
the os.ReadFile call for orchPath silently skips the assertion when the file is
missing because the error check only proceeds if readErr is nil. To fail fast
when gentle-orchestrator.md is absent, add a check that calls t.Fatal or similar
if readErr is not nil, before proceeding to validate the file content. This
ensures the test immediately fails if the file cannot be read, rather than
silently skipping the mode: primary validation.
| func KiloModelID(alias KiloModelAlias) string { | ||
| switch alias { | ||
| // Kilo Gateway | ||
| case KiloModelAuto: | ||
| return "kilo/kilo-auto/free" | ||
| case KiloModelGateway: | ||
| return "gateway/auto" | ||
| // Anthropic | ||
| case KiloModelOpus: | ||
| return "anthropic/claude-opus-4-20250514" | ||
| case KiloModelSonnet: | ||
| return "anthropic/claude-sonnet-4-20250514" | ||
| case KiloModelHaiku: | ||
| return "anthropic/claude-haiku-4-20250514" | ||
| case KiloModelSonnet4: | ||
| return "anthropic/claude-sonnet-4-20250514" | ||
| case KiloModelOpus4: | ||
| return "anthropic/claude-opus-4-20250514" | ||
| // OpenAI | ||
| case KiloModelGPT4o: | ||
| return "openai/gpt-4o" | ||
| case KiloModelGPT4oMini: | ||
| return "openai/gpt-4o-mini" | ||
| case KiloModelO1: | ||
| return "openai/o1" | ||
| case KiloModelO3: | ||
| return "openai/o3" | ||
| case KiloModelO3Mini: | ||
| return "openai/o3-mini" | ||
| case KiloModelO4Mini: | ||
| return "openai/o4-mini" | ||
| case KiloModelGemini25Pro: | ||
| return "google/gemini-2.5-pro" | ||
| case KiloModelGemini25Flash: | ||
| return "google/gemini-2.5-flash" | ||
| case KiloModelGemini20Flash: | ||
| return "google/gemini-2.0-flash" | ||
| // Open-weight | ||
| case KiloModelDeepSeek: | ||
| return "deepseek/deepseek-chat" | ||
| case KiloModelDeepSeekR1: | ||
| return "deepseek/deepseek-reasoner" | ||
| case KiloModelQwen: | ||
| return "qwen/qwen-2.5-72b-instruct" | ||
| case KiloModelQwen3: | ||
| return "qwen/qwen3-235b-a22b" | ||
| case KiloModelLlama: | ||
| return "meta-llama/llama-4-maverick" | ||
| case KiloModelMistral: | ||
| return "mistral/mistral-large-latest" | ||
| default: | ||
| // Pass-through: unknown alias is used as-is as the model ID. | ||
| // This allows users to type any provider/model string that Kilo Gateway accepts. | ||
| return string(alias) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the latest Anthropic Claude model IDs available in the API as of June 2026?
💡 Result:
As of June 16, 2026, the Anthropic Claude API provides access to several generations of models [1][2]. Anthropic uses a versioning scheme where newer models (4.6 generation and later) use dateless, canonical model IDs that are pinned snapshots [3][1]. Older models may use dated snapshots or convenience aliases [3][1]. The following are the active, current Claude models available in the API [4]: - Claude Fable 5: Identified as claude-fable-5 [4]. - Claude Mythos 5: Identified as claude-mythos-5 (noted as available for Project Glasswing participants) [4]. - Claude Opus 4.8: Identified as claude-opus-4-8 [1][4]. - Claude Opus 4.7: Identified as claude-opus-4-7 [1][4]. - Claude Opus 4.6: Identified as claude-opus-4-6 [1][4]. - Claude Sonnet 4.6: Identified as claude-sonnet-4-6 [1][4]. - Claude Haiku 4.5: Identified as claude-haiku-4-5 (associated with the specific dated snapshot claude-haiku-4-5-20251001) [1][4]. Note that some older models, such as Claude Opus 4.1 (claude-opus-4-1-20250805), are in the process of deprecation or have been retired [2][4]. As of June 15, 2026, the Claude Sonnet 4 and Claude Opus 4 models were retired from the API [2]. Anthropic recommends using the GET /v1/models endpoint in the API to programmatically discover the full list of available models, validate specific IDs, and resolve aliases for your account [2][5][6][7]. The API response provides metadata, including capabilities, max_input_tokens, and created_at timestamps, to help determine the appropriate model for your use case [8][2][5].
Citations:
- 1: https://platform.claude.com/docs/en/about-claude/models/overview
- 2: https://platform.claude.com/docs/en/release-notes/overview
- 3: https://platform.claude.com/docs/en/about-claude/models/model-ids-and-versions
- 4: https://github.com/anthropics/skills/blob/main/skills/claude-api/shared/models.md
- 5: https://platform.claude.com/docs/en/api/models/list
- 6: https://platform.claude.com/docs/en/api/typescript/models
- 7: https://platform.claude.com/docs/en/api/go/models/list
- 8: https://platform.claude.com/docs/en/api/models
Update Anthropic model IDs—Claude Opus 4 and Sonnet 4 with snapshot dates were retired.
The Anthropic models at lines 120–128 use the dated snapshot IDs claude-opus-4-20250514 and claude-sonnet-4-20250514, which were retired from the Anthropic API as of June 15, 2026. These will fail when called. Update to the current canonical model IDs: claude-opus-4-8, claude-sonnet-4-6, and claude-haiku-4-5.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/model/kilo_model.go` around lines 111 - 167, In the KiloModelID
function's switch statement, update the Anthropic model IDs that use retired
dated snapshots. Replace the return value for KiloModelOpus from
"anthropic/claude-opus-4-20250514" to "anthropic/claude-opus-4-8", replace
KiloModelSonnet from "anthropic/claude-sonnet-4-20250514" to
"anthropic/claude-sonnet-4-6", replace KiloModelHaiku from
"anthropic/claude-haiku-4-20250514" to "anthropic/claude-haiku-4-5", replace
KiloModelSonnet4 from "anthropic/claude-sonnet-4-20250514" to
"anthropic/claude-sonnet-4-6", and replace KiloModelOpus4 from
"anthropic/claude-opus-4-20250514" to "anthropic/claude-opus-4-8" to use the
current canonical model IDs instead of the retired dated snapshot identifiers.
| func TestModelConfig_KiloPickerNavigation(t *testing.T) { | ||
| m := NewModel(system.DetectionResult{}, "dev") | ||
| m.Screen = ScreenModelConfig | ||
| m.Cursor = 3 | ||
|
|
||
| updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyEnter}) | ||
| state := updated.(Model) | ||
|
|
||
| if state.Screen != ScreenKiloModelPicker { | ||
| t.Fatalf("ModelConfig cursor=3 (Kilo): screen = %v, want %v", state.Screen, ScreenKiloModelPicker) | ||
| } | ||
| if !state.ModelConfigMode { | ||
| t.Fatalf("ModelConfigMode should be true after entering Kilo picker from ModelConfig") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add a ModelConfigMode “confirm Kilo selection → ScreenSync” parity test.
This new test only validates entry into ScreenKiloModelPicker. Please add a follow-up assertion path (like Claude/Kiro/OpenCode/Codex tests in this file) that confirming Kilo selection exits ModelConfigMode, routes to ScreenSync, and sets PendingSyncOverrides with Kilo assignments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tui/model_test.go` around lines 1942 - 1956, The test
TestModelConfig_KiloPickerNavigation currently only validates entry into
ScreenKiloModelPicker but does not verify the complete flow when confirming a
Kilo selection. After the existing assertions in
TestModelConfig_KiloPickerNavigation that verify entry with ModelConfigMode
being true, add a follow-up section that calls Update again to simulate
confirming the Kilo selection, then assert that ModelConfigMode becomes false,
the Screen transitions to ScreenSync, and PendingSyncOverrides is properly
populated with Kilo assignments. Reference similar confirmation test patterns in
the file (such as Claude, Kiro, OpenCode, or Codex tests) to ensure consistency
in the assertion structure.
| if cursor < len(sddPhases) { | ||
| phase := sddPhases[cursor] | ||
| state.CustomAssignments[phase] = nextKiloAlias(state.CustomAssignments[phase]) | ||
| return true, nil |
There was a problem hiding this comment.
Apply default fallback for missing phase aliases in custom mode.
Line 136 cycles from state.CustomAssignments[phase] directly, and Lines 204-207 render missing keys as auto. But runtime resolution falls back to "default" before auto, so partial maps can behave differently than what the picker shows/cycles.
💡 Suggested fix
+func effectiveKiloAlias(assignments map[string]model.KiloModelAlias, phase string) model.KiloModelAlias {
+ if assignments != nil {
+ if a, ok := assignments[phase]; ok && a != "" {
+ return a
+ }
+ if d, ok := assignments["default"]; ok && d != "" {
+ return d
+ }
+ }
+ return model.KiloModelAuto
+}
+
func handleKiloCustomPhaseNav(
key string,
state *KiloModelPickerState,
cursor int,
) (bool, map[string]model.KiloModelAlias) {
@@
case "enter":
if cursor < len(sddPhases) {
phase := sddPhases[cursor]
- state.CustomAssignments[phase] = nextKiloAlias(state.CustomAssignments[phase])
+ if state.CustomAssignments == nil {
+ state.CustomAssignments = map[string]model.KiloModelAlias{}
+ }
+ current := effectiveKiloAlias(state.CustomAssignments, phase)
+ state.CustomAssignments[phase] = nextKiloAlias(current)
return true, nil
}
@@
for idx, phase := range sddPhases {
focused := idx == cursor
- alias := state.CustomAssignments[phase]
- if alias == "" {
- alias = model.KiloModelAuto
- }
+ alias := effectiveKiloAlias(state.CustomAssignments, phase)Also applies to: 204-207
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tui/screens/kilo_model_picker.go` around lines 134 - 137, The
kilo_model_picker.go file has an inconsistency between how aliases are cycled
and how they are displayed. At the cycling location (lines 134-137), when
nextKiloAlias is called on state.CustomAssignments[phase], missing phase entries
cause inconsistent behavior because runtime resolution falls back to "default"
but the picker shows "auto" for missing keys. At the render location (lines
204-207), missing keys are displayed as "auto" but this doesn't match the actual
runtime behavior. Apply a "default" fallback in both places: when cycling
through aliases in the phase assignment logic, ensure that missing phases in
state.CustomAssignments are initialized with or fallback to "default" before
being passed to nextKiloAlias, and when rendering at lines 204-207, ensure the
display logic also respects "default" as the initial fallback value before
displaying "auto", so the picker's behavior matches runtime resolution.
…s signal The verify-report parser treated any line containing TODO or PENDING as a blocker, even when the line also carried an explicit pass signal (✅ glyph, COMPLIANT status, or PASS verdict). This caused false negatives that blocked archive for fully passing reports. Fix: - reportLineHasBlocker: gate pending pattern on !reportLineHasPassSignal - reportLineHasPassSignal: detect ✅ glyph as pass signal in prose - Add 3 test cases covering compliant+TODO, standalone TODO, and mixed Closes Gentleman-Programming#848
9a98865 to
83b2c3e
Compare
There was a problem hiding this comment.
I’m requesting one change before merge. reportLineHasPassSignal now makes any ✅ count as whole-report pass evidence, but the bug only needs a local TODO/PENDING exception. That can mark a report as archive-ready without an explicit PASS or SUCCESS verdict. Please split local row handling from whole-report verdict detection, or require the glyph to appear with a clear compliant/pass token.
Summary
The verify-report parser in
sdd-statustreated any line containingTODOorPENDINGas a blocker, even when the line also carried an explicit pass signal (✅ glyph, COMPLIANT status, or PASS verdict). This caused false negatives that blocked archive for fully passing reports.Root Cause
reportPendingPatternregex matchedTODO|PENDINGanywhere in a line.reportLineHasBlockershort-circuited on this match before checking for pass signals. Meanwhile,reportLineHasPassSignaldidn't recognize the ✅ glyph as a pass marker.Fix
reportLineHasBlocker: Gate pending pattern on!reportLineHasPassSignal(line)— lines with ✅ or other pass signals are no longer treated as blockersreportLineHasPassSignal: Detect ✅ glyph as pass signal in prose textTest Cases
✅ TODO: finish audittodo: security review(standalone)✅ TODO+ standalonetodoVerification
go vetcleanScope
2 files changed, +45/-1 lines. Well under 400-line review budget.
Closes #848
Summary by CodeRabbit
Bug Fixes
Tests