Skip to content

Commit 79e7f2f

Browse files
authored
Fix: km config output matches AppConfig structure (#1095)
## Problem `km config` output used DTOs with different structure than the actual AppConfig format, making it impossible for users to copy/paste output back into their config file. **Before (wrong structure):** ```json { "nodes": [ // ❌ Array (wrong!) { "nodeId": "personal", ... } ], "cache": { // ❌ Wrong property name "embeddingsCache": {...} } } ``` **After (correct structure):** ```json { "nodes": { // ✅ Object/Dictionary (correct!) "personal": { "id": "personal", ... } }, "embeddingsCache": { // ✅ At root level (correct!) ... } } ``` ## Solution - ConfigCommand default output is now `this.Config` (actual AppConfig object) - Users can now copy/paste the output directly into their config file - `--show-nodes` and `--show-cache` flags still use DTOs (unchanged) ## TDD Approach 1. ✅ Test written first (RED) - verified structure mismatch 2. ✅ Code fixed (GREEN) - output now matches AppConfig 3. ✅ All validation scripts pass ## Changes - Update `ConfigCommand` to output actual AppConfig (not DTOs) - Add test verifying output structure matches config file format - Add `embeddingsCache` to test fixture for complete validation ## Definition of Done - [x] Test written first (TDD approach) - [x] `./build.sh` passes - 0 warnings, 0 errors - [x] `./format.sh` passes - [x] `./coverage.sh` passes - 81.64% coverage (>80% minimum) - [x] All 263 tests passing (186 Main + 77 Core) - [x] Zero skipped tests
1 parent 5b70560 commit 79e7f2f

File tree

2 files changed

+66
-43
lines changed

2 files changed

+66
-43
lines changed

src/Main/CLI/Commands/ConfigCommand.cs

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -81,48 +81,9 @@ public override async Task<int> ExecuteAsync(
8181
}
8282
else
8383
{
84-
// Default: show entire configuration with all nodes
85-
var config = this.Config;
86-
output = new
87-
{
88-
Nodes = config.Nodes.Select(kvp => new NodeDetailsDto
89-
{
90-
NodeId = kvp.Key,
91-
Access = kvp.Value.Access.ToString(),
92-
ContentIndex = new ContentIndexConfigDto
93-
{
94-
Type = kvp.Value.ContentIndex.Type.ToString(),
95-
Path = kvp.Value.ContentIndex is KernelMemory.Core.Config.ContentIndex.SqliteContentIndexConfig sqlite
96-
? sqlite.Path
97-
: null
98-
},
99-
FileStorage = kvp.Value.FileStorage != null ? new StorageConfigDto
100-
{
101-
Type = kvp.Value.FileStorage.Type.ToString()
102-
} : null,
103-
RepoStorage = kvp.Value.RepoStorage != null ? new StorageConfigDto
104-
{
105-
Type = kvp.Value.RepoStorage.Type.ToString()
106-
} : null,
107-
SearchIndexes = kvp.Value.SearchIndexes.Select(si => new SearchIndexDto
108-
{
109-
Type = si.Type.ToString()
110-
}).ToList()
111-
}).ToList(),
112-
Cache = new CacheInfoDto
113-
{
114-
EmbeddingsCache = config.EmbeddingsCache != null ? new CacheConfigDto
115-
{
116-
Type = config.EmbeddingsCache.Type.ToString(),
117-
Path = config.EmbeddingsCache.Path
118-
} : null,
119-
LlmCache = config.LLMCache != null ? new CacheConfigDto
120-
{
121-
Type = config.LLMCache.Type.ToString(),
122-
Path = config.LLMCache.Path
123-
} : null
124-
}
125-
};
84+
// Default: show the actual AppConfig structure (not DTOs)
85+
// This allows users to copy/paste the output into their config file
86+
output = this.Config;
12687
}
12788

12889
formatter.Format(output);

tests/Main.Tests/Integration/ConfigCommandTests.cs

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

33
using KernelMemory.Core.Config;
4+
using KernelMemory.Core.Config.Cache;
45
using KernelMemory.Main.CLI.Commands;
56
using Spectre.Console.Cli;
67
using Xunit;
@@ -32,7 +33,9 @@ public ConfigCommandTests()
3233
["personal"] = NodeConfig.CreateDefaultPersonalNode(Path.Combine(this._tempDir, "nodes", "personal")),
3334
["work"] = NodeConfig.CreateDefaultPersonalNode(Path.Combine(this._tempDir, "nodes", "work")),
3435
["shared"] = NodeConfig.CreateDefaultPersonalNode(Path.Combine(this._tempDir, "nodes", "shared"))
35-
}
36+
},
37+
EmbeddingsCache = CacheConfig.CreateDefaultSqliteCache(Path.Combine(this._tempDir, "embeddings-cache.db")),
38+
LLMCache = null
3639
};
3740

3841
var json = System.Text.Json.JsonSerializer.Serialize(config);
@@ -109,6 +112,65 @@ public void ConfigCommand_WithoutFlags_ShouldShowEntireConfiguration()
109112
}
110113
}
111114

115+
[Fact]
116+
public void ConfigCommand_OutputStructure_ShouldMatchAppConfigFormat()
117+
{
118+
// This test verifies the BUG: km config output should match AppConfig structure
119+
// so users can copy/paste the output back into their config file
120+
121+
// Arrange
122+
var config = ConfigParser.LoadFromFile(this._configPath);
123+
124+
var settings = new ConfigCommandSettings
125+
{
126+
ConfigPath = this._configPath,
127+
Format = "json"
128+
};
129+
130+
var command = new ConfigCommand(config);
131+
var context = new CommandContext(
132+
new[] { "--config", this._configPath },
133+
new EmptyRemainingArguments(),
134+
"config",
135+
null);
136+
137+
// Capture stdout
138+
using var outputCapture = new StringWriter();
139+
var originalOutput = Console.Out;
140+
Console.SetOut(outputCapture);
141+
142+
try
143+
{
144+
// Act
145+
var exitCode = command.ExecuteAsync(context, settings).GetAwaiter().GetResult();
146+
147+
// Assert
148+
Assert.Equal(Constants.ExitCodeSuccess, exitCode);
149+
150+
var output = outputCapture.ToString();
151+
var outputJson = System.Text.Json.JsonDocument.Parse(output);
152+
153+
// BUG: Current output has "nodes" as an array
154+
// EXPECTED: "nodes" should be an object/dictionary (like in config file)
155+
var nodesElement = outputJson.RootElement.GetProperty("nodes");
156+
Assert.Equal(System.Text.Json.JsonValueKind.Object, nodesElement.ValueKind);
157+
// ^ This will FAIL with current code (it's an Array)
158+
159+
// BUG: Current output has "embeddingsCache" nested under "cache"
160+
// EXPECTED: "embeddingsCache" should be at root level (like in config file)
161+
Assert.True(outputJson.RootElement.TryGetProperty("embeddingsCache", out _));
162+
// ^ This will FAIL with current code (no "embeddingsCache" at root)
163+
164+
// Should NOT have a "cache" wrapper
165+
Assert.False(outputJson.RootElement.TryGetProperty("cache", out _));
166+
// ^ This will FAIL with current code (it has "cache" wrapper)
167+
}
168+
finally
169+
{
170+
Console.SetOut(originalOutput);
171+
}
172+
}
173+
112174
[Fact]
113175
public void ConfigCommand_WithShowNodesFlag_ShouldShowAllNodesSummary()
114176
{

0 commit comments

Comments
 (0)