From 7fbf122532af847e7dbcb49989eb871ba1948144 Mon Sep 17 00:00:00 2001 From: debug23win Date: Tue, 20 Oct 2026 05:18:21 +0300 Subject: [PATCH 1/3] Fix audit findings in result integrity, model units and agent execution --- .../ClaudeRevit.CliTests.csproj | 1 + ClaudeRevit.NativeTests/README.md | 3 + ClaudeRevit.NativeTests/ReviewFixtures.cs | 78 +++++++ .../ClaudeRevit.PaneTests.csproj | 3 + ClaudeRevit.PaneTests/Program.cs | 18 ++ ClaudeRevit.PaneTests/Stubs.cs | 7 +- ClaudeRevit.Tests/AuditSafetyTests.cs | 210 ++++++++++++++++++ ClaudeRevit.Tests/BenchmarkRunnerStubs.cs | 1 + ClaudeRevit.Tests/BenchmarkRunnerTests.cs | 2 +- ClaudeRevit.Tests/ClaudeRevit.Tests.csproj | 1 + ClaudeRevit.Tests/ToolResultAgingTests.cs | 2 +- ClaudeRevit/Services/AttachmentImage.cs | 4 +- ClaudeRevit/Services/BenchmarkRunner.cs | 4 +- ClaudeRevit/Services/CatalogPage.cs | 42 ++++ ClaudeRevit/Services/ChatService.cs | 58 ++--- ClaudeRevit/Services/ClaudeCodeBackend.cs | 1 + ClaudeRevit/Services/CodexBackend.cs | 1 + ClaudeRevit/Services/HeadlessResultLimits.cs | 6 +- ClaudeRevit/Services/McpServer.cs | 2 +- ClaudeRevit/Services/TaskJournal.cs | 25 ++- ClaudeRevit/Services/ToolResult.cs | 42 +++- ClaudeRevit/Services/ToolResultAging.cs | 15 +- ClaudeRevit/Services/ToolResultArchive.cs | 21 +- ClaudeRevit/Services/ToolResultCap.cs | 89 ++++++-- ClaudeRevit/Services/ToolSearchLogic.cs | 36 ++- ClaudeRevit/Tools/AuditSpdsSchedule.cs | 2 +- ClaudeRevit/Tools/CalculateWeight.cs | 9 +- ClaudeRevit/Tools/CleanModel.cs | 116 +++++----- ClaudeRevit/Tools/CreateBeam.cs | 8 +- ClaudeRevit/Tools/CreateFloorType.cs | 11 +- ClaudeRevit/Tools/CreateIsolatedFoundation.cs | 2 +- ClaudeRevit/Tools/CreateWallType.cs | 11 +- ClaudeRevit/Tools/DeriveParameters.cs | 22 +- ClaudeRevit/Tools/FindTools.cs | 3 +- ClaudeRevit/Tools/GenerateDiagnosticReport.cs | 2 +- ClaudeRevit/Tools/GetElementsOnLevel.cs | 2 +- ClaudeRevit/Tools/GetFullResult.cs | 8 +- ClaudeRevit/Tools/GetProjectCatalog.cs | 35 ++- ClaudeRevit/Tools/LiveSpds.cs | 21 +- ClaudeRevit/Tools/ModelChangeCapture.cs | 48 +++- ClaudeRevit/Tools/NameResolve.cs | 19 +- ClaudeRevit/Tools/RunBatch.cs | 8 +- ClaudeRevit/Tools/RunPython.cs | 4 +- ClaudeRevit/Tools/ToolCatalog.cs | 4 +- ClaudeRevit/Tools/ToolDispatcher.cs | 59 +++-- ClaudeRevit/Tools/ToolInput.cs | 7 + ClaudeRevit/Tools/TypeChangePreservation.cs | 3 +- ClaudeRevit/Tools/UndoLast.cs | 24 +- ClaudeRevit/Tools/Units.cs | 3 + ClaudeRevit/Tools/UnloadFamily.cs | 17 +- ClaudeRevit/Tools/WritePlans.cs | 1 + ClaudeRevit/UI/MarkdownProps.cs | 4 +- README.md | 4 + README.ru.md | 4 + docs/audit-fixes-2026-10-20.md | 63 ++++++ docs/benchmark.md | 7 +- 56 files changed, 957 insertions(+), 246 deletions(-) create mode 100644 ClaudeRevit.Tests/AuditSafetyTests.cs create mode 100644 ClaudeRevit/Services/CatalogPage.cs create mode 100644 docs/audit-fixes-2026-10-20.md diff --git a/ClaudeRevit.CliTests/ClaudeRevit.CliTests.csproj b/ClaudeRevit.CliTests/ClaudeRevit.CliTests.csproj index 27f7b9b..71a4f85 100644 --- a/ClaudeRevit.CliTests/ClaudeRevit.CliTests.csproj +++ b/ClaudeRevit.CliTests/ClaudeRevit.CliTests.csproj @@ -5,6 +5,7 @@ codexMajor + diff --git a/ClaudeRevit.NativeTests/README.md b/ClaudeRevit.NativeTests/README.md index 4ab3eb3..708a012 100644 --- a/ClaudeRevit.NativeTests/README.md +++ b/ClaudeRevit.NativeTests/README.md @@ -77,3 +77,6 @@ catalogue description or the presence of a fixture generator cannot do so. See [executed October 2026 cases](../docs/native-acceptance-2026-10.md) for results and the [BIMStarter findings](../docs/bimstarter-reference-cases.md) for expected minimum-count behavior and remaining visibility checks. + + +`review_fixes` also contains the four independent October 20 audit cases (total multilayer width, derived Length units, material density and placed zero-area rooms). See [audit validation notes](../docs/audit-fixes-2026-10-20.md); their native execution is pending and is not implied by a successful build. diff --git a/ClaudeRevit.NativeTests/ReviewFixtures.cs b/ClaudeRevit.NativeTests/ReviewFixtures.cs index 08d680f..0cee0ec 100644 --- a/ClaudeRevit.NativeTests/ReviewFixtures.cs +++ b/ClaudeRevit.NativeTests/ReviewFixtures.cs @@ -24,9 +24,87 @@ void Check(string name, Func body) Check("change_element_type_actually_changes_type", () => WithProject(app, ChangeTypeSticks)); Check("execute_csharp_result_serialization_never_throws", () => WithProject(app, ScriptResultsSerialize)); Check("set_parameter_reaches_material_named_like_a_type", () => WithProject(app, MaterialBeatsSameNamedType)); + Check("audit_total_wall_thickness", () => WithProject(app, TotalThickness)); + Check("audit_derived_length_internal_units", () => WithProject(app, DerivedLength)); + Check("audit_material_density_7850", () => WithProject(app, MaterialDensity)); + Check("audit_placed_unbounded_room_survives_cleanup", () => WithProject(app, PlacedRoom)); return new { passed = checks.Count(c => (bool)c.GetType().GetProperty("ok")!.GetValue(c)!), total = checks.Count, checks }; } + private static object TotalThickness(UIApplication app, Document doc) + { + using var tx = new Transaction(doc, "QA total width"); tx.Start(); + var original = new FilteredElementCollector(doc).OfClass(typeof(WallType)).Cast().First(t => t.Kind == WallKind.Basic); + var basis = (WallType)original.Duplicate("QA_Multilayer_Base"); + var structure = CompoundStructure.CreateSimpleCompoundStructure(new List + { + new(20 / 304.8, MaterialFunctionAssignment.Finish1, ElementId.InvalidElementId), + new(200 / 304.8, MaterialFunctionAssignment.Structure, ElementId.InvalidElementId), + new(30 / 304.8, MaterialFunctionAssignment.Finish2, ElementId.InvalidElementId) + }); + structure.SetNumberOfShellLayers(ShellLayerType.Exterior, 1); + structure.SetNumberOfShellLayers(ShellLayerType.Interior, 1); + basis.SetCompoundStructure(structure); + using var result = JsonDocument.Parse(new CreateWallType().Execute(SyntheticFixtures.Args(new { name = "QA_Total_300", based_on = basis.Name, thickness_mm = 300 }), app)); + var created = (WallType)doc.GetElement(new ElementId(result.RootElement.GetProperty("id").GetInt64())); + doc.Regenerate(); + var actual = created.Width * 304.8; + if (Math.Abs(actual - 300) > .001) throw new InvalidOperationException($"Expected total 300 mm, got {actual}."); + tx.RollBack(); return new { expected_mm = 300, actual_mm = actual }; + } + + private static object DerivedLength(UIApplication app, Document doc) + { + using var tx = new Transaction(doc, "QA derive length"); tx.Start(); + var (wall, _, _) = WallWithTwoTypes(doc, "QA_Derive_Type"); + doc.Regenerate(); + var parameter = wall.get_Parameter(BuiltInParameter.WALL_BASE_OFFSET); + var expectedFeet = ((LocationCurve)wall.Location).Curve.Length; + using var result = JsonDocument.Parse(new DeriveParameters().Execute(SyntheticFixtures.Args(new + { + element_ids = new[] { wall.Id.Value }, parameter_name = parameter.Definition.Name, source = "length_mm", round = 6 + }), app)); + doc.Regenerate(); + var actualFeet = parameter.AsDouble(); + if (Math.Abs(actualFeet - expectedFeet) > 1e-7) throw new InvalidOperationException($"Expected {expectedFeet} ft, got {actualFeet} ft."); + tx.RollBack(); return new { expected_feet = expectedFeet, actual_feet = actualFeet }; + } + + private static object MaterialDensity(UIApplication app, Document doc) + { + using var tx = new Transaction(doc, "QA density"); tx.Start(); + var materialId = Material.Create(doc, "QA_Steel_7850"); + var material = (Material)doc.GetElement(materialId); + var asset = new StructuralAsset("QA_Steel_Asset", StructuralAssetClass.Metal) { Density = 7850 * Math.Pow(.3048, 3) }; + material.SetMaterialAspectByPropertySet(MaterialAspect.Structural, PropertySetElement.Create(doc, asset).Id); + var side = 1 / .3048; + var loop = CurveLoop.Create(new List + { + Line.CreateBound(XYZ.Zero, new XYZ(side, 0, 0)), Line.CreateBound(new XYZ(side, 0, 0), new XYZ(side, side, 0)), + Line.CreateBound(new XYZ(side, side, 0), new XYZ(0, side, 0)), Line.CreateBound(new XYZ(0, side, 0), XYZ.Zero) + }); + var solid = GeometryCreationUtilities.CreateExtrusionGeometry(new List { loop }, XYZ.BasisZ, side, + new SolidOptions(materialId, ElementId.InvalidElementId)); + var shape = DirectShape.CreateElement(doc, new ElementId(BuiltInCategory.OST_GenericModel)); shape.SetShape(new GeometryObject[] { solid }); + doc.Regenerate(); + using var result = JsonDocument.Parse(new CalculateWeight().Execute(SyntheticFixtures.Args(new { element_ids = new[] { shape.Id.Value } }), app)); + var mass = result.RootElement.GetProperty("total_kg").GetDouble(); + if (Math.Abs(mass - 7850) > .02) throw new InvalidOperationException($"A 1 m3 steel solid must weigh 7850 kg, got {mass}."); + tx.RollBack(); return new { volume_m3 = 1, expected_kg = 7850, actual_kg = mass }; + } + + private static object PlacedRoom(UIApplication app, Document doc) + { + using var tx = new Transaction(doc, "QA unbounded room"); tx.Start(); + var level = new FilteredElementCollector(doc).OfClass(typeof(Level)).Cast().First(); + var room = doc.Create.NewRoom(level, new UV(1, 1)); doc.Regenerate(); + if (room.Location == null || room.Area > 0) throw new InvalidOperationException("Fixture must be placed and unbounded."); + var id = room.Id; + var result = new CleanModel().Execute(SyntheticFixtures.Args(new { targets = new[] { "unplaced_rooms" }, apply = true }), app); + if (doc.GetElement(id) == null) throw new InvalidOperationException("Cleanup deleted a placed room."); + tx.RollBack(); return new { preserved = true, result }; + } + private static object WithProject(UIApplication app, Func body) { Document? project = null; diff --git a/ClaudeRevit.PaneTests/ClaudeRevit.PaneTests.csproj b/ClaudeRevit.PaneTests/ClaudeRevit.PaneTests.csproj index c8e0aae..2ed0eba 100644 --- a/ClaudeRevit.PaneTests/ClaudeRevit.PaneTests.csproj +++ b/ClaudeRevit.PaneTests/ClaudeRevit.PaneTests.csproj @@ -8,9 +8,12 @@ Major + + + diff --git a/ClaudeRevit.PaneTests/Program.cs b/ClaudeRevit.PaneTests/Program.cs index 5a1e7d0..4af8346 100644 --- a/ClaudeRevit.PaneTests/Program.cs +++ b/ClaudeRevit.PaneTests/Program.cs @@ -9,6 +9,21 @@ internal static class Program { + private static async Task CheckStreamingMarkdown() + { + var target = new TextBlock(); MarkdownProps.SetMarkdown(target, "first"); + bool renderedWhileStreaming = false; + for (var i = 0; i < 40; i++) + { + MarkdownProps.SetMarkdown(target, "delta " + i); + await Task.Delay(15); + if (i > 10 && string.Concat(target.Inlines.OfType().Select(r => r.Text)).StartsWith("delta", StringComparison.Ordinal)) renderedWhileStreaming = true; + } + Check(renderedWhileStreaming, "Continuous deltas starved markdown rendering"); + await Task.Delay(150); + Check(string.Concat(target.Inlines.OfType().Select(r => r.Text)) == "delta 39", "Final streaming delta was not rendered"); + } + [STAThread] private static void Main(string[] args) { @@ -19,6 +34,7 @@ private static void Main(string[] args) { DocumentHarness.Change("a", false); await CheckIndependentChat(); + await CheckStreamingMarkdown(); var pane = new ChatPaneView(); CheckRepeatedDocumentWrappers(pane); DocumentHarness.Change("a", false); @@ -73,6 +89,8 @@ private static void Main(string[] args) var noisePath = Path.Combine(attachmentRoot, "noise.png"); using (var output = File.Create(noisePath)) noisyEncoder.Save(output); Check(new FileInfo(noisePath).Length > 5_000_000, "Dense image fixture did not exercise the image size cap"); + var encoded = AttachmentImage.EncodePng(noisePath); + Check(Convert.ToBase64String(encoded).Length <= ChatBlockOrder.MaxInlineImageBase64, "Encoded image exceeds actual chat transport budget"); var smallNoise = await Task.Run(() => AttachmentImage.EncodePng(noisePath)); Check(smallNoise.Length <= 5_000_000, "Dense attachment PNG exceeded the native image content cap"); var attach = typeof(ChatPaneView).GetMethod("AttachFilesAsync", BindingFlags.NonPublic | BindingFlags.Instance)!; diff --git a/ClaudeRevit.PaneTests/Stubs.cs b/ClaudeRevit.PaneTests/Stubs.cs index ae58479..732187b 100644 --- a/ClaudeRevit.PaneTests/Stubs.cs +++ b/ClaudeRevit.PaneTests/Stubs.cs @@ -131,10 +131,5 @@ public static class ElementIdLinker public static void SetText(DependencyObject target, string value) => target.SetValue(TextProperty, value); public static string GetText(DependencyObject target) => (string)target.GetValue(TextProperty); } - public static class MarkdownProps - { - public static readonly DependencyProperty MarkdownProperty = DependencyProperty.RegisterAttached("Markdown", typeof(string), typeof(MarkdownProps)); - public static void SetMarkdown(DependencyObject target, string value) => target.SetValue(MarkdownProperty, value); - public static string GetMarkdown(DependencyObject target) => (string)target.GetValue(MarkdownProperty); - } + } diff --git a/ClaudeRevit.Tests/AuditSafetyTests.cs b/ClaudeRevit.Tests/AuditSafetyTests.cs new file mode 100644 index 0000000..7b1f9b3 --- /dev/null +++ b/ClaudeRevit.Tests/AuditSafetyTests.cs @@ -0,0 +1,210 @@ +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using ClaudeRevit.Services; +using ClaudeRevit.Tools; +using ModPlus.Headless; +using Xunit; + +namespace ClaudeRevit.Tests; + +public class AuditSafetyTests +{ + [Theory] + [InlineData("{\"succeeded\":false,\"traceback\":\"boom\"}")] + [InlineData("{\"error\":\"Python host error\"}")] + [InlineData("{\"ok\":true,\"succeeded\":false}")] + [InlineData("{\"ok\":false}")] + [InlineData("{\"ok\":0,\"failed\":2}")] + [InlineData("{\"status\":\"failed\"}")] + public void LegacyFailuresStayFailuresAcrossCompletionAndCap(string raw) + { + var source = JsonNode.Parse(raw)!.AsObject(); source["payload"] = new string('Я', 45_000); + var result = ToolResultCap.Apply(ToolResult.Complete(source.ToJsonString()), "test"); + Assert.False(ToolResult.LooksSuccessful(result)); + using var parsed = JsonDocument.Parse(result); + Assert.False(ToolResult.LooksSuccessful(parsed.RootElement.GetRawText())); + Assert.True(result.Length <= ToolResultCap.MaxChars); + } + + [Fact] + public void LargePreviewKeepsControlAndErrorFields() + { + var source = new JsonObject { ["ok"] = false, ["preview"] = true, ["committed"] = false, + ["rolled_back"] = true, ["plan_token"] = "token", ["error"] = "failed", ["payload"] = new string('x', 45_000) }; + using var parsed = JsonDocument.Parse(ToolResultCap.Apply(source.ToJsonString(), "preview")); + var r = parsed.RootElement; + Assert.True(r.GetProperty("preview").GetBoolean()); + Assert.True(r.GetProperty("rolled_back").GetBoolean()); + Assert.False(r.GetProperty("committed").GetBoolean()); + Assert.Equal("token", r.GetProperty("plan_token").GetString()); + Assert.Equal("failed", r.GetProperty("error").GetString()); + } + + [Fact] + public void LargeDiagnosticsStayBoundedAndKeepArrayContracts() + { + var source = new JsonObject { ["ok"] = false, ["error"] = new string('\0', 40_000), + ["warnings"] = new JsonArray(new string('Я', 40_000)), ["errors"] = new JsonArray(new string('\n', 40_000)) }; + var result = ToolResultCap.Apply(source.ToJsonString(), "failure"); + Assert.True(result.Length <= ToolResultCap.MaxChars); + using var parsed = JsonDocument.Parse(result); + Assert.Equal(JsonValueKind.Array, parsed.RootElement.GetProperty("errors").ValueKind); + Assert.Equal(JsonValueKind.Array, parsed.RootElement.GetProperty("warnings").ValueKind); + Assert.False(parsed.RootElement.GetProperty("ok").GetBoolean()); + } + + [Theory] + [InlineData("Я\"\\\n")] + [InlineData("😀")] + public void PagesRoundTripBeyondOldThirtyThousandLimit(string pattern) + { + var text = string.Concat(Enumerable.Repeat(pattern, 40_000)); + var restored = new StringBuilder(); var offset = 0; + do + { + var page = ToolResultCap.Page("fixture", text, offset, 30_000); + // Pages must survive the production dispatcher cap unchanged. + Assert.Equal(page, ToolResultCap.Apply(page, "get_full_result")); + using var parsed = JsonDocument.Parse(page); var r = parsed.RootElement; + restored.Append(r.GetProperty("content").GetString()); + if (!r.GetProperty("has_more").GetBoolean()) break; + var next = r.GetProperty("next_offset").GetInt32(); Assert.True(next > offset); offset = next; + } while (true); + Assert.Equal(text, restored.ToString()); + } + + [Theory] + [InlineData(-1, 100)] + [InlineData(4, 100)] + [InlineData(0, 0)] + [InlineData(0, 30001)] + public void InvalidPageBoundsFailExplicitly(int offset, int limit) + => Assert.Throws(() => ToolResultCap.Page("x", "abc", offset, limit)); + + [Fact] + public void UnicodePageRequestsCannotStallOrSplitPairs() + { + Assert.Throws(() => ToolResultCap.Page("x", "😀", 0, 1)); + Assert.Throws(() => ToolResultCap.Page("x", "😀", 1, 2)); + } + + [Fact] + public void AgingRetainsFailureAndRollbackAsOneJsonValue() + { + var raw = new JsonObject { ["ok"] = false, ["preview"] = true, ["committed"] = false, + ["payload"] = new string('x', 2000) }.ToJsonString(); + var turn = new ApiTurn { Blocks = { new ChatToolResultBlock("test", raw, true) } }; + Assert.Equal(1, ToolResultAging.AgeAll(new[] { turn })); + var result = Assert.IsType(turn.Blocks[0]); + using var parsed = JsonDocument.Parse(result.Content); + Assert.False(parsed.RootElement.GetProperty("ok").GetBoolean()); + Assert.True(parsed.RootElement.GetProperty("preview").GetBoolean()); + Assert.False(parsed.RootElement.GetProperty("committed").GetBoolean()); + Assert.Equal(0, ToolResultAging.AgeAll(new[] { turn })); + } + + [Fact] + public void ArchiveRetainsMoreThanTheFormerHundredThousandCharacters() + { + var text = new string('x', 120_000) + "THE_END"; + var id = "test_" + Guid.NewGuid().ToString("N"); ToolResultArchive.Record(id, text); + Assert.Equal(text, ToolResultArchive.Lookup(id)); + } + + [Fact] + public void PreviewAndPartialStatusesAreDistinctFromCommittedSuccess() + { + using var preview = JsonDocument.Parse(ToolResult.Execution("{\"ok\":true,\"preview\":true}", true, false)); + Assert.False(preview.RootElement.GetProperty("committed").GetBoolean()); + Assert.Equal("preview", preview.RootElement.GetProperty("execution_status").GetString()); + using var partial = JsonDocument.Parse(ToolResult.Complete("{\"ok\":3,\"failed\":1}")); + Assert.Equal(3, partial.RootElement.GetProperty("ok").GetInt32()); + Assert.Equal("partial", partial.RootElement.GetProperty("status").GetString()); + } + + [Fact] + public void CyrillicFitsHeadlessBudgetWithoutAsciiExpansion() + { + var raw = new JsonObject { ["ok"] = true, ["text"] = new string('Я', 40_000) }.ToJsonString(ToolResult.Options); + var result = HeadlessResultLimits.Compact(raw); + Assert.True(result.Length <= 30_000); + Assert.DoesNotContain("\\u042F", result); + Assert.Contains(new string('Я', 9000), result); + } + + [Theory] + [InlineData(7850)] + [InlineData(2500)] + [InlineData(20)] + public void DensityUsesKilogramsRatherThanPounds(double expected) + { + // Independently defined cubic-foot conversion; do not reuse Units constants. + var internalDensity = expected * Math.Pow(0.3048, 3); + Assert.Equal(expected, Units.DensityToKgM3(internalDensity), 8); + } + + [Theory] + [InlineData("1.0", 1)] + [InlineData("\"1\"", 1)] + [InlineData("0", 0)] + public void IntegralJsonNumbersUseTheValidatedReader(string raw, int expected) + { + using var parsed = JsonDocument.Parse(raw); + Assert.Equal(expected, ToolInput.NonnegativeInteger(parsed.RootElement, "steps")); + } + + [Theory] + [InlineData("1.5")] + [InlineData("-1")] + [InlineData("null")] + [InlineData("true")] + public void InvalidIntegersAreValidationErrors(string raw) + { + using var parsed = JsonDocument.Parse(raw); + Assert.Throws(() => ToolInput.NonnegativeInteger(parsed.RootElement, "steps")); + } + + [Fact] + public void AxisGridAndSectionSizeDoNotLoadRebarOrFamilyAuthoring() + => Assert.Empty(ToolSearchLogic.Prewarm("Создай колонны сечением 400х400 по сетке осей")); + + [Fact] + public void ExactDiscoveryLoadsOneSchemaAndVagueVerbLoadsNone() + { + var catalog = Enumerable.Range(0, 50).Select(i => new ToolSearchLogic.ToolInfo("create_rebar_" + i, "Creates rebar.", "Rebar", false)).ToArray(); + Assert.Single(ToolSearchLogic.Search(catalog, "create_rebar_24").ToolNames); + Assert.Empty(ToolSearchLogic.Search(catalog, "create").ToolNames); + Assert.InRange(ToolSearchLogic.Search(catalog, "rebar").ToolNames.Count, 1, 12); + } + + [Fact] + public void CatalogPagingKeepsCountsAndDoesNotDiscardLateEntries() + { + using var catalog = JsonDocument.Parse("{\"materials\":[\"A\",\"B\",\"C\"],\"levels\":[1,2]}"); + var page = CatalogPage.Select(catalog.RootElement, new[] { "materials" }, null, 1, 1); + Assert.False(page["catalog"]!.AsObject().ContainsKey("levels")); + Assert.Equal("B", page["catalog"]!["materials"]![0]!.GetValue()); + Assert.Equal(3, page["paging"]!["materials"]!["total"]!.GetValue()); + Assert.Equal(2, page["paging"]!["materials"]!["next_offset"]!.GetValue()); + } + + [Fact] + public void CatalogFiltersCategoriesAndPreservesQualifiedTypeNames() + { + using var catalog = JsonDocument.Parse("{\"family_types_by_category\":{\"Columns\":[\"Family A: Type 1\"],\"Doors\":[\"Door: Type 1\"]}}"); + var page = CatalogPage.Select(catalog.RootElement, null, "Family A", 0, 40, "Columns"); + var categories = page["catalog"]!["family_types_by_category"]!.AsObject(); + Assert.Single(categories); Assert.Equal("Family A: Type 1", categories["Columns"]![0]!.GetValue()); + } + + [Fact] + public void PhaseMeasurementsAreScopedAndDisposeOnlyOnce() + { + using var task = TaskJournal.Start("audit-test"); + var phase = TaskJournal.MeasurePhase("verification"); phase.Dispose(); phase.Dispose(); + var metrics = TaskJournal.ReadPhases(TaskJournal.CurrentId); + Assert.Equal(1, metrics["verification"].Count); + Assert.True(metrics["verification"].Seconds >= 0); + } +} diff --git a/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs b/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs index 87f6371..6ca5532 100644 --- a/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs +++ b/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs @@ -15,6 +15,7 @@ public sealed class ChatService { public ChatService(bool ephemeral) { } public bool SubscriptionMode { get; set; } + public int? MaxToolRoundsOverride { get; set; } public Action? OnRound { get; set; } public Action? OnStatus { get; set; } public TurnTimings Timings { get; } = new() { ModelAndToolsSeconds = 0.01, ToolWaitSeconds = 0.004, QueueSeconds = 0.002, RevitExecutionSeconds = 0.001 }; diff --git a/ClaudeRevit.Tests/BenchmarkRunnerTests.cs b/ClaudeRevit.Tests/BenchmarkRunnerTests.cs index 933d85c..446c65a 100644 --- a/ClaudeRevit.Tests/BenchmarkRunnerTests.cs +++ b/ClaudeRevit.Tests/BenchmarkRunnerTests.cs @@ -34,7 +34,7 @@ public async Task SkipsWrongDocumentAndWritesQualitySpeedAndFormulaWithoutInvent using var first = JsonDocument.Parse(lines[0]); using var second = JsonDocument.Parse(lines[1]); Assert.Equal(BenchmarkScoring.Version, first.RootElement.GetProperty("scoring_version").GetString()); Assert.Equal(30, first.RootElement.GetProperty("reference_seconds").GetInt32()); - Assert.Equal("api_progressive_v1", first.RootElement.GetProperty("execution_profile").GetString()); + Assert.Equal("api_selective_v2", first.RootElement.GetProperty("execution_profile").GetString()); Assert.Equal(0.001, first.RootElement.GetProperty("phase_seconds").GetProperty("revit_execution_sum").GetDouble()); Assert.True(second.RootElement.GetProperty("skipped").GetBoolean()); Assert.False(second.RootElement.GetProperty("graded").GetBoolean()); diff --git a/ClaudeRevit.Tests/ClaudeRevit.Tests.csproj b/ClaudeRevit.Tests/ClaudeRevit.Tests.csproj index f4a137b..12fa87e 100644 --- a/ClaudeRevit.Tests/ClaudeRevit.Tests.csproj +++ b/ClaudeRevit.Tests/ClaudeRevit.Tests.csproj @@ -12,6 +12,7 @@ + diff --git a/ClaudeRevit.Tests/ToolResultAgingTests.cs b/ClaudeRevit.Tests/ToolResultAgingTests.cs index 0bdb72f..7613178 100644 --- a/ClaudeRevit.Tests/ToolResultAgingTests.cs +++ b/ClaudeRevit.Tests/ToolResultAgingTests.cs @@ -26,7 +26,7 @@ private static ApiTurn Assistant() => }; private static bool IsAged(ApiTurn t) => - t.Blocks.OfType().Any(b => b.Content.Contains("[aged")); + t.Blocks.OfType().Any(b => b.Content.Contains("\"aged\":true")); [Fact] public void AgeOlderTurns_keeps_the_last_two_result_turns_full() diff --git a/ClaudeRevit/Services/AttachmentImage.cs b/ClaudeRevit/Services/AttachmentImage.cs index 37f97ef..0a00efa 100644 --- a/ClaudeRevit/Services/AttachmentImage.cs +++ b/ClaudeRevit/Services/AttachmentImage.cs @@ -23,7 +23,9 @@ public static byte[] EncodePng(string path) { var encoder = new PngBitmapEncoder(); encoder.Frames.Add(BitmapFrame.Create(source)); using var output = new MemoryStream(); encoder.Save(output); - if (output.Length <= 5_000_000) return output.ToArray(); + // Base64 adds 4/3 overhead. Match the actual chat transport budget. + if (4 * ((output.Length + 2) / 3) <= ChatBlockOrder.MaxInlineImageBase64) return output.ToArray(); + if (source.PixelWidth <= 1 || source.PixelHeight <= 1) throw new InvalidDataException("Cannot fit image into the chat transport budget."); source = new TransformedBitmap(source, new ScaleTransform(0.75, 0.75)); source.Freeze(); } } diff --git a/ClaudeRevit/Services/BenchmarkRunner.cs b/ClaudeRevit/Services/BenchmarkRunner.cs index 213e215..2071126 100644 --- a/ClaudeRevit/Services/BenchmarkRunner.cs +++ b/ClaudeRevit/Services/BenchmarkRunner.cs @@ -95,7 +95,7 @@ public static async Task RunAsync( before = await StatsAsync(task, false, documentKey, ct); } if (DocumentSessions.CurrentDocumentKey != documentKey) throw new InvalidOperationException("Active document changed before the benchmark task."); - var chat = new ChatService(ephemeral: true) { SubscriptionMode = false }; + var chat = new ChatService(ephemeral: true) { SubscriptionMode = false, MaxToolRoundsOverride = maxRoundsPerTask > 0 ? maxRoundsPerTask : int.MaxValue }; chat.OnStatus = phase => onStatus($"{task.Id} · {task.Title} · {phase}"); var conversation = new ObservableCollection { new() { Role = "user", Text = task.Prompt + "\nBenchmark: keep the current document active; do not save or close it, open another document, or change application/global settings. " + @@ -295,7 +295,7 @@ private static void Append(BenchmarkResult r, BenchmarkExecution execution, Benc scoring_formula = "quality * (0.8 + 0.2 * min(1, reference_seconds / seconds))", reference_seconds = r.ReferenceSeconds, reference_source = r.ReferenceSource, timing_scope = "modeller_and_tools_excluding_probe_judge_reset", - execution_profile = execution.Backend == "api" ? "api_progressive_v1" : "compact_mcp_v1", + execution_profile = execution.Backend == "api" ? "api_selective_v2" : "compact_mcp_v2", task_suite_version = "v3.8.4", objective_checks = r.Objective, seed_fingerprint = r.SeedFingerprint, diff --git a/ClaudeRevit/Services/CatalogPage.cs b/ClaudeRevit/Services/CatalogPage.cs new file mode 100644 index 0000000..9d5166b --- /dev/null +++ b/ClaudeRevit/Services/CatalogPage.cs @@ -0,0 +1,42 @@ +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace ClaudeRevit.Services; + +// Preserve catalog field shapes, while exposing real counts and continuation offsets +// for every bounded list. The cached source itself is never cut down to a preview. +public static class CatalogPage +{ + public static JsonObject Select(JsonElement catalog, string[]? sections, string? query, int offset, int limit, string? familyCategory = null) + { + if (offset < 0 || limit is < 1 or > 300) throw new ArgumentOutOfRangeException(nameof(limit)); + var source = JsonNode.Parse(catalog.GetRawText())!.AsObject(); + var paging = new JsonObject(); + JsonNode? Visit(JsonNode? node, string path) + { + if (node is JsonArray array) + { + var filtered = array.Where(item => string.IsNullOrWhiteSpace(query) || + (item?.ToJsonString(ToolResult.Options).Contains(query, StringComparison.OrdinalIgnoreCase) ?? false)).ToArray(); + var items = filtered.Skip(offset).Take(limit).ToArray(); + paging[path] = new JsonObject { ["total"] = filtered.Length, ["offset"] = offset, + ["next_offset"] = offset + items.Length < filtered.Length ? JsonValue.Create(offset + items.Length) : null }; + return new JsonArray(items.Select(n => n?.DeepClone()).ToArray()); + } + if (node is JsonObject obj) + { + var result = new JsonObject(); + foreach (var (key, value) in obj) + if (path != "family_types_by_category" || string.IsNullOrWhiteSpace(familyCategory) || key.Contains(familyCategory, StringComparison.OrdinalIgnoreCase)) + result[key] = Visit(value, path + "." + key); + return result; + } + return node?.DeepClone(); + } + var selected = new JsonObject(); + foreach (var (key, value) in source) + if (sections == null || sections.Contains(key, StringComparer.Ordinal)) selected[key] = Visit(value, key); + if (sections?.Any(key => !source.ContainsKey(key)) == true) throw new ArgumentException("Unknown catalog section.", nameof(sections)); + return new JsonObject { ["catalog"] = selected, ["paging"] = paging }; + } +} diff --git a/ClaudeRevit/Services/ChatService.cs b/ClaudeRevit/Services/ChatService.cs index da8402f..4e87acf 100644 --- a/ClaudeRevit/Services/ChatService.cs +++ b/ClaudeRevit/Services/ChatService.cs @@ -208,16 +208,15 @@ public sealed record TaskMetrics( private int _execCsharpOk; private bool _promoteNudged; - // Progressive tool loading: only the core toolset is sent every request; a specialised group - // is added here (in reveal order) once the user's message points at it or the model calls - // find_tools. Session-scoped — cleared on ClearHistory. See ToolCatalog for the mechanism. - private readonly List _revealedCategories = new(); + // Core schemas plus explicitly selected tool names, in stable append order. + // Search reveals at most 12 names; clearing history resets discovery. + public int? MaxToolRoundsOverride { get; set; } + private readonly List _revealedTools = new(); - private bool Reveal(string category) + private static IEnumerable SearchableTools() { - if (_revealedCategories.Contains(category)) return false; - _revealedCategories.Add(category); - return true; + var disabled = new HashSet(SettingsStore.DisabledToolGroups, StringComparer.OrdinalIgnoreCase); + return ToolRegistry.Instance.All.Where(t => !ToolCatalog.IsModPlusAlias(t) && ToolPolicy.IsEnabled(t, disabled)); } public ChatService() : this(false) { } @@ -329,7 +328,7 @@ public void SwitchWorkspace(bool preserveHistory = false, DocumentSessions.Snaps })) / 4; _execCsharpOk = 0; _promoteNudged = false; - _revealedCategories.Clear(); + _revealedTools.Clear(); } private static string? ReadSession(string path) { try { return File.Exists(path) ? File.ReadAllText(path).Trim() : null; } catch { return null; } } @@ -385,7 +384,7 @@ public void ClearHistory() _lastPromptTokens = 0; _execCsharpOk = 0; _promoteNudged = false; - _revealedCategories.Clear(); + _revealedTools.Clear(); _claudeCodeSessionId = null; PersistClaudeCodeSession(); _codexSessionId = null; @@ -541,7 +540,7 @@ void Append(string piece) } OnStatus?.Invoke("running model · waiting for first tool/result…"); res = await CodexBackend.RunAsync( - McpServer.DrivingRules + "\n\n" + contextedPrompt, + (string.IsNullOrEmpty(_codexSessionId) ? McpServer.DrivingRules + "\n\n" : "") + contextedPrompt, ClientDirectory, channel.Url(McpServer.Url), SettingsStore.McpToken, _codexSessionId, onText: Append, onTool: _ => { toolCount++; OnRound?.Invoke(toolCount, toolCount); }, @@ -750,7 +749,7 @@ public Task SendAsync( TaskJournal.Append(new { kind="task_completed",utc=DateTime.UtcNow,task_id=taskId,document_key=documentKey,wall_seconds=watch.Elapsed.TotalSeconds, model=LastTask?.Model,rounds=LastTask?.Rounds,input_tokens=LastTask?.InputTokens,output_tokens=LastTask?.OutputTokens, usage_scope=LastTask?.UsageScope,cached_input_tokens=LastTask?.CachedInputTokens,reasoning_tokens=LastTask?.ReasoningTokens,worker_seconds=TaskJournal.ReadWorker(taskId), - timings=Timings,cancelled=ct.IsCancellationRequested,error=LastRunError }); + timings=Timings,phase_metrics=TaskJournal.ReadPhases(taskId),timing_scope="Phase sums can overlap; provider request includes network/server/streaming, not pure model reasoning.",journal_write_error=TaskJournal.LastWriteError,cancelled=ct.IsCancellationRequested,error=LastRunError }); } }); } @@ -818,7 +817,9 @@ private async Task SendCoreAsync( // ("add rebar", "make a section") so those tools are present on the first round and the // model doesn't have to spend a find_tools round first. It can still find_tools for the // rest. Purely additive to what's already revealed this session. - foreach (var cat in ToolCatalog.PrewarmCategories(lastUser)) Reveal(cat); + if (ToolCatalog.PrewarmCategories(lastUser).Count > 0) + foreach (var name in ToolCatalog.Search(SearchableTools(), lastUser).ToolNames) + if (!_revealedTools.Contains(name)) _revealedTools.Add(name); var appendOnly = !alt && ClaudeModels.AppendOnlyHistory(model); if (!appendOnly) await CompactIfNeededAsync(conversation, ui, alt, ct); @@ -903,7 +904,7 @@ List ClaudeTools() var turnLabel = "Claude: " + Truncate(lastUser, 60); // Read once per prompt so a mid-turn Settings change doesn't shift the cap. - var maxIterations = SettingsStore.MaxToolRounds; + var maxIterations = MaxToolRoundsOverride ?? SettingsStore.MaxToolRounds; // Auto-escalation to a stronger model: keep a cheap/fast model for routine work, switch // to a stronger one when the task looks hard up front, or once the fast model starts @@ -995,6 +996,7 @@ await ui.InvokeAsync(() => conversation.Add(new ChatMessage BackendTurn turn; try { + using var providerPhase = TaskJournal.MeasurePhase("provider_request"); turn = alt ? await StreamAltTurnAsync(altSystem!, altTools!, dynamicContext, conversation, ui, ct, altModelForTurn) : await StreamAnthropicTurnAsync(claudeModelForTurn, systemBlocks!, toolDefs!, dynamicContext, conversation, ui, ct, advisorMode); @@ -1175,9 +1177,10 @@ await ui.InvokeAsync(() => conversation.Add(new ChatMessage { var query = inp.TryGetValue("query", out var qEl) && qEl.ValueKind == JsonValueKind.String ? qEl.GetString() ?? "" : ""; - var search = ToolCatalog.Search(ToolRegistry.Instance.All.Where(t => !ToolCatalog.IsModPlusAlias(t)), query); + var search = ToolCatalog.Search(SearchableTools(), query); var revealedAny = false; - foreach (var c in search.Categories) revealedAny |= Reveal(c); + foreach (var toolName in search.ToolNames) + if (!_revealedTools.Contains(toolName)) { _revealedTools.Add(toolName); revealedAny = true; } if (revealedAny) toolSetChanged = true; await ui.InvokeAsync(() => conversation.Add(new ChatMessage @@ -1186,7 +1189,7 @@ await ui.InvokeAsync(() => conversation.Add(new ChatMessage ToolName = name, Text = FormatInput(inp) + "\n→ " + Truncate(search.Message, 400) })); - resultTurn.Blocks.Add(new ChatToolResultBlock(use.Id, search.Message, false)); + resultTurn.Blocks.Add(new ChatToolResultBlock(use.Id, ToolResult.Complete(Services.Json.Serialize(new { tools = search.ToolNames, message = search.Message })), false)); continue; } @@ -1231,8 +1234,9 @@ await ui.InvokeAsync(() => ? await pending : await ToolDispatcher.Instance.ExecuteAsync(name, inp, ct, documentKey); if(name is "open_family_editor" or "reload_family_into_document") documentKey=DocumentSessions.CurrentDocumentKey; + isError = !ToolResult.LooksSuccessful(content); var display = FormatInput(inp) + "\n→ " + Truncate(FormatResult(content), 400); - await ui.InvokeAsync(() => toolMsg.Text = display); + await ui.InvokeAsync(() => { toolMsg.Text = display; toolMsg.IsError = isError; }); } catch (OperationCanceledException) { @@ -1263,13 +1267,9 @@ await ui.InvokeAsync(() => _execCsharpOk++; if (!_promoteNudged && _execCsharpOk >= 2) { - content += "\n\n[SYSTEM: you have run execute_csharp successfully " + - _execCsharpOk + " times this session. If any of these is essentially " + - "the SAME kind of operation as one you already did (a repeatable " + - "pattern, not a genuine one-off), generalize it now — turn the specific " + - "ids/sizes/names into input parameters — and call save_tool ONCE to " + - "compile it into a persistent, 0-token tool the user can rerun from the " + - "▶ Run tool window. Skip this only if it was truly one-off.]"; + var guided = System.Text.Json.Nodes.JsonNode.Parse(content)!.AsObject(); + guided["tool_guidance"] = "If this repeats a proven pattern, consider saving a parameterized tool. Verification is still required; choose a better approach freely."; + content = ToolResultCap.Apply(guided.ToJsonString(ToolResult.Options), name); _promoteNudged = true; } } @@ -1476,6 +1476,8 @@ private static bool LooksUnfinished(string text) // continue, would go and do something the user never asked for. if (tail.Contains("let me know") || tail.Contains("дайте знать") || tail.Contains("дай знать")) return false; + if (tail.Contains("можно") || tail.Contains("могу") || tail.Contains("если хотите") || + tail.Contains("could ") || tail.Contains("can also") || tail.Contains("if you want")) return false; return ContinueCues.Any(c => StartsAtWordBoundary(tail, c)); } @@ -1801,10 +1803,8 @@ private List AllowedTools() .OrderBy(t => t.Name, StringComparer.Ordinal).ToList(); var revealed = new List(); - foreach (var cat in _revealedCategories) // reveal order → append-only growth - revealed.AddRange(all - .Where(t => !ToolCatalog.IsCore(t) && ToolCatalog.CategoryOf(t) == cat) - .OrderBy(t => t.Name, StringComparer.Ordinal)); + foreach (var name in _revealedTools) // stable append order, bounded discovery + if (all.FirstOrDefault(t => t.Name == name && !ToolCatalog.IsCore(t)) is { } found) revealed.Add(found); var custom = all.Where(ToolCatalog.IsCustom) .OrderBy(t => t.Name, StringComparer.Ordinal).ToList(); diff --git a/ClaudeRevit/Services/ClaudeCodeBackend.cs b/ClaudeRevit/Services/ClaudeCodeBackend.cs index aa3eddf..1d82a0e 100644 --- a/ClaudeRevit/Services/ClaudeCodeBackend.cs +++ b/ClaudeRevit/Services/ClaudeCodeBackend.cs @@ -418,6 +418,7 @@ private static async Task VerifySubscriptionAsync(string exe, string workDir, Ca var key = AuthFingerprint.For(exe, Path.Combine(home, ".credentials.json"), Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".claude.json")); if (SignedIn.TryGetValue(key, out var at) && DateTime.UtcNow - at < TimeSpan.FromMinutes(30)) return; + using var authPhase = TaskJournal.MeasurePhase("authorization_process"); try { await VerifySubscriptionCoreAsync(exe, workDir, ct); } catch (OperationCanceledException) when (!ct.IsCancellationRequested) { diff --git a/ClaudeRevit/Services/CodexBackend.cs b/ClaudeRevit/Services/CodexBackend.cs index a25ab0e..f8c288d 100644 --- a/ClaudeRevit/Services/CodexBackend.cs +++ b/ClaudeRevit/Services/CodexBackend.cs @@ -146,6 +146,7 @@ private static async Task EnsureSignedInAsync(string exe, string workDir, Cancel : Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".codex"); var key = AuthFingerprint.For(exe, Path.Combine(codexHome, "auth.json")); if (SignedIn.TryGetValue(key, out var at) && DateTime.UtcNow - at < SignInValidFor) return; + using var authPhase = TaskJournal.MeasurePhase("authorization_process"); using var login = Start(exe, new[] { "login", "status" }, workDir); using var timeout = CancellationTokenSource.CreateLinkedTokenSource(ct); timeout.CancelAfter(TimeSpan.FromSeconds(15)); diff --git a/ClaudeRevit/Services/HeadlessResultLimits.cs b/ClaudeRevit/Services/HeadlessResultLimits.cs index e4f62c9..6f040b3 100644 --- a/ClaudeRevit/Services/HeadlessResultLimits.cs +++ b/ClaudeRevit/Services/HeadlessResultLimits.cs @@ -6,13 +6,15 @@ namespace ModPlus.Headless; // rollback status in the host's generic output cap. This helper never changes a success flag. public static class HeadlessResultLimits { + private static readonly System.Text.Json.JsonSerializerOptions Options = new() + { Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; public static string Compact(string raw) { if (raw.Length <= 30_000) return raw; var source = JsonNode.Parse(raw) as JsonObject ?? throw new InvalidOperationException("Expected a structured headless result."); var result = new JsonObject { ["truncated"] = true, ["total_chars"] = raw.Length }; foreach (var name in new[] { "ok", "preview", "committed", "rolledBack", "rejected", "idsTemporary", "modelModified", "module", "operation", "error", "errorCode" }) - if (source[name] is { } value) result[name] = value.ToJsonString().Length <= 2_000 ? value.DeepClone() : JsonValue.Create(value.ToJsonString()[..2_000] + "…"); + if (source[name] is { } value) result[name] = value.ToJsonString(Options).Length <= 2_000 ? value.DeepClone() : JsonValue.Create(value.ToJsonString(Options)[..2_000] + "…"); result["note"] = "Payload shortened; control fields retain their original status. Narrow the request or set emit=false on intermediate pipeline steps."; // JSON escaping can expand each Cyrillic/control character to six characters. // Budget the serialized envelope, rather than only the source substring. @@ -20,7 +22,7 @@ public static string Compact(string raw) while (true) { result["head"] = raw[..length]; - var envelope = result.ToJsonString(); + var envelope = result.ToJsonString(Options); if (envelope.Length <= 30_000) return envelope; if (length == 0) throw new InvalidOperationException("Headless control fields exceed the transport budget."); length /= 2; diff --git a/ClaudeRevit/Services/McpServer.cs b/ClaudeRevit/Services/McpServer.cs index 0e5c6b6..8335df5 100644 --- a/ClaudeRevit/Services/McpServer.cs +++ b/ClaudeRevit/Services/McpServer.cs @@ -541,7 +541,7 @@ private static JsonObject ToolResult(string text, bool isError, bool attachUserU using var json=JsonDocument.Parse(text); if(json.RootElement.TryGetProperty("image_id",out var id) && ViewImageStore.Find(id.GetString()??"",ExecutingChannel?.DocumentKey??DocumentSessions.CurrentDocumentKey,ExecutingChannel?.Id) is { } image) content.Add(new JsonObject { ["type"]="image",["mimeType"]="image/png",["data"]=image.Base64 }); - if(json.RootElement.TryGetProperty("ok",out var ok)&&ok.ValueKind==JsonValueKind.False)isError=true; + if(!Services.ToolResult.LooksSuccessful(text))isError=true; } catch(JsonException) { } return new JsonObject { ["content"]=content,["isError"]=isError }; diff --git a/ClaudeRevit/Services/TaskJournal.cs b/ClaudeRevit/Services/TaskJournal.cs index ca64631..2171518 100644 --- a/ClaudeRevit/Services/TaskJournal.cs +++ b/ClaudeRevit/Services/TaskJournal.cs @@ -12,10 +12,28 @@ public static class TaskJournal private static readonly AsyncLocal Current=new(); private static readonly System.Collections.Concurrent.ConcurrentDictionary Timings=new(); private static readonly System.Collections.Concurrent.ConcurrentDictionary Workers=new(); + private static readonly System.Collections.Concurrent.ConcurrentDictionary> Phases = new(); + public sealed record PhaseMetric(int Count, double Seconds); + private static string? _lastWriteError; + public static string? LastWriteError => Volatile.Read(ref _lastWriteError); public static string? CurrentId => Current.Value; + public static IDisposable MeasurePhase(string phase) => new PhaseScope(CurrentId, phase); + private sealed class PhaseScope(string? id, string phase) : IDisposable + { + private readonly long _started = System.Diagnostics.Stopwatch.GetTimestamp(); + private int _disposed; + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) != 0 || id == null || !Phases.TryGetValue(id, out var phases)) return; + var seconds = System.Diagnostics.Stopwatch.GetElapsedTime(_started).TotalSeconds; + phases.AddOrUpdate(phase, new PhaseMetric(1, seconds), (_, old) => new(old.Count + 1, old.Seconds + seconds)); + } + } + public static IReadOnlyDictionary ReadPhases(string? id) => + id != null && Phases.TryGetValue(id, out var phases) ? new Dictionary(phases) : new Dictionary(); public static IDisposable Start(string documentKey) - { var old=Current.Value;Current.Value=Guid.NewGuid().ToString("N");Timings[Current.Value]=default;Workers[Current.Value]=0;Append(new { kind="task_started",task_id=Current.Value,document_key=documentKey,utc=DateTime.UtcNow });return new Scope(old,Current.Value); } - private sealed class Scope(string? old,string id):IDisposable { public void Dispose(){ Timings.TryRemove(id,out _);Workers.TryRemove(id,out _);Current.Value=old; } } + { var old=Current.Value;Current.Value=Guid.NewGuid().ToString("N");Timings[Current.Value]=default;Workers[Current.Value]=0;Phases[Current.Value]=new();Append(new { kind="task_started",task_id=Current.Value,document_key=documentKey,utc=DateTime.UtcNow });return new Scope(old,Current.Value); } + private sealed class Scope(string? old,string id):IDisposable { public void Dispose(){ Timings.TryRemove(id,out _);Workers.TryRemove(id,out _);Phases.TryRemove(id,out _);Current.Value=old; } } public static void RecordWorker(string? id,double seconds) {if(id!=null&&Workers.ContainsKey(id))Workers.AddOrUpdate(id,seconds,(_,sum)=>sum+seconds);} public static double ReadWorker(string? id)=>id!=null&&Workers.TryGetValue(id,out var seconds)?seconds:0; public static void RecordTiming(string? id,double queue,double execution) @@ -32,8 +50,9 @@ public static void Append(object entry) if(File.Exists(PathName)&&new FileInfo(PathName).Length>10_000_000) { var lines=File.ReadAllLines(PathName);File.WriteAllLines(PathName,lines.TakeLast(1000)); } File.AppendAllText(PathName,line+"\n"); + Volatile.Write(ref _lastWriteError, null); } - catch { } + catch (Exception ex) { Volatile.Write(ref _lastWriteError, ex.Message); } },CancellationToken.None,TaskContinuationOptions.None,TaskScheduler.Default); } public static string ReadRecent(string documentKey) diff --git a/ClaudeRevit/Services/ToolResult.cs b/ClaudeRevit/Services/ToolResult.cs index 154dfcf..8ff1f61 100644 --- a/ClaudeRevit/Services/ToolResult.cs +++ b/ClaudeRevit/Services/ToolResult.cs @@ -7,6 +7,34 @@ namespace ClaudeRevit.Services; // Preserve existing payload fields for older clients. Every response remains ONE JSON value. public static class ToolResult { + // Shared by dispatcher, journals and chat transports. Python uses succeeded. + public static bool LooksSuccessful(string raw) + { + try + { + var obj = JsonNode.Parse(raw) as JsonObject; + if (obj == null) return true; + if (IsFalse(obj["ok"]) || IsFalse(obj["succeeded"])) return false; + if (obj["status"] is JsonValue status && status.TryGetValue(out var label) && label is "failed" or "error") return false; + if (obj["ok"] is JsonValue count && count.TryGetValue(out var n) && n == 0 && + (HasFailures(obj["failed"]) || HasFailures(obj["failures"]))) return false; + if (!obj.ContainsKey("ok") && obj["error"] is { } error && error.ToJsonString() is not ("null" or "\"\"")) return false; + return true; + } + catch (JsonException) { return true; } // legacy text results + } + private static bool IsFalse(JsonNode? node) => node is JsonValue v && v.TryGetValue(out var b) && !b; + private static bool IsTrue(JsonNode? node) => node is JsonValue v && v.TryGetValue(out var b) && b; + public static string Execution(string raw, bool committed, bool rolledBack) + { + var obj = JsonNode.Parse(Complete(raw))!.AsObject(); + var preview = IsTrue(obj["preview"]); + rolledBack |= preview || IsTrue(obj["rolledBack"]) || IsTrue(obj["rolled_back"]) || IsFalse(obj["committed"]); + obj["committed"] = committed && !rolledBack; + obj["rolled_back"] = rolledBack; + obj["execution_status"] = preview ? "preview" : rolledBack ? "rolled_back" : committed ? "committed" : "read_only"; + return obj.ToJsonString(Options); + } public static string? ErrorMessage(string? raw) { if(raw==null)return null; @@ -27,11 +55,16 @@ public static string Complete(string raw, IEnumerable? warnings = null) foreach (var warning in warnings ?? []) messages.Add(warning); result["warnings"] = messages; result["errors"] ??= new JsonArray(); + if (!result.ContainsKey("ok")) result["ok"] = LooksSuccessful(raw); + if (IsFalse(result["succeeded"])) result["ok"] = false; + if (IsFalse(result["ok"]) && result["error"] == null && result["traceback"] is { } traceback) + result["error"] = traceback.DeepClone(); + if (!result.ContainsKey("status")) + result["status"] = !LooksSuccessful(raw) ? "failed" : HasFailures(result["failed"]) || HasFailures(result["failures"]) ? "partial" : "success"; if(result["ok"] is JsonValue state && state.TryGetValue(out var success) && !success && result["errors"] is JsonArray errors && errors.Count==0 && result["error"] is { } error) errors.Add(new JsonObject { ["code"]="tool_error",["message"]=error.DeepClone() }); // Some batch tools use an integer 'ok' count. Do not reinterpret that contract. - if (!result.ContainsKey("ok")) result["ok"] = true; RoundNumbers(result); return result.ToJsonString(Options); } @@ -74,6 +107,13 @@ private static void RoundNumbers(JsonNode? node) return r == d ? null : JsonValue.Create(r); } + private static bool HasFailures(JsonNode? node) => node switch + { + JsonArray a => a.Count > 0, + JsonValue v when v.TryGetValue(out var n) => n > 0, + _ => false + }; + public static string Failure(string code, string message) => new JsonObject { ["ok"] = false, ["error"] = message, ["warnings"] = new JsonArray(), diff --git a/ClaudeRevit/Services/ToolResultAging.cs b/ClaudeRevit/Services/ToolResultAging.cs index 480f35d..ad5a18a 100644 --- a/ClaudeRevit/Services/ToolResultAging.cs +++ b/ClaudeRevit/Services/ToolResultAging.cs @@ -16,8 +16,8 @@ namespace ClaudeRevit.Services; // blocks produced last turn flip once, exactly where the cache is being extended anyway. public static class ToolResultAging { - // The aged replacement (KeepChars + truncation suffix + marker ≈ 550 chars) is - // always shorter than MinChars — that inequality IS the idempotence guarantee. + // The explicit aged marker keeps summaries idempotent even when preserved control + // fields make the envelope larger than the nominal head budget. public const int MinChars = 700; public const int KeepChars = 400; @@ -58,13 +58,18 @@ private static int AgeTurn(ApiTurn turn) { if (turn.Blocks[i] is not ChatToolResultBlock tr) continue; if (tr.Content.Length <= MinChars) continue; + try + { + using var parsed = System.Text.Json.JsonDocument.Parse(tr.Content); + if (parsed.RootElement.ValueKind == System.Text.Json.JsonValueKind.Object && + parsed.RootElement.TryGetProperty("aged", out var marker) && marker.ValueKind == System.Text.Json.JsonValueKind.True) continue; + } + catch (System.Text.Json.JsonException) { } ToolResultArchive.Record(tr.ToolUseId, tr.Content); turn.Blocks[i] = new ChatToolResultBlock( tr.ToolUseId, - TextUtil.Truncate(tr.Content, KeepChars) + - "\n[aged to save tokens — call get_full_result with id \"" + tr.ToolUseId + - "\" if you truly need the full original output]", + ToolResultCap.Summarize(tr.Content, tr.ToolUseId, "aged result", KeepChars, aged: true), tr.IsError); aged++; } diff --git a/ClaudeRevit/Services/ToolResultArchive.cs b/ClaudeRevit/Services/ToolResultArchive.cs index 1b958a2..f965a06 100644 --- a/ClaudeRevit/Services/ToolResultArchive.cs +++ b/ClaudeRevit/Services/ToolResultArchive.cs @@ -12,7 +12,7 @@ namespace ClaudeRevit.Services; // restart. Bounded like the script journal so it can't grow without limit. public static class ToolResultArchive { - private const int MaxContentChars = 100_000; + public const int MaxContentChars = 2_000_000; private const long MaxFileBytes = 20_000_000; private const int KeptEntriesOnTrim = 300; @@ -33,6 +33,7 @@ public static void Record(string id, string content) lock (Gate) { Memory[id] = content; + TrimMemory(); Directory.CreateDirectory(Path.GetDirectoryName(FilePath)!); File.AppendAllText(FilePath, JsonSerializer.Serialize(new { id, content }) + "\n"); @@ -66,7 +67,7 @@ public static void Record(string id, string content) doc.RootElement.TryGetProperty("content", out var c)) { var content = c.GetString(); - if (content != null) Memory[id] = content; + if (content != null) { Memory[id] = content; TrimMemory(); } return content; } } @@ -81,13 +82,27 @@ public static void Record(string id, string content) } } + private static void TrimMemory() + { + // Bound the hot cache as well as the disk archive (UTF-16 character budget). + while (Memory.Count > KeptEntriesOnTrim || Memory.Values.Sum(x => (long)x.Length) > MaxFileBytes / 2) + Memory.Remove(Memory.Keys.First()); + } + private static void TrimIfOversized() { try { if (new FileInfo(FilePath).Length <= MaxFileBytes) return; var lines = File.ReadAllLines(FilePath); - File.WriteAllLines(FilePath, lines.TakeLast(KeptEntriesOnTrim)); + var keep = new List(); long bytes = 0; + foreach (var line in Enumerable.Reverse(lines).Take(KeptEntriesOnTrim)) + { + var size = System.Text.Encoding.UTF8.GetByteCount(line) + 2; + if (bytes + size > MaxFileBytes) break; + keep.Add(line); bytes += size; + } + keep.Reverse(); File.WriteAllLines(FilePath, keep); } catch { /* best-effort */ } } diff --git a/ClaudeRevit/Services/ToolResultCap.cs b/ClaudeRevit/Services/ToolResultCap.cs index 1904ff4..89cf81e 100644 --- a/ClaudeRevit/Services/ToolResultCap.cs +++ b/ClaudeRevit/Services/ToolResultCap.cs @@ -1,36 +1,89 @@ using System; +using System.Text.Json; using System.Text.Json.Nodes; namespace ClaudeRevit.Services; -// A hard ceiling on one tool result. Some Get*/List* tools have no limit of their own, and a -// single call on a large model could return hundreds of thousands of characters — tens of -// thousands of tokens in the very round it happens, before aging or compaction can help, and -// again on every later request. Over the cap, the full result is archived (get_full_result -// brings it back) and the model gets the head plus a note telling it to narrow the query. +// Bound the serialized envelope without rewriting failure/preview/commit semantics. public static class ToolResultCap { public const int MaxChars = 40_000; public const int HeadChars = 30_000; + private static readonly string[] Controls = + ["ok", "succeeded", "preview", "committed", "rolledBack", "rolled_back", "execution_status", + "rejected", "idsTemporary", "modelModified", "plan_token", "errorCode", "status"]; public static string Apply(string result, string toolName) { if (result.Length <= MaxChars) return result; var id = "cap_" + Guid.NewGuid().ToString("N")[..12]; ToolResultArchive.Record(id, result); - return new JsonObject + return Summarize(result, id, toolName, HeadChars); + } + + public static string Summarize(string result, string id, string toolName, int headChars, bool aged = false) + { + var envelope = new JsonObject + { + ["ok"] = ToolResult.LooksSuccessful(result), ["truncated"] = true, ["aged"] = aged, + ["total_chars"] = result.Length, ["full_result_id"] = id, + ["archive_truncated"] = result.Length > ToolResultArchive.MaxContentChars, + ["note"] = $"{toolName}: narrow the query, or read get_full_result id='{id}' with offset/limit. Archived results are historical, not live model state." + }; + try + { + if (JsonNode.Parse(ToolResult.Complete(result)) is JsonObject source) + { + foreach (var key in Controls) + if (source[key] is { } value && value.ToJsonString(ToolResult.Options).Length <= 1_000) + envelope[key] = value.DeepClone(); + foreach (var key in new[] { "error", "warnings", "errors", "verification", "plan", "receipt" }) + if (source[key] is { } value) + envelope[key] = Bounded(value, key is "warnings" or "errors"); + } + } + catch (JsonException) { } + return WithBoundedText(envelope, "head", result, 0, Math.Min(headChars, result.Length)); + } + + private static JsonNode Bounded(JsonNode value, bool array) + { + var raw = value.ToJsonString(ToolResult.Options); + if (raw.Length <= 3_000) return value.DeepClone(); + var excerpt = raw[..400] + "… (see archive)"; + return array ? new JsonArray(new JsonObject { ["code"] = "details_archived", ["message"] = excerpt }) : JsonValue.Create(excerpt)!; + } + + public static string Page(string id, string content, int offset = 0, int limit = 12_000) + { + if (offset < 0 || offset > content.Length || limit < 1 || limit > HeadChars) + throw new ArgumentOutOfRangeException(nameof(offset), "offset must be within the archived text; limit must be 1..30000."); + if (offset < content.Length && ((offset > 0 && char.IsLowSurrogate(content[offset]) && char.IsHighSurrogate(content[offset-1])) || + (limit == 1 && char.IsHighSurrogate(content[offset])))) + throw new ArgumentOutOfRangeException(nameof(offset), "Do not split a Unicode surrogate pair; use the returned next_offset and a limit of at least 2."); + return WithBoundedText(new JsonObject + { + ["ok"] = true, ["id"] = id, ["offset"] = offset, ["total_chars"] = content.Length, + ["historical"] = true + }, "content", content, offset, Math.Min(limit, content.Length - offset)); + } + + private static string WithBoundedText(JsonObject envelope, string key, string text, int offset, int length) + { + while (true) { - ["ok"] = true, - ["truncated"] = true, - ["total_chars"] = result.Length, - ["shown_chars"] = HeadChars, - ["full_result_id"] = id, - ["note"] = $"{toolName} returned {result.Length:N0} characters, more than one result may carry. " + - "The start is shown below. Narrow the request (filters, level, category, limit) " + - $"rather than paging through it; get_full_result with id '{id}' returns the archived " + - "original if you really need it.", - ["head"] = result[..HeadChars], - ["warnings"] = new JsonArray(), ["errors"] = new JsonArray() - }.ToJsonString(ToolResult.Options); + if (length > 0 && offset + length < text.Length && char.IsHighSurrogate(text[offset + length - 1])) length--; + envelope[key] = text.Substring(offset, length); + envelope["shown_chars"] = length; + if (key == "content") + { + envelope["next_offset"] = offset + length < text.Length ? JsonValue.Create(offset + length) : null; + envelope["has_more"] = offset + length < text.Length; + } + var serialized = envelope.ToJsonString(ToolResult.Options); + if (serialized.Length <= MaxChars) return serialized; + if (length == 0) throw new InvalidOperationException("Tool control fields exceed the response budget."); + length /= 2; + } } } diff --git a/ClaudeRevit/Services/ToolSearchLogic.cs b/ClaudeRevit/Services/ToolSearchLogic.cs index bcd31ea..4882283 100644 --- a/ClaudeRevit/Services/ToolSearchLogic.cs +++ b/ClaudeRevit/Services/ToolSearchLogic.cs @@ -13,7 +13,20 @@ public static class ToolSearchLogic // A tool reduced to what the search needs — decoupled from IRevitTool. public readonly record struct ToolInfo(string Name, string Description, string Category, bool IsCore); - public sealed record SearchResult(List Categories, string Message); + public sealed record SearchResult(List Categories, string Message) + { + public List ToolNames { get; init; } = new(); + } + private static readonly HashSet StopWords = new(StringComparer.Ordinal) + { "create", "creates", "make", "the", "and", "with", "for", "tool", "tools", "сделай", "создай", "нужна", "добавь", "пожалуйста" }; + private static bool Matches(string text, string keyword) + { + var words = text.ToLowerInvariant().Split(new[] { ' ', ',', '.', ':', ';', '-', '\n', '\t', '_' }, StringSplitOptions.RemoveEmptyEntries); + var keys = keyword.Trim().Split(' ', StringSplitOptions.RemoveEmptyEntries); + for (var i = 0; i + keys.Length <= words.Length; i++) + if (keys.Select((k, j) => words[i + j].StartsWith(k, StringComparison.Ordinal)).All(x => x)) return true; + return false; + } // Core = the tools reached in almost every session. Curated by NAME so it stays tight; a tool // omitted here isn't lost, it just costs one find_tools round the first time it's needed. @@ -44,7 +57,7 @@ public sealed record SearchResult(List Categories, string Message); private static readonly (string Category, string[] Keys)[] GroupKeywords = { ("ModPlus", new[] { "modplus", "mod plus", "модплюс", "мод плюс", "mprsumparameters", "сумма параметров" }), - ("Rebar", new[] { "rebar", "reinforc", "stirrup", "арматур", "армир", "хомут", "стержн", "сетк", "fabric", "mesh", "ведомость детал", "bending" }), + ("Rebar", new[] { "rebar", "reinforc", "stirrup", "арматур", "армир", "хомут", "стержн", "арматурная сет", "сетка армат", "fabric reinforcement", "reinforcement mesh", "ведомость детал", "bending" }), ("MEP", new[] { "duct", "pipe", "hvac", "воздуховод", "труб", "инженерн" }), ("Schedules", new[] { "schedule", "quantit", "специфик", "ведомост", "расписан", "таблиц" }), ("Sheets", new[] { "sheet", "titleblock", "viewport", "лист", "штамп", "титул" }), @@ -61,7 +74,7 @@ private static readonly (string Category, string[] Keys)[] GroupKeywords = "view template", "crop", "view range", "разрез", "фасад", "камер", "видовой", "шаблон вид", "секущ" }), ("Family editor", new[] { "family editor", "family parameter", "reference plane", - "семейств", "опорн плоск", "формул", "parametric profile", "параметрическ", "сечени" }), + "семейств", "опорн плоск", "формул", "parametric profile", "параметрическ" }), ("Visibility", new[] { "isolate", "override", "hide categ", "graphic", "изолир", "переопредел", "фильтр вид" }), }; @@ -75,7 +88,7 @@ public static List Prewarm(string prompt) if (string.IsNullOrWhiteSpace(prompt)) return result; var p = prompt.ToLowerInvariant(); foreach (var (cat, keys) in GroupKeywords) - if (keys.Any(k => p.Contains(k))) result.Add(cat); + if (keys.Any(k => Matches(p, k))) result.Add(cat); return result; } @@ -87,11 +100,15 @@ public static SearchResult Search(IEnumerable tools, string query) .Split(new[] { ' ', ',', '.', '"', '\'', '(', ')', '/', '-', ':', ';', '\n', '\t' }, StringSplitOptions.RemoveEmptyEntries) .Select(t => t.ToLowerInvariant()) - .Where(t => t.Length >= 3) + .Where(t => t.Length >= 3 && !StopWords.Contains(t)) .Distinct() .ToList(); var deferred = tools.Where(t => !t.IsCore).ToList(); + var exact = deferred.FirstOrDefault(t => string.Equals(t.Name, query?.Trim(), StringComparison.OrdinalIgnoreCase)); + if (!string.IsNullOrEmpty(exact.Name)) + return new SearchResult(new() { exact.Category }, $"Loaded 1 tool: {exact.Name} — {FirstSentence(exact.Description, 90)}") + { ToolNames = new() { exact.Name } }; // Tool names and descriptions are English, so a Russian query used to score zero on every // tool and "арматура в плите" answered "nothing found" — the same words that the prewarm @@ -100,7 +117,7 @@ public static SearchResult Search(IEnumerable tools, string query) var keyedCategories = terms.Count == 0 ? new List() : GroupKeywords - .Where(g => g.Keys.Any(k => terms.Any(term => term.Contains(k) || k.Contains(term)))) + .Where(g => g.Keys.Any(k => Matches(query ?? "", k))) .Select(g => g.Category) .ToList(); @@ -146,14 +163,15 @@ int Score(ToolInfo t) .Take(3) .Select(x => x.Category) .ToList(); - var revealed = deferred.Where(t => cats.Contains(t.Category)) - .OrderBy(t => t.Name, StringComparer.Ordinal).ToList(); + // A group is a search hint, not permission to append every schema in it. + var revealed = scored.Where(x => cats.Contains(x.Tool.Category)).Take(12) + .Select(x => x.Tool).ToList(); var sb = new StringBuilder(); sb.Append("Loaded ").Append(revealed.Count).Append(" tool(s) — now callable directly:\n"); foreach (var t in revealed) sb.Append("• ").Append(t.Name).Append(" — ").Append(FirstSentence(t.Description, 90)).Append('\n'); - return new SearchResult(cats, sb.ToString().TrimEnd()); + return new SearchResult(cats, sb.ToString().TrimEnd()) { ToolNames = revealed.Select(t => t.Name).ToList() }; } private static string FirstSentence(string? text, int max) diff --git a/ClaudeRevit/Tools/AuditSpdsSchedule.cs b/ClaudeRevit/Tools/AuditSpdsSchedule.cs index fc10858..2456b60 100644 --- a/ClaudeRevit/Tools/AuditSpdsSchedule.cs +++ b/ClaudeRevit/Tools/AuditSpdsSchedule.cs @@ -27,7 +27,7 @@ public string Execute(IReadOnlyDictionary input,UIApplicatio var sources=record==null?[]:LiveSpds.Sources(doc,record); return Services.Json.Serialize(new {id=s.Id.Value,live=s.Definition.GetFieldCount()>0,include_linked_files=s.Definition.CanIncludeLinkedFiles()&&s.Definition.IncludeLinkedFiles, fields=s.Definition.GetFieldOrder().Select(id=>s.Definition.GetField(id)).Select(f=>new {name=f.GetName(),heading=f.ColumnHeading,parameter_id=f.ParameterId.Value,spec=f.GetSpecTypeId().TypeId,total=f.DisplayType.ToString(),hidden=f.IsHidden,width_mm=f.SheetColumnWidth*Units.MmPerFoot}).ToArray(), - nesting_policy=record?.NestingPolicy,nested_sources=sources.OfType().Where(f=>f.SuperComponent!=null).Select(f=>new {id=f.Id.Value,parent_id=f.SuperComponent.Id.Value}).ToArray(),placements=layout, + refresh_error=record?.LastRefreshError,refresh_utc=record?.LastRefreshUtc,quantities_status=record==null?"unmanaged":record.LastRefreshError!=null?"stale_or_incomplete":record.LastRefreshUtc==null?"not_audited":"refreshed",nesting_policy=record?.NestingPolicy,nested_sources=sources.OfType().Where(f=>f.SuperComponent!=null).Select(f=>new {id=f.Id.Value,parent_id=f.SuperComponent.Id.Value}).ToArray(),placements=layout, linked_documents=new FilteredElementCollector(doc).OfClass(typeof(RevitLinkInstance)).Cast().Select(l=>new {id=l.Id.Value,loaded=l.GetLinkDocument()!=null,name=l.Name}).Take(100).ToArray(), visual_review="Uniform margins and rectangular overlaps only. Check title-block zones, split schedule headers, border style and text wrapping in an exported view before issuing drawings."}); } diff --git a/ClaudeRevit/Tools/CalculateWeight.cs b/ClaudeRevit/Tools/CalculateWeight.cs index e26a865..36ba203 100644 --- a/ClaudeRevit/Tools/CalculateWeight.cs +++ b/ClaudeRevit/Tools/CalculateWeight.cs @@ -201,14 +201,11 @@ private static double SolidVolume(GeometryObject g) return null; } - // Revit's internal structural density unit is pounds-mass per cubic foot in the API's - // unit-less double; convert to kg/m3. Guarded so an unexpected magnitude never silently - // produces a nonsense tonnage. + // Revit's internal density is kg/ft³, including light insulation materials. private static double? DensityToKgM3(double revitDensity) { - const double LbPerFt3ToKgPerM3 = 16.0184634; - var kg = revitDensity * LbPerFt3ToKgPerM3; - return kg > 100 && kg < 25000 ? kg : (double?)null; + var kg = Units.DensityToKgM3(revitDensity); + return double.IsFinite(kg) && kg > 0 ? kg : (double?)null; } private static double DefaultDensity(Element el) diff --git a/ClaudeRevit/Tools/CleanModel.cs b/ClaudeRevit/Tools/CleanModel.cs index 767a044..5ab675c 100644 --- a/ClaudeRevit/Tools/CleanModel.cs +++ b/ClaudeRevit/Tools/CleanModel.cs @@ -17,7 +17,7 @@ public class CleanModel : IRevitTool public string Name => "clean_model"; public string Description => - "Remove model bloat: unused family types (no instances), unused materials, exploded CAD imports, " + + "Remove model bloat: native Purge Unused family types, unused materials, explicitly selected non-linked CAD imports, " + "unplaced rooms, and empty groups. DRY RUN BY DEFAULT — returns what would be deleted with counts and " + "names; pass apply=true to actually delete. Choose what to touch with `targets`. " + "Pair with diagnose_model, which explains what is wrong before you clean it."; @@ -29,7 +29,7 @@ public class CleanModel : IRevitTool ["targets"] = JsonSerializer.SerializeToElement(new { type = "array", - description = "What to clean. Default: all of them.", + description = "What to clean. Default excludes non-linked CAD imports; exploded_imports explicitly selects all non-linked imports.", items = new { type = "string", @@ -54,7 +54,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var apply = input.TryGetValue("apply", out var a) && a.ValueKind == JsonValueKind.True; - var all = new[] { "unused_family_types", "unused_materials", "exploded_imports", "unplaced_rooms", "empty_groups" }; + var all = new[] { "unused_family_types", "unused_materials", "unplaced_rooms", "empty_groups" }; var targets = input.TryGetValue("targets", out var t) && t.ValueKind == JsonValueKind.Array ? t.EnumerateArray().Select(x => x.GetString() ?? "").Where(x => x.Length > 0).ToHashSet() : all.ToHashSet(); @@ -70,92 +70,77 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var deleted = 0; var failures = new List(); - if (apply && toDelete.Count > 0) + var cascades = new List(); + var candidates = toDelete.Distinct().ToHashSet(); + // Rehearse actual Revit deletion in a subtransaction even for dry runs. + // Never silently cascade outside the candidate set, including on apply. + foreach (var id in candidates) { - // Delete one at a time: Revit cascades dependent deletions, so a single bad id in a - // bulk call would abort the whole clean-up. - foreach (var id in toDelete.Distinct()) + ToolContext.ThrowIfCancelled(); + if (doc.GetElement(id) == null) continue; + using var sub = new SubTransaction(doc); + sub.Start(); + try { - try - { - if (doc.GetElement(id) == null) continue; // already cascaded away - var removed = doc.Delete(id); - deleted += removed?.Count ?? 1; - } - catch (Exception ex) - { - if (failures.Count < 25) failures.Add(new { id = id.Value, reason = Short(ex.Message) }); - } + var removed = doc.Delete(id).ToArray(); + var unexpected = removed.Where(x => !candidates.Contains(x)).ToArray(); + cascades.Add(new { id = id.Value, deleted_ids = removed.Select(x => x.Value).ToArray(), + unexpected_ids = unexpected.Select(x => x.Value).ToArray() }); + if (!apply || unexpected.Length > 0) sub.RollBack(); + else { sub.Commit(); deleted += removed.Length; } + if (unexpected.Length > 0) failures.Add(new { id = id.Value, reason = "Dependent elements outside the candidate set: deletion refused." }); + } + catch (Autodesk.Revit.Exceptions.RegenerationFailedException) { throw; } + catch (OperationCanceledException) { throw; } + catch (Exception ex) + { + if (sub.GetStatus() == TransactionStatus.Started) sub.RollBack(); + failures.Add(new { id = id.Value, reason = Short(ex.Message) }); } } return Services.Json.Serialize(new { + preview = !apply, mode = apply ? "applied" : "dry_run", candidates = toDelete.Distinct().Count(), deleted_elements = apply ? deleted : 0, failures, + cascades, details = report, note = apply - ? "Deleted. The whole clean-up is one undo step (Ctrl+Z)." + ? "Applied safe candidates only; inspect failures and cascades. Each tool call is one undo step." : "DRY RUN — nothing was deleted. Show this to the user, then call again with apply=true to proceed." }); } - // A family symbol is unused when no instance of it exists. Skip in-place and system-critical - // families, and never leave a family with zero types behind. + // Native Purge Unused understands references in parameters, nested types, + // painted faces and other model data that instance counting misses. private static object UnusedFamilyTypes(Document doc, List sink) { - var usedTypeIds = new HashSet( - new FilteredElementCollector(doc).WhereElementIsNotElementType() - .Select(e => e.GetTypeId()) - .Where(id => id != ElementId.InvalidElementId)); - - var names = new List(); - var count = 0; - - foreach (var sym in new FilteredElementCollector(doc).OfClass(typeof(FamilySymbol)) - .Cast()) + var purgeable = doc.GetUnusedElements(new HashSet()); + var candidates = new FilteredElementCollector(doc).OfClass(typeof(FamilySymbol)).Cast() + .Where(s => purgeable.Contains(s.Id) && s.Family?.IsInPlace != true).ToList(); + // Retain at least one type in every loaded family. + var selected = new List(); + foreach (var group in candidates.GroupBy(s => s.Family.Id)) { - try - { - if (usedTypeIds.Contains(sym.Id)) continue; - if (sym.Family?.IsInPlace == true) continue; // in-place has no reusable type - sink.Add(sym.Id); - count++; - if (names.Count < 50) names.Add($"{sym.Family?.Name}: {sym.Name}"); - } - catch { } + var family = group.First().Family; + var ordered = group.OrderBy(s => s.Id.Value).ToList(); + var remove = ordered.Take(Math.Min(ordered.Count, Math.Max(0, family.GetFamilySymbolIds().Count - 1))); + selected.AddRange(remove); } - return new { target = "unused_family_types", count, sample = names }; + sink.AddRange(selected.Select(s => s.Id)); + return new { target = "unused_family_types", count = selected.Count, sample = selected.Take(50).Select(s => $"{s.Family.Name}: {s.Name}") }; } - // A material is unused when nothing references it. Checking every element's material ids is the - // only reliable test — appearance/structural assets alone don't prove usage. private static object UnusedMaterials(Document doc, List sink) { - var used = new HashSet(); - foreach (var e in new FilteredElementCollector(doc).WhereElementIsNotElementType()) - { - ToolContext.ThrowIfCancelled(); - - try { foreach (var m in e.GetMaterialIds(false)) used.Add(m); } catch { } - } - foreach (var e in new FilteredElementCollector(doc).WhereElementIsElementType()) - { - try { foreach (var m in e.GetMaterialIds(false)) used.Add(m); } catch { } - } - - var names = new List(); - var count = 0; - foreach (var mat in new FilteredElementCollector(doc).OfClass(typeof(Material)).Cast()) - { - if (used.Contains(mat.Id)) continue; - sink.Add(mat.Id); - count++; - if (names.Count < 50) names.Add(mat.Name); - } - return new { target = "unused_materials", count, sample = names }; + var purgeable = doc.GetUnusedElements(new HashSet()); + var materials = new FilteredElementCollector(doc).OfClass(typeof(Material)).Cast() + .Where(m => purgeable.Contains(m.Id)).ToList(); + sink.AddRange(materials.Select(m => m.Id)); + return new { target = "unused_materials", count = materials.Count, sample = materials.Take(50).Select(m => m.Name) }; } // Exploded (non-linked) CAD imports drag every line style and text type of the source DWG into @@ -188,8 +173,7 @@ private static object UnplacedRooms(Document doc, List sink) { try { - var area = r.get_Parameter(BuiltInParameter.ROOM_AREA); - if (area != null && area.HasValue && area.AsDouble() > 0) continue; + if (r.Location != null) continue; // placed but unbounded/redundant rooms are NOT unplaced sink.Add(r.Id); count++; if (names.Count < 50) names.Add(r.Name); diff --git a/ClaudeRevit/Tools/CreateBeam.cs b/ClaudeRevit/Tools/CreateBeam.cs index cad99b6..58af8b2 100644 --- a/ClaudeRevit/Tools/CreateBeam.cs +++ b/ClaudeRevit/Tools/CreateBeam.cs @@ -67,12 +67,8 @@ double Coordinate(string key) => input.TryGetValue(key, out var v) && v.TryGetDo if (input.TryGetValue("type_name", out var tn) && tn.ValueKind == JsonValueKind.String) { var name = tn.GetString(); - symbol = new FilteredElementCollector(doc) - .OfCategory(BuiltInCategory.OST_StructuralFraming) - .OfClass(typeof(FamilySymbol)) - .Cast() - .FirstOrDefault(s => s.Name == name) - ?? throw NameResolve.MissingType(doc, name, "Beam type", BuiltInCategory.OST_StructuralFraming); + symbol = NameResolve.ByName(doc, name, "Beam type", + c => c.OfCategory(BuiltInCategory.OST_StructuralFraming).Cast()); } else { diff --git a/ClaudeRevit/Tools/CreateFloorType.cs b/ClaudeRevit/Tools/CreateFloorType.cs index d1f64be..49aa51b 100644 --- a/ClaudeRevit/Tools/CreateFloorType.cs +++ b/ClaudeRevit/Tools/CreateFloorType.cs @@ -99,7 +99,14 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var idx = cs.GetFirstCoreLayerIndex(); if (thicknessMm.HasValue) { - try { cs.SetLayerWidth(idx, thicknessMm.Value * MmToFeet); } + try + { + if (idx < 0) throw new ToolInputException("Base type has no editable core layer."); + var target = thicknessMm.Value * MmToFeet; + var core = target - (cs.GetWidth() - cs.GetLayerWidth(idx)); + if (!double.IsFinite(core) || core <= 0) throw new ToolInputException("Requested total thickness does not leave a positive core layer."); + cs.SetLayerWidth(idx, core); + } catch (Exception ex) { throw new InvalidOperationException( @@ -122,7 +129,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat id = newType.Id.Value, name, based_on = baseType.Name, - thickness_mm = thicknessMm, + thickness_mm = newType.GetCompoundStructure()?.GetWidth() * Units.MmPerFoot, material = appliedMaterial }); } diff --git a/ClaudeRevit/Tools/CreateIsolatedFoundation.cs b/ClaudeRevit/Tools/CreateIsolatedFoundation.cs index 6501e4e..3478bed 100644 --- a/ClaudeRevit/Tools/CreateIsolatedFoundation.cs +++ b/ClaudeRevit/Tools/CreateIsolatedFoundation.cs @@ -52,7 +52,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat symbol = new FilteredElementCollector(doc) .OfCategory(BuiltInCategory.OST_StructuralFoundation) .OfClass(typeof(FamilySymbol)).Cast() - .FirstOrDefault(s => s.Name == name) + .FirstOrDefault(s => NameResolve.Matches(s, name)) ?? throw NameResolve.MissingType(doc, name, "Foundation type", BuiltInCategory.OST_StructuralFoundation); } else diff --git a/ClaudeRevit/Tools/CreateWallType.cs b/ClaudeRevit/Tools/CreateWallType.cs index 2d0a69a..d6923c5 100644 --- a/ClaudeRevit/Tools/CreateWallType.cs +++ b/ClaudeRevit/Tools/CreateWallType.cs @@ -107,7 +107,14 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var idx = cs.GetFirstCoreLayerIndex(); if (thicknessMm.HasValue) { - try { cs.SetLayerWidth(idx, thicknessMm.Value * MmToFeet); } + try + { + if (idx < 0) throw new ToolInputException("Base type has no editable core layer."); + var target = thicknessMm.Value * MmToFeet; + var core = target - (cs.GetWidth() - cs.GetLayerWidth(idx)); + if (!double.IsFinite(core) || core <= 0) throw new ToolInputException("Requested total thickness does not leave a positive core layer."); + cs.SetLayerWidth(idx, core); + } catch (Exception ex) { throw new InvalidOperationException( @@ -130,7 +137,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat id = newType.Id.Value, name, based_on = baseType.Name, - thickness_mm = thicknessMm, + thickness_mm = newType.GetCompoundStructure()?.GetWidth() * Units.MmPerFoot, material = appliedMaterial }); } diff --git a/ClaudeRevit/Tools/DeriveParameters.cs b/ClaudeRevit/Tools/DeriveParameters.cs index 2cdb1fa..c966765 100644 --- a/ClaudeRevit/Tools/DeriveParameters.cs +++ b/ClaudeRevit/Tools/DeriveParameters.cs @@ -24,7 +24,7 @@ public class DeriveParameters : IRevitTool "template example: \"{level_name}-{type_name}\" — any source name in braces, plus {index} for a running " + "number. Use `round` for numeric sources and `factor` to scale (e.g. tonnes).\n" + "Target a category (optionally on_level) or an explicit element_ids list. Writes text or numeric " + - "parameters; numeric targets receive Revit-internal units unless the source is already unit-specific."; + "parameters; Length/Area/Volume targets convert the declared source units to Revit internal units; mismatched dimensions are rejected. Number/Text targets retain source values."; public InputSchema InputSchema => new() { @@ -103,7 +103,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat { d = Math.Round(d * factor, decimals); shown = d.ToString(CultureInfo.InvariantCulture); - if (p.StorageType == StorageType.Double) p.Set(d); + if (p.StorageType == StorageType.Double) SetMeasured(p, d, source); else if (p.StorageType == StorageType.Integer) p.Set((int)Math.Round(d)); else if (p.StorageType == StorageType.String) p.Set(shown); else { failed.Add(new { id = el.Id.Value, reason = "incompatible parameter type" }); continue; } @@ -114,7 +114,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat if (p.StorageType == StorageType.String) p.Set(shown); else if (p.StorageType == StorageType.Integer && int.TryParse(shown, out var iv)) p.Set(iv); else if (p.StorageType == StorageType.Double && - double.TryParse(shown, NumberStyles.Any, CultureInfo.InvariantCulture, out var dv)) p.Set(dv); + double.TryParse(shown, NumberStyles.Any, CultureInfo.InvariantCulture, out var dv)) SetMeasured(p, dv, source); else { failed.Add(new { id = el.Id.Value, reason = "incompatible parameter type" }); continue; } } written.Add(new { id = el.Id.Value, value = shown }); @@ -133,6 +133,22 @@ public string Execute(IReadOnlyDictionary input, UIApplicat }); } + private static void SetMeasured(Parameter p, double value, string source) + { + var spec = p.Definition.GetDataType(); + // Text/Number parameters keep the declared source value. Measurable targets + // require a matching source dimension; template strings carry no unit contract. + if (spec == SpecTypeId.Number) { p.Set(value); return; } + var unit = source switch + { + "length_mm" or "height_mm" or "elevation_mm" or "top_elevation_mm" when spec == SpecTypeId.Length => UnitTypeId.Millimeters, + "area_m2" when spec == SpecTypeId.Area => UnitTypeId.SquareMeters, + "volume_m3" when spec == SpecTypeId.Volume => UnitTypeId.CubicMeters, + _ => throw new ToolInputException("Source dimension does not match the destination parameter. Use a Number/Text target for unitless or template values.") + }; + p.Set(UnitUtils.ConvertToInternalUnits(value, unit)); + } + private static bool HasValue(Parameter p) => p.StorageType switch { StorageType.String => !string.IsNullOrWhiteSpace(p.AsString()), diff --git a/ClaudeRevit/Tools/FindTools.cs b/ClaudeRevit/Tools/FindTools.cs index e2697c5..41b03c5 100644 --- a/ClaudeRevit/Tools/FindTools.cs +++ b/ClaudeRevit/Tools/FindTools.cs @@ -47,6 +47,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var query = input.TryGetValue("query", out var q) && q.ValueKind == JsonValueKind.String ? q.GetString() ?? "" : ""; // No reveal here (the manual Run-tool window has no session): just report matches. - return ToolCatalog.Search(ToolRegistry.Instance.All, query).Message; + var search = ToolCatalog.Search(ToolRegistry.Instance.All.Where(t => ToolPolicy.IsEnabled(t) && !ToolCatalog.IsModPlusAlias(t)), query); + return Services.Json.Serialize(new { tools = search.ToolNames, message = search.Message }); } } diff --git a/ClaudeRevit/Tools/GenerateDiagnosticReport.cs b/ClaudeRevit/Tools/GenerateDiagnosticReport.cs index 62d8b6d..eda025e 100644 --- a/ClaudeRevit/Tools/GenerateDiagnosticReport.cs +++ b/ClaudeRevit/Tools/GenerateDiagnosticReport.cs @@ -36,6 +36,6 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var report = ExperienceStore.BuildDiagnosticReport(); // Also persist a fresh copy so the on-disk file matches what was just shown. ExperienceStore.WriteDiagnosticReport(); - return Json.Serialize(new { report, saved_to = ExperienceStore.ReportPath }); + return Json.Serialize(new { report, saved_to = ExperienceStore.ReportPath, journal_write_error = TaskJournal.LastWriteError, task_journal = TaskJournal.PathName }); } } diff --git a/ClaudeRevit/Tools/GetElementsOnLevel.cs b/ClaudeRevit/Tools/GetElementsOnLevel.cs index 83f36ed..ab38302 100644 --- a/ClaudeRevit/Tools/GetElementsOnLevel.cs +++ b/ClaudeRevit/Tools/GetElementsOnLevel.cs @@ -53,7 +53,7 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var raw = collector .WhereElementIsNotElementType() - .Where(e => e.LevelId == levelId) + .WherePasses(new ElementLevelFilter(levelId)) .Take(limit + 1) .ToList(); var truncated = raw.Count > limit; diff --git a/ClaudeRevit/Tools/GetFullResult.cs b/ClaudeRevit/Tools/GetFullResult.cs index 217d42d..0f95608 100644 --- a/ClaudeRevit/Tools/GetFullResult.cs +++ b/ClaudeRevit/Tools/GetFullResult.cs @@ -15,7 +15,7 @@ public class GetFullResult : IRevitTool public string Name => "get_full_result"; public string Description => - "Returns the archived FULL text of an aged tool result. Older tool outputs are " + + "Returns a bounded page of archived text. Follow next_offset until has_more=false. Older tool outputs are " + "truncated in the conversation to save tokens; the truncation marker contains the " + "id to pass here. Use it only when the preview is insufficient AND the data cannot " + "have changed — when elements may have been modified since, re-query the live model " + @@ -25,6 +25,8 @@ public class GetFullResult : IRevitTool { Properties = new Dictionary { + ["offset"] = JsonSerializer.SerializeToElement(new { type = "integer", minimum = 0, description = "Character offset, default 0. Use next_offset from the previous page." }), + ["limit"] = JsonSerializer.SerializeToElement(new { type = "integer", minimum = 1, maximum = 30000, description = "Maximum characters, default 12000; JSON escaping may shorten a page." }), ["id"] = JsonSerializer.SerializeToElement(new { type = "string", @@ -50,6 +52,8 @@ public string Execute(IReadOnlyDictionary input, UIApplicat "large results — re-run the original query instead."); // The model asked for the full text explicitly, but still bound the response. - return TextUtil.Truncate(content, 50_000); + return ToolResultCap.Page(id!, content, + input.TryGetValue("offset", out var offset) ? ToolInput.NonnegativeInteger(offset, "offset") : 0, + input.TryGetValue("limit", out var limit) ? ToolInput.NonnegativeInteger(limit, "limit") : 12_000); } } diff --git a/ClaudeRevit/Tools/GetProjectCatalog.cs b/ClaudeRevit/Tools/GetProjectCatalog.cs index 9e90326..b006d9c 100644 --- a/ClaudeRevit/Tools/GetProjectCatalog.cs +++ b/ClaudeRevit/Tools/GetProjectCatalog.cs @@ -22,7 +22,7 @@ public class GetProjectCatalog : IRevitTool "kind/discipline), loadable family types grouped by category, system types (walls, " + "floors, roofs), materials, and the full reinforcement catalog (bar types with " + "diameters, rebar shapes, hook types, cover types, area/path reinforcement types). " + - "Call this ONCE when starting work on a project instead of many separate list_* calls. " + + "Request sections and a family_category/query for the current task; use paging offsets for additional entries. " + "The result is cached for the session; pass refresh=true after loading new families or " + "creating new types."; @@ -30,6 +30,11 @@ public class GetProjectCatalog : IRevitTool { Properties = new Dictionary { + ["sections"] = JsonSerializer.SerializeToElement(new { type = "array", items = new { type = "string", @enum = new[] { "document", "levels", "view_templates", "family_types_by_category", "system_types", "materials", "reinforcement" } }, description = "Optional sections; request only assets needed for the current task." }), + ["family_category"] = JsonSerializer.SerializeToElement(new { type = "string", description = "Optional category substring for family_types_by_category, e.g. Structural Framing." }), + ["query"] = JsonSerializer.SerializeToElement(new { type = "string", description = "Optional substring filter within catalog lists." }), + ["offset"] = JsonSerializer.SerializeToElement(new { type = "integer", minimum = 0, description = "Default 0; list continuation offsets are in paging." }), + ["limit"] = JsonSerializer.SerializeToElement(new { type = "integer", minimum = 1, maximum = 300, description = "Default 40 per list." }), ["refresh"] = JsonSerializer.SerializeToElement(new { type = "boolean", @@ -50,6 +55,7 @@ public class GetProjectCatalog : IRevitTool // not outlive type/family changes made by the assistant itself. (User edits and Ctrl+Z // are still invisible; the refresh flag remains the manual escape hatch.) public static void Invalidate() => Cache.Clear(); + public static void Invalidate(Document doc) => Cache.TryRemove(DocKey.For(doc), out _); public string Execute(IReadOnlyDictionary input, UIApplication app) { @@ -68,24 +74,13 @@ public string Execute(IReadOnlyDictionary input, UIApplicat } // One serialization site so the cached and fresh response shapes can't diverge. - return Services.Json.Serialize(new - { - cached, - note = cached - ? "Served from the session cache — pass refresh=true if families/types changed " + - "outside this assistant (its own tool calls invalidate the cache automatically)." - : null, - catalog - }); + var response = Services.CatalogPage.Select(catalog, + input.TryGetValue("sections", out var sections) ? sections.EnumerateArray().Select(s => s.GetString()!).ToArray() : null, + NativeToolUtil.Text(input, "query"), ToolInput.OptionalInt(input, "offset") ?? 0, ToolInput.OptionalInt(input, "limit") ?? 40, NativeToolUtil.Text(input, "family_category")); + response["cached"] = cached; + return response.ToJsonString(Services.ToolResult.Options); } - // Lists are capped so a big production model can't blow up the response (and the - // conversation context) — the dedicated list_* tools return full data when needed. - private static List Cap(List items, int max) => - items.Count <= max - ? items - : items.Take(max).Append($"… +{items.Count - max} more (use the dedicated list tool for the full set)").ToList(); - private static JsonElement BuildCatalog(Document doc) { var levels = new FilteredElementCollector(doc).OfClass(typeof(Level)).Cast() @@ -102,7 +97,7 @@ private static JsonElement BuildCatalog(Document doc) .OrderBy(g => g.Key) .ToDictionary( g => g.Key, - g => Cap(g.Select(s => $"{s.FamilyName}: {s.Name}").OrderBy(n => n).ToList(), 60)); + g => g.Select(s => $"{s.FamilyName}: {s.Name}").OrderBy(n => n).ToList()); var wallTypes = new FilteredElementCollector(doc).OfClass(typeof(WallType)) .Select(t => t.Name).OrderBy(n => n).ToList(); @@ -111,8 +106,8 @@ private static JsonElement BuildCatalog(Document doc) var roofTypes = new FilteredElementCollector(doc).OfClass(typeof(RoofType)) .Select(t => t.Name).OrderBy(n => n).ToList(); - var materials = Cap(new FilteredElementCollector(doc).OfClass(typeof(Material)) - .Select(m => m.Name).OrderBy(n => n).ToList(), 150); + var materials = new FilteredElementCollector(doc).OfClass(typeof(Material)) + .Select(m => m.Name).OrderBy(n => n).ToList(); var barTypes = new FilteredElementCollector(doc).OfClass(typeof(RebarBarType)).Cast() .Select(t => new { name = t.Name, diameter_mm = Math.Round(t.BarNominalDiameter * Units.MmPerFoot, 1) }) diff --git a/ClaudeRevit/Tools/LiveSpds.cs b/ClaudeRevit/Tools/LiveSpds.cs index 985b05a..feb5a63 100644 --- a/ClaudeRevit/Tools/LiveSpds.cs +++ b/ClaudeRevit/Tools/LiveSpds.cs @@ -12,6 +12,8 @@ namespace ClaudeRevit.Tools; internal sealed class LiveSpdsRecord { public string Key {get;set;}=""; + public string? LastRefreshError {get;set;} + public string? LastRefreshUtc {get;set;} public string Profile {get;set;}=""; public long CategoryId {get;set;} public long[] CategoryIds {get;set;}=[]; @@ -69,16 +71,27 @@ public void Execute(UpdaterData data) try { var doc=data.GetDocument();if(doc.IsFamilyDocument)return; - var records=LiveSpds.All(doc).Select(x=>x.Record).ToList(); + var records=LiveSpds.All(doc).ToList(); if(records.Count==0)return; var changed=data.GetModifiedElementIds().Concat(data.GetAddedElementIds()).Select(doc.GetElement).Where(e=>e?.Category!=null).Select(e=>e!.Category.Id.Value).ToHashSet(); bool deleted=data.GetDeletedElementIds().Count>0; bool materialsChanged=data.GetModifiedElementIds().Select(doc.GetElement).Any(e=>e is Material or PropertySetElement); - foreach(var record in records) + foreach(var (storage,record) in records) { if(!(deleted||materialsChanged||record.Categories.Any(changed.Contains)))continue; - try{LiveSpds.Refresh(doc,record,false);} - catch(Exception ex){Services.Log.Error($"Live SPDS table '{record.Key}' could not refresh; its quantities are stale until the table is refreshed (create_spds_table)",ex);} + try + { + var refresh = LiveSpds.Refresh(doc,record,false); + record.LastRefreshError = refresh.Errors.Count == 0 ? null : $"{refresh.Errors.Count} sources have incomplete data."; + record.LastRefreshUtc = DateTime.UtcNow.ToString("o"); + LiveSpds.Save(storage,record); + } + catch(Exception ex) + { + record.LastRefreshError = ex.Message; + LiveSpds.Save(storage,record); + Services.Log.Error($"Live SPDS table '{record.Key}' could not refresh; audit_spds_schedule reports its stale state",ex); + } } } catch(Exception ex){Services.Log.Error("Live SPDS updater failed; the user's edit was kept",ex);} diff --git a/ClaudeRevit/Tools/ModelChangeCapture.cs b/ClaudeRevit/Tools/ModelChangeCapture.cs index c36462d..d022aa4 100644 --- a/ClaudeRevit/Tools/ModelChangeCapture.cs +++ b/ClaudeRevit/Tools/ModelChangeCapture.cs @@ -20,7 +20,22 @@ private sealed record Info(string Category,string Representation); private readonly Document _doc; private readonly HashSet _added=new(),_modified=new(),_deleted=new(); private readonly Dictionary _seen=new(); - private static Info Snapshot(Element? e)=>new(e?.Category?.Name??"(no category)",e==null?"deleted":ModelProvenance.Representation(e)); + private static Info Snapshot(Element? e) + { + try { return new(e?.Category?.Name ?? "(no category)", e == null ? "deleted" : ModelProvenance.Representation(e)); } + catch { return new("(unavailable)", "(unavailable)"); } + } + private object Readback(long id) + { + try + { + var element = _doc.GetElement(new ElementId(id)); var box = element?.get_BoundingBox(null); + return new { id, exists = element != null, category = element?.Category?.Name, + representation = element == null ? "missing" : ModelProvenance.Representation(element), + bounds_mm = box == null ? null : new[] { (box.Max.X-box.Min.X)*Units.MmPerFoot, (box.Max.Y-box.Min.Y)*Units.MmPerFoot, (box.Max.Z-box.Min.Z)*Units.MmPerFoot } }; + } + catch (Exception ex) { return new { id, status = "incomplete", reason = ex.Message }; } + } public ModelChangeCapture(Document doc,string key){_doc=doc;_current=this;} // What the finished call left in the model (zero when it rolled back), for the action history. public int AddedCount{get;private set;} @@ -35,13 +50,20 @@ public static void Changed(DocumentChangedEventArgs e) UndoSequencer.Changed(e); var c=_current; // Edits outside a tool call (by hand, other add-ins, undo/redo) invalidate pending plans. - if((c==null||!Services.DocumentSessions.Same(c._doc,e.GetDocument()))&&Services.DocumentSessions.ExistingKey(e.GetDocument()) is { } changedKey)WritePlans.Bump(changedKey); + if((c==null||!Services.DocumentSessions.Same(c._doc,e.GetDocument()))&&Services.DocumentSessions.ExistingKey(e.GetDocument()) is { } changedKey) + { + WritePlans.Bump(changedKey); + GetProjectCatalog.Invalidate(e.GetDocument()); + // Manual edits/Undo/Redo destroy the correspondence to Revit's undo stack. + if (!UndoSequencer.Active) ActionHistory.Invalidate(changedKey); + } if(c==null||!Services.DocumentSessions.Same(c._doc,e.GetDocument()))return; foreach(var id in e.GetAddedElementIds()){c._added.Add(id.Value);c.Sample(id);} foreach(var id in e.GetModifiedElementIds()){c._modified.Add(id.Value);c.Sample(id);} foreach(var id in e.GetDeletedElementIds())c._deleted.Add(id.Value); } private void Sample(ElementId id){if(_seen.Count!_added.Contains(id)&&!deletedSet.Contains(id)&&Exists(id)).ToArray(); - Info InfoOf(long id)=>_seen.TryGetValue(id,out var i)?i:new("(not sampled)","(not sampled)"); + // Resolve only affected surviving elements, not the entire model. Counts by + // category must not silently become sampled counts on large batches. + foreach (var id in added.Concat(modified)) _seen[id] = Snapshot(_doc.GetElement(new ElementId(id))); + Info InfoOf(long id)=>_seen.TryGetValue(id,out var i)?i:new("(deleted category unknown)","deleted"); object ByCategory(long[] ids)=>ids.GroupBy(id=>InfoOf(id).Category).ToDictionary(g=>g.Key,g=>g.Count()); AddedCount=added.Length;ModifiedCount=modified.Length;DeletedCount=deleted.Length;_addedSample=added.Take(IdLimit).ToArray(); - return new { added_count=added.Length,modified_count=modified.Length,deleted_count=deleted.Length, + var receiptId = "changes_" + Guid.NewGuid().ToString("N"); + var detail = Services.Json.Serialize(new + { + added_ids = added, modified_ids = modified, deleted_ids = deleted, + rolled_back = rolledBack, units = "Revit internal: ft, kg; bounds below in mm" + }); + if (added.Length + modified.Length + deleted.Length > 0 || rolledBack) Services.ToolResultArchive.Record(receiptId, detail); + Receipt = new + { + added = added.Length, modified = modified.Length, deleted = deleted.Length, + rolled_back = rolledBack, details_id = receiptId, + details_truncated = detail.Length > Services.ToolResultArchive.MaxContentChars, + verification = "Changed IDs re-read after execution; dimensional/parameter expectations require verify_model_result.", + readback = added.Concat(modified).Take(8).Select(Readback).ToArray() + }; + return new { receipt_id=receiptId, added_count=added.Length,modified_count=modified.Length,deleted_count=deleted.Length, added_by_category=ByCategory(added),modified_by_category=ByCategory(modified), added_by_representation=added.GroupBy(id=>InfoOf(id).Representation).ToDictionary(g=>g.Key,g=>g.Count()), added_ids=added.Take(IdLimit).ToArray(),modified_ids=modified.Take(IdLimit).ToArray(),deleted_ids=deleted.Take(IdLimit).ToArray(), diff --git a/ClaudeRevit/Tools/NameResolve.cs b/ClaudeRevit/Tools/NameResolve.cs index 7d24193..250bdbb 100644 --- a/ClaudeRevit/Tools/NameResolve.cs +++ b/ClaudeRevit/Tools/NameResolve.cs @@ -19,13 +19,22 @@ public static T ByName(Document doc, string? name, string kind, var col = new FilteredElementCollector(doc).OfClass(typeof(T)); var all = (source != null ? source(col) : col.Cast()).ToList(); - var hit = all.FirstOrDefault(e => e.Name == name) - ?? all.FirstOrDefault(e => string.Equals(e.Name?.Trim(), name?.Trim(), - StringComparison.OrdinalIgnoreCase)); - if (hit != null) return hit; + var matches = all.Where(e => Matches(e, name)).ToArray(); + if (matches.Length == 1) return matches[0]; + if (matches.Length > 1) throw new ToolInputException($"Ambiguous {kind} '{name}'; use Family: Type or an explicit id. Matches: {string.Join(", ", matches.Select(Display))}"); + throw new InvalidOperationException(NotFound(name, kind, all.Select(Display))); + } - throw new InvalidOperationException(NotFound(name, kind, all.Select(e => e.Name))); + public static bool Matches(Element element, string? name) + { + if (string.IsNullOrWhiteSpace(name)) return false; + var requested = name.Trim(); + if (string.Equals(element.Name?.Trim(), requested, StringComparison.OrdinalIgnoreCase)) return true; + if (element is not FamilySymbol symbol) return false; + return string.Equals($"{symbol.FamilyName}: {symbol.Name}", requested, StringComparison.OrdinalIgnoreCase) || + string.Equals($"{symbol.FamilyName}:{symbol.Name}", requested, StringComparison.OrdinalIgnoreCase); } + private static string Display(Element element) => element is FamilySymbol symbol ? $"{symbol.FamilyName}: {symbol.Name}" : element.Name; // A ready-made exception for call sites that already did their own lookup. public static ToolInputException Missing(string? name, string kind, IEnumerable available) => diff --git a/ClaudeRevit/Tools/RunBatch.cs b/ClaudeRevit/Tools/RunBatch.cs index a2b540a..df8e068 100644 --- a/ClaudeRevit/Tools/RunBatch.cs +++ b/ClaudeRevit/Tools/RunBatch.cs @@ -106,10 +106,12 @@ public string Execute(IReadOnlyDictionary input, UIApplicat { sub.Start(); var raw = inner.Execute(args, app); - sub.Commit(); - results.Add(new { index, ok = true, result = AsJson(raw) }); - ok++; + var succeeded = Services.ToolResult.LooksSuccessful(raw); + if (succeeded) { sub.Commit(); ok++; } else sub.RollBack(); + results.Add(new { index, ok = succeeded, rolled_back = !succeeded, result = AsJson(raw) }); } + catch (OperationCanceledException) { throw; } + catch (Autodesk.Revit.Exceptions.RegenerationFailedException) { throw; } catch (Exception ex) { try { if (sub.HasStarted() && !sub.HasEnded()) sub.RollBack(); } catch { } diff --git a/ClaudeRevit/Tools/RunPython.cs b/ClaudeRevit/Tools/RunPython.cs index ddc9e60..5dfd742 100644 --- a/ClaudeRevit/Tools/RunPython.cs +++ b/ClaudeRevit/Tools/RunPython.cs @@ -150,14 +150,16 @@ public string Execute(IReadOnlyDictionary input, UIApplicat return Services.Json.Serialize(new { engine = _engineSource, + ok = traceback == null, succeeded = traceback == null, + error = traceback == null ? null : Trim(traceback, 4000), output = Describe(outValue), printed = string.IsNullOrEmpty(printed) ? null : Trim(printed, 8000), traceback = traceback == null ? null : Trim(traceback, 4000), document = doc?.Title, note = traceback == null ? "Ran in-process (no Dynamo). Changes are in this call's transaction — one Ctrl+Z undoes this script." - : "The script raised. Model changes made before the error may have been applied — " + + : "The script raised. The dispatcher rolls back this call's model transaction; external file/network effects cannot be rolled back. " + "the traceback is reported verbatim rather than re-running the snippet." }); } diff --git a/ClaudeRevit/Tools/ToolCatalog.cs b/ClaudeRevit/Tools/ToolCatalog.cs index 7b9f01d..781adcd 100644 --- a/ClaudeRevit/Tools/ToolCatalog.cs +++ b/ClaudeRevit/Tools/ToolCatalog.cs @@ -8,7 +8,7 @@ namespace ClaudeRevit.Tools; // The point is token budget: every request re-sends the full tool list (and on // OpenAI-compatible providers there is no prompt caching, so it is re-counted every turn), // which alone can blow a free tier's tokens-per-minute limit. Turning off the groups you -// don't need shrinks each request a lot. Categorisation is a coarse, class-name heuristic — +// don't need shrinks each request. Provider cache behavior varies. Categorisation is a coarse, class-name heuristic — // it only has to be good enough to disable big chunks (MEP, sheets, schedules, views…). public static class ToolCatalog { @@ -100,7 +100,7 @@ public static List PrewarmCategories(string prompt) => Services.ToolSearchLogic.Prewarm(prompt); // find_tools backend: adapt the registered tools to plain (name, desc, category, isCore) - // records and delegate the scoring/reveal to the pure logic. + // records and delegate bounded individual-tool selection to the pure logic. public static Services.ToolSearchLogic.SearchResult Search(IEnumerable allTools, string query) { var infos = allTools.Select(t => diff --git a/ClaudeRevit/Tools/ToolDispatcher.cs b/ClaudeRevit/Tools/ToolDispatcher.cs index 49dd23c..20cfa36 100644 --- a/ClaudeRevit/Tools/ToolDispatcher.cs +++ b/ClaudeRevit/Tools/ToolDispatcher.cs @@ -464,10 +464,19 @@ private void HandleTool(UIApplication app, ToolJob job) { var target = Services.DocumentSessions.Find(job.DocumentKey); if (target == null) { job.Tcs.TrySetResult(Services.ToolResult.Failure("validation", "No bound document. Nothing was undone.")); job.Operation.Dispose(); return; } - int? steps = job.Input.TryGetValue("steps", out var st) && st.ValueKind == JsonValueKind.Number ? Math.Max(1, st.GetInt32()) : null; - Services.Log.Info($"tool → undo_last steps={steps?.ToString() ?? "action"}"); - job.Tcs.Task.ContinueWith(_ => job.Operation.Dispose(), TaskScheduler.Default); - UndoSequencer.Start(app, target, job.DocumentKey, steps, job.Tcs); + try + { + int? steps = job.Input.ContainsKey("steps") ? ToolInput.RequiredInt(job.Input, "steps") : null; + if (steps is <= 0) throw new ToolInputException("steps must be a positive integer."); + Services.Log.Info($"tool → undo_last steps={steps?.ToString() ?? "action"}"); + job.Tcs.Task.ContinueWith(_ => job.Operation.Dispose(), TaskScheduler.Default); + UndoSequencer.Start(app, target, job.DocumentKey, steps, job.Tcs); + } + catch (Exception ex) + { + job.Tcs.TrySetResult(Services.ToolResult.Failure("validation", ex.Message)); + job.Operation.Dispose(); + } return; } if (UndoSequencer.Active) @@ -494,6 +503,7 @@ private void HandleTool(UIApplication app, ToolJob job) string? completedResult = null; Exception? completedError = null; bool cancelled = false; + bool executionRolledBack = false; try { if (boundDocument == null) throw new InvalidOperationException(job.DocumentKey == "none" @@ -554,11 +564,16 @@ private void HandleTool(UIApplication app, ToolJob job) { result = job.PreparedScript != null ? job.PreparedScript.ExecutePrepared(job.Input, app, job.PreparedBytes!) : tool.Execute(job.Input, app); job.Ct.ThrowIfCancellationRequested(); - var status = tx.Commit(); + // Logical failure is not an exception (notably Python traceback). + // Roll back BEFORE reporting it; changing only the JSON would leave + // partial edits behind and make retries unsafe. + var succeeded = Services.ToolResult.LooksSuccessful(result); + var status = succeeded ? tx.Commit() : tx.RollBack(); + executionRolledBack = !succeeded; // An unresolved error rolls the transaction back WITHOUT throwing — surface // it as a tool error so the model knows this step did NOT take effect. - if (status != TransactionStatus.Committed) + if (succeeded && status != TransactionStatus.Committed) throw new InvalidOperationException( "Revit rolled this operation back — it did NOT take effect" + (failures.Messages.Count > 0 @@ -569,6 +584,7 @@ private void HandleTool(UIApplication app, ToolJob job) // can verify the result rather than assume it was clean. if (failures.Messages.Count > 0) warnings.AddRange(failures.Messages); + result = Services.ToolResult.Execution(result, !executionRolledBack, executionRolledBack); } catch { @@ -587,7 +603,12 @@ private void HandleTool(UIApplication app, ToolJob job) { result = tool.Execute(job.Input, app); job.Ct.ThrowIfCancellationRequested(); - group?.Assimilate(); + if (group != null) + { + if (Services.ToolResult.LooksSuccessful(result)) group.Assimilate(); + else { group.RollBack(); executionRolledBack = true; } + result = Services.ToolResult.Execution(result, !executionRolledBack, executionRolledBack); + } } catch { if (group?.GetStatus() == TransactionStatus.Started) group.RollBack(); throw; } } @@ -640,13 +661,14 @@ private void HandleTool(UIApplication app, ToolJob job) catch (Exception ex) { Services.Log.Error("Document context update failed", ex); } finally { ToolContext.Clear(); } job.Channel?.RecordExecution(queueTime, executionWatch.Elapsed); - var rolledBack=cancelled || completedError != null || completedResult == null; + var rolledBack=executionRolledBack || cancelled || completedError != null || completedResult == null; if (completedResult != null) { try { using var json=JsonDocument.Parse(completedResult); if(json.RootElement.TryGetProperty("preview",out var preview)&&preview.ValueKind==JsonValueKind.True)rolledBack=true; + if(json.RootElement.TryGetProperty("rolled_back",out var rb)&&rb.ValueKind==JsonValueKind.True)rolledBack=true; if(job.Name.StartsWith("modplus_mpr",StringComparison.Ordinal)||job.Name=="run_modplus_headless") { if((json.RootElement.TryGetProperty("committed",out var committed)&&committed.ValueKind==JsonValueKind.False)|| @@ -657,13 +679,22 @@ private void HandleTool(UIApplication app, ToolJob job) } try { + object? delta; + using (Services.TaskJournal.MeasurePhase("post_write_readback")) delta = changes?.Complete(rolledBack); + if (completedResult != null && changes?.Receipt is { } receipt && + (changes.AddedCount + changes.ModifiedCount + changes.DeletedCount > 0 || executionRolledBack)) + { + var obj = System.Text.Json.Nodes.JsonNode.Parse(completedResult)!.AsObject(); + obj["receipt"] = System.Text.Json.JsonSerializer.SerializeToNode(receipt, Services.ToolResult.Options); + completedResult = Services.ToolResultCap.Apply(obj.ToJsonString(Services.ToolResult.Options), job.Name); + } Services.TaskJournal.RecordTiming(job.TaskId, queueTime.TotalSeconds, executionWatch.Elapsed.TotalSeconds); Services.TaskJournal.Append(new { kind="tool",utc=DateTime.UtcNow,task_id=job.TaskId,channel_id=job.Session?.ChannelId, document_key=job.DocumentKey,tool=job.Name,queue_seconds=queueTime.TotalSeconds,revit_seconds=executionWatch.Elapsed.TotalSeconds, // What the result costs the model: the one number that shows which tools overload the conversation. result_chars=completedResult?.Length??0, ok=!cancelled&&completedError==null&&completedResult!=null&&ResultLooksOk(completedResult),cancelled,error=completedError?.Message??Services.ToolResult.ErrorMessage(completedResult), - changes=changes?.Complete(rolledBack) }); + changes=delta }); if (changes != null && !rolledBack) ActionHistory.Record(job.DocumentKey, job.TaskId, job.Name, changes); } catch (Exception ex) { Services.Log.Error("Tool diagnostics failed",ex); } @@ -683,15 +714,7 @@ private static string SafeArgs(IReadOnlyDictionary input) // Script tool results are our own JSON with a top-level "ok"; absence means success. private static bool ResultLooksOk(string result) { - try - { - using var d = JsonDocument.Parse(result); - return !(d.RootElement.TryGetProperty("ok", out var o) && o.ValueKind == JsonValueKind.False); - } - catch - { - return true; - } + return Services.ToolResult.LooksSuccessful(result); } private void HandleGetContext(UIApplication app, GetContextJob job) diff --git a/ClaudeRevit/Tools/ToolInput.cs b/ClaudeRevit/Tools/ToolInput.cs index 2ec0043..bff8190 100644 --- a/ClaudeRevit/Tools/ToolInput.cs +++ b/ClaudeRevit/Tools/ToolInput.cs @@ -8,6 +8,13 @@ namespace ClaudeRevit.Tools; // Null-kind element, so absence and null must be treated the same. internal static class ToolInput { + public static int NonnegativeInteger(JsonElement value, string name) + { + var input = new Dictionary { [name] = value }; + var n = RequiredInt(input, name); + if (n < 0) throw new ToolInputException($"Parameter '{name}' must be nonnegative."); + return n; + } public static double? OptionalDouble(IReadOnlyDictionary input, string name) => input.TryGetValue(name, out var v) && v.ValueKind == JsonValueKind.Number ? v.GetDouble() : null; diff --git a/ClaudeRevit/Tools/TypeChangePreservation.cs b/ClaudeRevit/Tools/TypeChangePreservation.cs index 8ebb82b..c44a167 100644 --- a/ClaudeRevit/Tools/TypeChangePreservation.cs +++ b/ClaudeRevit/Tools/TypeChangePreservation.cs @@ -24,7 +24,8 @@ internal static bool IsTypeIdentity(Parameter p) => // deliberately stay on the selected type. Throw to roll back if a requested value is lost. public static (Element Element, List Changes) Change(Element source, ElementId target, bool preserve, string[]? names = null) { - var before=source.Parameters.Cast().Where(p=>p.HasValue && p.StorageType!=StorageType.None && !IsTypeIdentity(p)) + var before=source.Parameters.Cast().Where(p=>p.HasValue && !p.IsReadOnly && p.StorageType!=StorageType.None && !IsTypeIdentity(p) && + (names != null || p.Definition is not InternalDefinition d || d.BuiltInParameter != BuiltInParameter.INSTANCE_HEAD_HEIGHT_PARAM)) .Select(p=>new Value(p.Id.Value,p.IsShared?p.GUID.ToString():null,p.Definition.Name,p.Definition.GetDataType().TypeId,p.StorageType,Raw(p))).ToArray(); if(names!=null && names.Any(name=>!before.Any(p=>p.Name==name)))throw new ToolInputException("A preserve_parameters name is missing from the instance."); var doc=source.Document;var oldId=source.Id;var newId=source.ChangeTypeId(target); diff --git a/ClaudeRevit/Tools/UndoLast.cs b/ClaudeRevit/Tools/UndoLast.cs index 6ff5aad..34919fe 100644 --- a/ClaudeRevit/Tools/UndoLast.cs +++ b/ClaudeRevit/Tools/UndoLast.cs @@ -25,7 +25,7 @@ public static void Record(string documentKey, string? actionId, string tool, Mod var list = ByDocument.GetOrAdd(documentKey, _ => new()); lock (list) { - list.Add(new(actionId, tool, DateTime.UtcNow, changes.AddedCount, changes.ModifiedCount, changes.DeletedCount, changes.AddedSample)); + list.Add(new(actionId ?? Guid.NewGuid().ToString("N"), tool, DateTime.UtcNow, changes.AddedCount, changes.ModifiedCount, changes.DeletedCount, changes.AddedSample)); if (list.Count > Limit) list.RemoveRange(0, list.Count - Limit); } } @@ -36,6 +36,8 @@ public static List Snapshot(string documentKey) lock (list) return list.ToList(); } + public static void Invalidate(string documentKey) => ByDocument.TryRemove(documentKey, out _); + public static void Pop(string documentKey, int count) { if (!ByDocument.TryGetValue(documentKey, out var list)) return; @@ -100,7 +102,7 @@ internal sealed class UndoSequencer private readonly UIApplication _app; private readonly Document _doc; private readonly string _key; private readonly List _entries; private readonly TaskCompletionSource _tcs; private readonly List _undone = new(); - private int _done; private bool _waiting; private string? _foreign; private bool _redoPosted; private DateTime _deadline; + private int _done; private bool _waiting; private string? _foreign; private bool _redoPosted; private bool _redoConfirmed; private DateTime _deadline; private UndoSequencer(UIApplication app, Document doc, string key, List entries, TaskCompletionSource tcs) { _app = app; _doc = doc; _key = key; _entries = entries; _tcs = tcs; } @@ -115,11 +117,13 @@ public static void Start(UIApplication app, Document doc, string key, int? steps var s = new UndoSequencer(app, doc, key, entries, tcs); _active = s; app.Idling += s.OnIdling; - s.PostUndo(); + try { s.PostUndo(); } catch (Exception ex) { s.Finish("Cannot post Undo: " + ex.Message); } } private void PostUndo() { + if (!Services.DocumentSessions.Same(_doc, _app.ActiveUIDocument?.Document)) + throw new InvalidOperationException("The active document changed; no further Undo was posted."); _waiting = true; _deadline = DateTime.UtcNow.AddSeconds(20); _app.PostCommand(RevitCommandId.LookupPostableCommandId(PostableCommand.Undo)); } @@ -129,9 +133,11 @@ public static void Changed(DocumentChangedEventArgs e) { var s = _active; if (s == null || !s._waiting || !Services.DocumentSessions.Same(s._doc, e.GetDocument())) return; + if (s._redoPosted && e.Operation == UndoOperation.TransactionRedone) + { s._redoConfirmed = true; s._waiting = false; return; } if (e.Operation != UndoOperation.TransactionUndone) return; var names = e.GetTransactionNames(); - var theirs = names.FirstOrDefault(n => !n.StartsWith("Claude", StringComparison.Ordinal)); + var theirs = names.Count == 0 ? "(unnamed transaction)" : names.FirstOrDefault(n => !n.StartsWith("Claude", StringComparison.Ordinal)); if (theirs != null) s._foreign = theirs; else { s._done++; s._undone.AddRange(names); } s._waiting = false; @@ -141,6 +147,8 @@ private void OnIdling(object? sender, IdlingEventArgs e) { try { + if (!Services.DocumentSessions.Same(_doc, _app.ActiveUIDocument?.Document)) + { Finish("Active document changed; the sequence stopped. Inspect the original document's Undo/Redo state."); return; } if (_waiting) { if (DateTime.UtcNow > _deadline) Finish("Revit did not perform Undo (a dialog may be open, or the document is not active)."); @@ -148,8 +156,12 @@ private void OnIdling(object? sender, IdlingEventArgs e) } if (_foreign != null) { - if (!_redoPosted) { _redoPosted = true; _app.PostCommand(RevitCommandId.LookupPostableCommandId(PostableCommand.Redo)); return; } - Finish($"Stopped: the next undo step was not the agent's ('{_foreign}') — it was restored with Redo."); + if (!_redoPosted) + { + _redoPosted = true; _waiting = true; _deadline = DateTime.UtcNow.AddSeconds(20); + _app.PostCommand(RevitCommandId.LookupPostableCommandId(PostableCommand.Redo)); return; + } + Finish($"Stopped at a foreign undo step ('{_foreign}'); Redo confirmed: {_redoConfirmed}."); return; } if (_done < _entries.Count) { PostUndo(); return; } diff --git a/ClaudeRevit/Tools/Units.cs b/ClaudeRevit/Tools/Units.cs index 03459db..ab02b84 100644 --- a/ClaudeRevit/Tools/Units.cs +++ b/ClaudeRevit/Tools/Units.cs @@ -18,4 +18,7 @@ public static class Units public static double CuFeetToCuM(double cuFeet) => cuFeet * CuMPerCuFoot; public static double CuMToCuFeet(double cuM) => cuM / CuMPerCuFoot; + + // Revit mass is kg and length is feet: internal density is kg/ft³. + public static double DensityToKgM3(double kgPerCuFoot) => kgPerCuFoot / CuMPerCuFoot; } diff --git a/ClaudeRevit/Tools/UnloadFamily.cs b/ClaudeRevit/Tools/UnloadFamily.cs index efe02c0..ea5e78b 100644 --- a/ClaudeRevit/Tools/UnloadFamily.cs +++ b/ClaudeRevit/Tools/UnloadFamily.cs @@ -14,7 +14,7 @@ public class UnloadFamily : IRevitTool public string Description => "Removes a loaded family from the document. All instances of this family must already be deleted, " + - "or the deletion will fail. Use list_loaded_families to find the family_id."; + "otherwise the operation is refused before deletion. Use list_loaded_families to find the family_id."; public InputSchema InputSchema => new() { @@ -36,7 +36,20 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var family = doc.GetElement(id) as Family ?? throw new InvalidOperationException($"Element {id.Value} is not a Family."); - var familyName = family.Name; + var symbols = family.GetFamilySymbolIds().ToHashSet(); + var instances = new FilteredElementCollector(doc).OfClass(typeof(FamilyInstance)).Cast() + .Where(i => symbols.Contains(i.GetTypeId())).Select(i => i.Id.Value).ToArray(); + if (instances.Length > 0) throw new ToolInputException($"Family has {instances.Length} placed instances; nothing was deleted."); + var allowed = symbols.Append(id).ToHashSet(); + using (var preview = new SubTransaction(doc)) + { + preview.Start(); + var cascade = doc.Delete(id).ToArray(); + preview.RollBack(); + if (cascade.Any(x => !allowed.Contains(x))) + throw new ToolInputException("Family deletion would remove other dependent elements; nothing was deleted."); + } + var familyName = ((Family)doc.GetElement(id)).Name; var deleted = doc.Delete(id); return Services.Json.Serialize(new diff --git a/ClaudeRevit/Tools/WritePlans.cs b/ClaudeRevit/Tools/WritePlans.cs index d0dc5ea..0b9e68f 100644 --- a/ClaudeRevit/Tools/WritePlans.cs +++ b/ClaudeRevit/Tools/WritePlans.cs @@ -56,6 +56,7 @@ public static string Annotate(string documentKey, string tool, IReadOnlyDictiona JsonObject? obj; try { obj = JsonNode.Parse(result) as JsonObject; } catch (JsonException) { return result; } if (obj == null) return result; + if (!Services.ToolResult.LooksSuccessful(result)) return result; var isPreview = obj["preview"] is JsonValue pv && pv.TryGetValue(out var b) && b; if (isPreview) { diff --git a/ClaudeRevit/UI/MarkdownProps.cs b/ClaudeRevit/UI/MarkdownProps.cs index 49fb481..86b60ef 100644 --- a/ClaudeRevit/UI/MarkdownProps.cs +++ b/ClaudeRevit/UI/MarkdownProps.cs @@ -59,8 +59,8 @@ private static void OnMarkdownChanged(DependencyObject d, DependencyPropertyChan return; } - timer.Stop(); - timer.Start(); + // Throttle rather than debounce: continuous streaming must still render. + if (!timer.IsEnabled) timer.Start(); } catch (Exception ex) { diff --git a/README.md b/README.md index c9e2f8e..90be438 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,10 @@ See [`ClaudeRevit/Tools/`](ClaudeRevit/Tools) for tool implementations and share --- +## Audit fixes and validation + +Result/rollback integrity, paged archives and catalogs, metric conversions, guarded cleanup/Undo, bounded discovery, streaming images/markdown and phase diagnostics are documented in [the audit validation notes](docs/audit-fixes-2026-10-20.md). New native fixtures are compiled separately; their live Revit acceptance status is explicit. + ## License MIT — do whatever you want with it. diff --git a/README.ru.md b/README.ru.md index 4331d21..f9c2836 100644 --- a/README.ru.md +++ b/README.ru.md @@ -291,6 +291,10 @@ Workflow `.github/workflows/release.yml` собирает все три верс --- +## Исправления по аудиту + +Сохранение статусов ошибок и отката, постраничные архивы и каталоги, единицы, проверка удаления и Undo, поиск инструментов, обновление чата и журнал этапов описаны в [протоколе проверки](docs/audit-fixes-2026-10-20.md). Новые нативные примеры требуют отдельного выполнения в Revit; сборка не считается их прохождением. + ## Лицензия MIT — делайте что хотите. diff --git a/docs/audit-fixes-2026-10-20.md b/docs/audit-fixes-2026-10-20.md new file mode 100644 index 0000000..1ef55d0 --- /dev/null +++ b/docs/audit-fixes-2026-10-20.md @@ -0,0 +1,63 @@ +# Audit fixes and validation — 20 October 2026 + +Baseline: `main` commit `24d62d5dc8f7013c1830b1c5287f1dae03f42f19` (15 October). +The supplied interim report contains 168 entries (1 critical, 50 high, 103 medium, 14 low), including overlaps and unverified native hypotheses. This change implements the confirmed result-safety, units, cleanup, discovery and observability recommendations; it does not certify all 168 entries as independent defects or resolved findings. + +## Result and execution contract + +- Completion recognizes `succeeded:false` and legacy `error` results. Batch integer `ok` counts remain integers; `status` distinguishes success, partial and failure. +- Transactional logical failures roll back before responding. Failed items in `run_batch` roll back their subtransaction while successful items remain in the batch. Script transaction groups also roll back on logical failures. This protects Revit model changes, not external file/network effects of arbitrary code. +- Results and aged history stay one JSON value. Truncation preserves failure, preview, commit and rollback control fields. Large diagnostics retain an archive reference. +- `get_full_result` returns JSON pages: `content`, `offset`, `next_offset`, `has_more`, `total_chars`. Follow the returned offset. Each serialized page fits the response ceiling, including escaped characters and emoji. +- Archive entries retain up to 2,000,000 characters; `archive_truncated` explicitly identifies larger originals. Memory and disk are bounded. Archives are historical evidence, not a substitute for re-querying edited model elements. + +## Model correctness and deletion + +- Structural density uses internal kg/ft³, not lb/ft³. Light materials are not rejected by a fabricated minimum density. +- `derive_parameters` converts millimetres/m²/m³ for matching measurable target specs. Number/Text targets retain source values; incompatible measurable dimensions are rejected. +- Requested wall/floor thickness means total compound width; the edited core width accounts for retained finish layers. Nonpositive core widths fail. +- Default type preservation excludes read-only values and derived head height; explicit named preservation remains available. +- Cleanup uses Revit's native Purge Unused candidates. It retains at least one family type and treats location-less rooms as unplaced. Non-linked imports require explicit selection; being non-linked does not mean an import is exploded or disposable. +- Every candidate deletion is rehearsed in a subtransaction. Cascades outside the candidate set are reported and rolled back on apply as well as preview. Family unloading checks placed instances and dependency cascades before deletion. +- Undo arguments accept integral `1.0` and string integers with validation, never silently turn invalid input into a whole-action undo. Unscoped calls get independent action IDs. External edits/manual Undo/Redo invalidate agent history. Each posted step checks the active document; restoration of a foreign step waits for the Redo event. + +## Context and responsiveness + +- Direct API discovery reveals individual tools, at most 12 per search; exact names reveal one. Ambiguous Russian grid/section-size stems no longer prewarm reinforcement or family authoring. Disabled tools are excluded. +- Subscription MCP discovery keeps its existing compact gateway; API discovery and subscription timings are different execution profiles. +- Codex driving rules are included in the initial prompt, not duplicated in every resumed prompt. MCP initialization continues to deliver current session instructions. +- Catalog lists retain complete cached data and expose bounded pages/counts, section selection, family-category and substring filters. External edits invalidate the affected document cache. +- Cyrillic headless results use Unicode serialization and a serialized-size budget. +- Prepared PNG attachments fit the actual inline base64 budget. Markdown throttles continuous deltas instead of postponing rendering until streaming stops. +- Optional closing suggestions do not trigger automatic continuation. Benchmark API round limits use the benchmark's configured budget rather than a smaller pane setting. + +## Evidence and diagnostics + +Every changed call returns a receipt with exact change counts, a paged archive of changed IDs, and a small post-write readback sample (existence/category/representation/bounds in mm). Category counts resolve affected surviving elements rather than labelling the unsampled remainder as counted geometry. Deleted categories that cannot be known remain explicitly unknown. + +A receipt proves only the readback scope. Before claiming dimensional or parameter correctness, use `verify_model_result` with declared expected values; it already performs independent native checks. No generic success flag certifies engineering adequacy or drawing compliance. + +Task journals expose authorization-process and API-provider request counts/times plus post-write readback time, in addition to existing total, queue, Revit, worker and token/cache metrics. Phase sums may overlap; provider time includes network/server/streaming and must not be presented as pure model reasoning time. Failed journal writes are surfaced in diagnostics. SPDS updater failures/incomplete sources persist in the model and are exposed by `audit_spds_schedule`. + +## Validation + +Local checks on Windows: **412 unit tests passed**, isolated pane/MCP/CLI checks passed, **51 ModPlus adapter checks passed**, builds for Revit 2025/2026/2027 and native-harness compilation succeeded (existing compiler warnings remain). No production project was modified, no live model calls were billed, and native Revit execution remains pending. + +Pure regression tests exercise failure preservation, escaped/Unicode paging, archive retention, Cyrillic budgeting, density including light materials, integer parsing, selective tool discovery, catalog paging, aged result structure and scoped phase accounting. WPF checks use the real markdown renderer and a dense image fixture. Transport/CLI checks use isolated fake clients/processes and no paid model calls. + +`ClaudeRevit.NativeTests` adds four independent `review_fixes` cases: + +| Case | Independent expected result | +| --- | --- | +| `audit_total_wall_thickness` | 20 + editable core + 30 mm totals 300 mm | +| `audit_derived_length_internal_units` | A wall's measured length copied into a Length parameter remains the same number of internal feet | +| `audit_material_density_7850` | A 1 m³ solid with structural steel density weighs 7850 kg | +| `audit_placed_unbounded_room_survives_cleanup` | A placed room with zero area remains after cleaning unplaced rooms | + +Compilation is not execution: these new native cases still require Revit 2027. Also run live acceptance for Python exception rollback, painted/nested references during purge, family unload cascades, Undo/Redo interleaved with manual edits/tab switching, and SPDS refresh recovery. Use disposable RVT/RFA fixtures, not production models. + +## Claims intentionally not converted into speculative fixes + +Physical SPDS text height requires an independent exported-PDF calibration. Auth configuration-file churn needs traces; no weakening of subscription authentication was introduced. Exact token/cost savings require measured backend-specific baselines. External repository candidate counts need source lists and deduplication; existence of a repository is not a reviewed implementation. ElementLevelFilter is a native slow filter, not a quick filter. + +For each remaining report entry record: source commit, Revit build, backend/model, fixture hash, exact call, expected/actual result, reproduction status, severity/confidence, duplicate parent and native evidence. Measure before/after using identical seed/model/limits; keep quality checks separate from speed. diff --git a/docs/benchmark.md b/docs/benchmark.md index 21fc55c..6de2ba5 100644 --- a/docs/benchmark.md +++ b/docs/benchmark.md @@ -59,13 +59,12 @@ quality. Use `scoring_version=quality-speed-v1` to distinguish new records. ## Latency and subscription tool discovery (v3.7.4) -The pane and benchmark CLI use `execution_profile=compact_mcp_v1`: 13 common +The pane and benchmark CLI use `execution_profile=compact_mcp_v2`: 13 common native tools plus `discover_revit_tools` and `invoke_revit_tool`. Discovery returns up to five matching input schemas with a next-page offset; the gateway -uses the ordinary document-bound dispatcher, always-enabled code execution and cancellation. +uses the ordinary document-bound dispatcher, the code-execution permission setting and cancellation. Specialised family/rebar/standards tools remain available. Ordinary external MCP -clients still receive the full catalogue. API mode uses its existing progressive -group loading and records `api_progressive_v1`. +clients still receive the full catalogue. API mode reveals up to 12 individual tools per discovery (one for an exact name) and records `api_selective_v2`. Do not combine these runs with older execution profiles when comparing speed. The prompt supplies actual level IDs/elevations, active view and selection. Agents query only missing facts instead of surveying the whole project for every From 6f104f231b6f811bba623a206bf33a45454f2be8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 04:39:40 +0000 Subject: [PATCH 2/3] Fix review findings in tool discovery, SPDS updater, headless budget and archive cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Discovery: a group named only by a keyword (a Russian request gives English descriptions nothing to rank by) is loaded whole instead of its first 12 tools in registration order; every keyword group in the prompt is loaded; ranked matches beyond 12 are listed by name. - Keyword matching accepts common Russian verb prefixes (образмерь, замаркируй, сгруппируй, заармируй), splits keys like the text so "технико-эконом" and "СП 1.13130" match, treats quotes, guillemets and brackets as separators, and restores English welded/wire mesh triggers. - Live SPDS updater writes its DataStorage only when the refresh state changes, so ordinary edits no longer modify it on every transaction (ownership conflicts in workshared models). - Headless results are re-encoded without \uXXXX escapes before the budget is measured; the test now uses the production serializer. - Archive hot cache is a real LRU; Dictionary slot reuse made it evict the newest entry. 14 new regression tests, each failing on the previous revision. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RCGB6xWx3ZtmWZRQEJtN6B --- ClaudeRevit.Tests/AuditSafetyTests.cs | 78 +++++++++++++++++++- ClaudeRevit/Services/HeadlessResultLimits.cs | 10 ++- ClaudeRevit/Services/ToolResultArchive.cs | 42 +++++++++-- ClaudeRevit/Services/ToolSearchLogic.cs | 70 +++++++++++++----- ClaudeRevit/Tools/LiveSpds.cs | 19 +++-- docs/audit-fixes-2026-10-20.md | 10 ++- 6 files changed, 193 insertions(+), 36 deletions(-) diff --git a/ClaudeRevit.Tests/AuditSafetyTests.cs b/ClaudeRevit.Tests/AuditSafetyTests.cs index 7b1f9b3..3ea043a 100644 --- a/ClaudeRevit.Tests/AuditSafetyTests.cs +++ b/ClaudeRevit.Tests/AuditSafetyTests.cs @@ -126,13 +126,25 @@ public void PreviewAndPartialStatusesAreDistinctFromCommittedSuccess() [Fact] public void CyrillicFitsHeadlessBudgetWithoutAsciiExpansion() { - var raw = new JsonObject { ["ok"] = true, ["text"] = new string('Я', 40_000) }.ToJsonString(ToolResult.Options); + // Production callers use the default serializer, which escapes every Cyrillic character. + var raw = System.Text.Json.JsonSerializer.Serialize(new { ok = true, text = new string('Я', 40_000) }); + Assert.Contains("\\u042F", raw); var result = HeadlessResultLimits.Compact(raw); Assert.True(result.Length <= 30_000); Assert.DoesNotContain("\\u042F", result); Assert.Contains(new string('Я', 9000), result); } + [Fact] + public void EscapedCyrillicUnderTheBudgetIsNotTruncated() + { + // 20 000 characters of text serialize to 120 000 escaped characters, yet fit as text. + var raw = System.Text.Json.JsonSerializer.Serialize(new { ok = true, text = new string('Я', 20_000) }); + var result = HeadlessResultLimits.Compact(raw); + Assert.DoesNotContain("\"truncated\"", result); + Assert.Contains(new string('Я', 20_000), result); + } + [Theory] [InlineData(7850)] [InlineData(2500)] @@ -169,6 +181,70 @@ public void InvalidIntegersAreValidationErrors(string raw) public void AxisGridAndSectionSizeDoNotLoadRebarOrFamilyAuthoring() => Assert.Empty(ToolSearchLogic.Prewarm("Создай колонны сечением 400х400 по сетке осей")); + [Theory] + [InlineData("образмерь стены на плане", "Annotation")] + [InlineData("замаркируй все двери", "Annotation")] + [InlineData("сгруппируй эти элементы", "Groups")] + [InlineData("разгруппируй группу", "Groups")] + [InlineData("заармируй плиту перекрытия", "Rebar")] + [InlineData("посчитай технико-экономические показатели", "Documentation")] + [InlineData("проверь эвакуацию по СП 1.13130", "Documentation")] + [InlineData("lay welded mesh on the slab", "Rebar")] + [InlineData("открой вид «Разрез 1»", "Views")] + public void PrefixedRussianVerbsAndPunctuatedKeysStillPrewarm(string prompt, string expected) + => Assert.Contains(expected, ToolSearchLogic.Prewarm(prompt)); + + private static ToolSearchLogic.ToolInfo[] LargeRebarAndViewsCatalog() => + Enumerable.Range(0, 27).Select(i => new ToolSearchLogic.ToolInfo($"rebar_tool_{i:00}", "Places reinforcement.", "Rebar", false)) + .Append(new ToolSearchLogic.ToolInfo("create_stirrup_zones", "Places stirrups by zones.", "Rebar", false)) + .Concat(Enumerable.Range(0, 20).Select(i => new ToolSearchLogic.ToolInfo($"view_tool_{i:00}", "Creates a view.", "Views", false))) + .ToArray(); + + [Fact] + public void RussianKeywordOnlyQueryLoadsTheWholeGroupInsteadOfTheFirstTwelve() + { + // English descriptions give a Russian query nothing to rank by; the group must stay reachable. + var result = ToolSearchLogic.Search(LargeRebarAndViewsCatalog(), "хомуты по зонам в балке"); + Assert.Contains("create_stirrup_zones", result.ToolNames); + Assert.Equal(28, result.ToolNames.Count); + } + + [Fact] + public void TwoKeywordGroupsInOnePromptAreBothLoaded() + { + var result = ToolSearchLogic.Search(LargeRebarAndViewsCatalog(), "добавь арматуру в колонну и сделай разрез"); + Assert.Contains("Rebar", result.Categories); + Assert.Contains("Views", result.Categories); + Assert.Contains("rebar_tool_00", result.ToolNames); + Assert.Contains("view_tool_00", result.ToolNames); + } + + [Fact] + public void RankedSearchNamesTheMatchesItDidNotLoad() + { + var result = ToolSearchLogic.Search(LargeRebarAndViewsCatalog(), "rebar tool"); + Assert.Equal(12, result.ToolNames.Count); + Assert.Contains("Also matched but not loaded", result.Message); + var omitted = Enumerable.Range(0, 27).Select(i => $"rebar_tool_{i:00}").Except(result.ToolNames).ToList(); + Assert.NotEmpty(omitted); + Assert.All(omitted, name => Assert.Contains(name, result.Message)); + } + + [Fact] + public void ArchiveCacheKeepsTheNewestEntriesAndEvictsTheOldest() + { + var prefix = "lru_" + Guid.NewGuid().ToString("N") + "_"; + for (var i = 0; i < 310; i++) ToolResultArchive.Record(prefix + i, "content " + i); + Assert.True(ToolResultArchive.IsCached(prefix + 309)); + Assert.True(ToolResultArchive.IsCached(prefix + 308)); + Assert.False(ToolResultArchive.IsCached(prefix + 0)); + // A lookup refreshes recency, so a re-read entry survives the next wave of inserts. + Assert.Equal("content 20", ToolResultArchive.Lookup(prefix + 20)); + for (var i = 310; i < 320; i++) ToolResultArchive.Record(prefix + i, "content " + i); + Assert.True(ToolResultArchive.IsCached(prefix + 20)); + Assert.True(ToolResultArchive.IsCached(prefix + 319)); + } + [Fact] public void ExactDiscoveryLoadsOneSchemaAndVagueVerbLoadsNone() { diff --git a/ClaudeRevit/Services/HeadlessResultLimits.cs b/ClaudeRevit/Services/HeadlessResultLimits.cs index 6f040b3..a5b2dc0 100644 --- a/ClaudeRevit/Services/HeadlessResultLimits.cs +++ b/ClaudeRevit/Services/HeadlessResultLimits.cs @@ -10,8 +10,16 @@ public static class HeadlessResultLimits { Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; public static string Compact(string raw) { + if (raw.Length <= 30_000 && !raw.Contains("\\u", StringComparison.Ordinal)) return raw; + // Callers serialize with the default encoder, which writes each Cyrillic character as a + // six-character \uXXXX escape. Re-encode first, so the budget counts real text and the + // head is readable instead of carrying literal escapes. + JsonNode? parsed; + try { parsed = JsonNode.Parse(raw); } + catch (System.Text.Json.JsonException) when (raw.Length <= 30_000) { return raw; } + raw = parsed?.ToJsonString(Options) ?? raw; if (raw.Length <= 30_000) return raw; - var source = JsonNode.Parse(raw) as JsonObject ?? throw new InvalidOperationException("Expected a structured headless result."); + var source = parsed as JsonObject ?? throw new InvalidOperationException("Expected a structured headless result."); var result = new JsonObject { ["truncated"] = true, ["total_chars"] = raw.Length }; foreach (var name in new[] { "ok", "preview", "committed", "rolledBack", "rejected", "idsTemporary", "modelModified", "module", "operation", "error", "errorCode" }) if (source[name] is { } value) result[name] = value.ToJsonString(Options).Length <= 2_000 ? value.DeepClone() : JsonValue.Create(value.ToJsonString(Options)[..2_000] + "…"); diff --git a/ClaudeRevit/Services/ToolResultArchive.cs b/ClaudeRevit/Services/ToolResultArchive.cs index f965a06..2201056 100644 --- a/ClaudeRevit/Services/ToolResultArchive.cs +++ b/ClaudeRevit/Services/ToolResultArchive.cs @@ -17,7 +17,12 @@ public static class ToolResultArchive private const int KeptEntriesOnTrim = 300; private static readonly object Gate = new(); - private static readonly Dictionary Memory = new(); + // Hot cache in least-recently-used order (oldest first), with its total size kept alongside. + // A plain Dictionary cannot serve here: Remove frees a slot, the next insert reuses it, and + // enumeration follows slots, so "Keys.First()" would evict the entry just added. + private static readonly Dictionary> Memory = new(); + private static readonly LinkedList<(string Id, string Content)> Recency = new(); + private static long _memoryChars; private static string FilePath => Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), @@ -32,8 +37,7 @@ public static void Record(string id, string content) { lock (Gate) { - Memory[id] = content; - TrimMemory(); + Remember(id, content); Directory.CreateDirectory(Path.GetDirectoryName(FilePath)!); File.AppendAllText(FilePath, JsonSerializer.Serialize(new { id, content }) + "\n"); @@ -50,7 +54,12 @@ public static void Record(string id, string content) { lock (Gate) { - if (Memory.TryGetValue(id, out var hit)) return hit; + if (Memory.TryGetValue(id, out var hit)) + { + Recency.Remove(hit); + Recency.AddLast(hit); + return hit.Value.Content; + } // Cold lookup after a restart: scan the file newest-first. // Enumerable.Reverse (not the array's .Reverse()) — overload resolution @@ -67,7 +76,7 @@ public static void Record(string id, string content) doc.RootElement.TryGetProperty("content", out var c)) { var content = c.GetString(); - if (content != null) { Memory[id] = content; TrimMemory(); } + if (content != null) Remember(id, content); return content; } } @@ -82,11 +91,28 @@ public static void Record(string id, string content) } } - private static void TrimMemory() + internal static bool IsCached(string id) + { + lock (Gate) return Memory.ContainsKey(id); + } + + private static void Remember(string id, string content) { + if (Memory.Remove(id, out var old)) + { + Recency.Remove(old); + _memoryChars -= old.Value.Content.Length; + } + Memory[id] = Recency.AddLast((id, content)); + _memoryChars += content.Length; // Bound the hot cache as well as the disk archive (UTF-16 character budget). - while (Memory.Count > KeptEntriesOnTrim || Memory.Values.Sum(x => (long)x.Length) > MaxFileBytes / 2) - Memory.Remove(Memory.Keys.First()); + while (Recency.Count > 1 && (Recency.Count > KeptEntriesOnTrim || _memoryChars > MaxFileBytes / 2)) + { + var oldest = Recency.First!; + Recency.RemoveFirst(); + Memory.Remove(oldest.Value.Id); + _memoryChars -= oldest.Value.Content.Length; + } } private static void TrimIfOversized() diff --git a/ClaudeRevit/Services/ToolSearchLogic.cs b/ClaudeRevit/Services/ToolSearchLogic.cs index 4882283..159f01e 100644 --- a/ClaudeRevit/Services/ToolSearchLogic.cs +++ b/ClaudeRevit/Services/ToolSearchLogic.cs @@ -19,12 +19,30 @@ public sealed record SearchResult(List Categories, string Message) } private static readonly HashSet StopWords = new(StringComparer.Ordinal) { "create", "creates", "make", "the", "and", "with", "for", "tool", "tools", "сделай", "создай", "нужна", "добавь", "пожалуйста" }; + private static readonly char[] Separators = + { ' ', ',', '.', ':', ';', '-', '_', '"', '\'', '«', '»', '(', ')', '[', ']', '/', '\\', '?', '!', '\n', '\r', '\t' }; + // Russian builds verbs with prefixes ("образмерь", "замаркируй", "сгруппируй", "заармируй"), + // so a stem may follow one of them; it still has to sit at the start of the word, which keeps + // "по сетке осей" from reading as a rebar mesh. + private static readonly string[] RussianPrefixes = + { "пере", "разо", "раз", "рас", "обо", "об", "под", "при", "про", "от", "за", "вы", "до", "на", "о", "с", "у" }; + private static bool WordMatches(string word, string key) + { + if (word.StartsWith(key, StringComparison.Ordinal)) return true; + if (key.Length < 3 || key[0] < 'а' || key[0] > 'я') return false; + foreach (var prefix in RussianPrefixes) + if (word.StartsWith(prefix, StringComparison.Ordinal) && word.AsSpan(prefix.Length).StartsWith(key, StringComparison.Ordinal)) + return true; + return false; + } private static bool Matches(string text, string keyword) { - var words = text.ToLowerInvariant().Split(new[] { ' ', ',', '.', ':', ';', '-', '\n', '\t', '_' }, StringSplitOptions.RemoveEmptyEntries); - var keys = keyword.Trim().Split(' ', StringSplitOptions.RemoveEmptyEntries); + var words = text.ToLowerInvariant().Split(Separators, StringSplitOptions.RemoveEmptyEntries); + // Split the key the same way, so "технико-эконом" and "сп 1.13130" can match at all. + var keys = keyword.ToLowerInvariant().Split(Separators, StringSplitOptions.RemoveEmptyEntries); + if (keys.Length == 0) return false; for (var i = 0; i + keys.Length <= words.Length; i++) - if (keys.Select((k, j) => words[i + j].StartsWith(k, StringComparison.Ordinal)).All(x => x)) return true; + if (keys.Select((k, j) => WordMatches(words[i + j], k)).All(x => x)) return true; return false; } @@ -57,7 +75,7 @@ private static bool Matches(string text, string keyword) private static readonly (string Category, string[] Keys)[] GroupKeywords = { ("ModPlus", new[] { "modplus", "mod plus", "модплюс", "мод плюс", "mprsumparameters", "сумма параметров" }), - ("Rebar", new[] { "rebar", "reinforc", "stirrup", "арматур", "армир", "хомут", "стержн", "арматурная сет", "сетка армат", "fabric reinforcement", "reinforcement mesh", "ведомость детал", "bending" }), + ("Rebar", new[] { "rebar", "reinforc", "stirrup", "арматур", "армир", "хомут", "стержн", "арматурная сет", "сетка армат", "fabric reinforcement", "reinforcement mesh", "welded mesh", "wire mesh", "fabric sheet", "fabric area", "сварн сетк", "ведомость детал", "bending" }), ("MEP", new[] { "duct", "pipe", "hvac", "воздуховод", "труб", "инженерн" }), ("Schedules", new[] { "schedule", "quantit", "специфик", "ведомост", "расписан", "таблиц" }), ("Sheets", new[] { "sheet", "titleblock", "viewport", "лист", "штамп", "титул" }), @@ -121,7 +139,7 @@ public static SearchResult Search(IEnumerable tools, string query) .Select(g => g.Category) .ToList(); - int Score(ToolInfo t) + int TermScore(ToolInfo t) { var name = t.Name.ToLowerInvariant(); var desc = (t.Description ?? "").ToLowerInvariant(); @@ -133,13 +151,13 @@ int Score(ToolInfo t) if (cat.Contains(term)) s += 2; if (desc.Contains(term)) s += 1; } - // A keyword hit names the group directly, so it outranks an incidental word match in - // some unrelated tool's description. - if (keyedCategories.Contains(t.Category)) s += 4; return s; } + // A keyword hit names the group directly, so it outranks an incidental word match in + // some unrelated tool's description. + int Score(ToolInfo t) => TermScore(t) + (keyedCategories.Contains(t.Category) ? 4 : 0); - var scored = deferred.Select(t => (Tool: t, S: Score(t))) + var scored = deferred.Select(t => (Tool: t, S: Score(t), Terms: TermScore(t))) .Where(x => x.S > 0).OrderByDescending(x => x.S).ToList(); if (scored.Count == 0) @@ -155,22 +173,38 @@ int Score(ToolInfo t) // groups of schemas on every later request. Only groups that actually match well get in: // the best one always, the others if they are at least half as good. var best = scored[0].S; - var cats = scored.Take(12) - .GroupBy(x => x.Tool.Category) - .Select(g => (Category: g.Key, S: g.Max(x => x.S))) - .OrderByDescending(x => x.S) - .Where((x, i) => i == 0 || x.S * 2 >= best) + var cats = keyedCategories + .Where(c => scored.Any(x => x.Tool.Category == c)) + .Concat(scored.Take(12) + .GroupBy(x => x.Tool.Category) + .Select(g => (Category: g.Key, S: g.Max(x => x.S))) + .OrderByDescending(x => x.S) + .Where((x, i) => i == 0 || x.S * 2 >= best) + .Select(x => x.Category)) + .Distinct() .Take(3) - .Select(x => x.Category) .ToList(); - // A group is a search hint, not permission to append every schema in it. - var revealed = scored.Where(x => cats.Contains(x.Tool.Category)).Take(12) - .Select(x => x.Tool).ToList(); + // A group is a search hint, not permission to append every schema in it: tools that the + // query's own words pick out are loaded, best first, at most 12. A group named only by a + // keyword gives no word to rank its tools by (tool descriptions are English, so a Russian + // request scores them all alike), and the first 12 in registration order would hide the + // rest of it for good. That group is loaded whole, as it was before ranking existed. + var keywordOnly = cats + .Where(c => keyedCategories.Contains(c) && !scored.Any(x => x.Tool.Category == c && x.Terms > 0)) + .ToHashSet(); + var ranked = scored.Where(x => cats.Contains(x.Tool.Category) && !keywordOnly.Contains(x.Tool.Category)).ToList(); + var revealed = ranked.Take(12).Select(x => x.Tool) + .Concat(deferred.Where(t => keywordOnly.Contains(t.Category)).OrderBy(t => t.Name, StringComparer.Ordinal)) + .ToList(); + var omitted = ranked.Skip(12).Select(x => x.Tool.Name).ToList(); var sb = new StringBuilder(); sb.Append("Loaded ").Append(revealed.Count).Append(" tool(s) — now callable directly:\n"); foreach (var t in revealed) sb.Append("• ").Append(t.Name).Append(" — ").Append(FirstSentence(t.Description, 90)).Append('\n'); + if (omitted.Count > 0) + sb.Append("Also matched but not loaded (find_tools with the exact name loads one): ") + .Append(string.Join(", ", omitted)).Append('\n'); return new SearchResult(cats, sb.ToString().TrimEnd()) { ToolNames = revealed.Select(t => t.Name).ToList() }; } diff --git a/ClaudeRevit/Tools/LiveSpds.cs b/ClaudeRevit/Tools/LiveSpds.cs index feb5a63..63e6cf1 100644 --- a/ClaudeRevit/Tools/LiveSpds.cs +++ b/ClaudeRevit/Tools/LiveSpds.cs @@ -79,23 +79,32 @@ public void Execute(UpdaterData data) foreach(var (storage,record) in records) { if(!(deleted||materialsChanged||record.Categories.Any(changed.Contains)))continue; + // The record is stored on a DataStorage element. Writing it on every edit would make + // that element part of every user transaction, and in a workshared model whoever + // edited last would own it, so a collaborator's next edit would fail on ownership. + // It is written only when the refresh state changes. try { var refresh = LiveSpds.Refresh(doc,record,false); - record.LastRefreshError = refresh.Errors.Count == 0 ? null : $"{refresh.Errors.Count} sources have incomplete data."; - record.LastRefreshUtc = DateTime.UtcNow.ToString("o"); - LiveSpds.Save(storage,record); + RecordState(storage,record,refresh.Errors.Count == 0 ? null : $"{refresh.Errors.Count} sources have incomplete data."); } catch(Exception ex) { - record.LastRefreshError = ex.Message; - LiveSpds.Save(storage,record); + try{RecordState(storage,record,ex.Message);}catch(Exception saveEx){Services.Log.Error($"Live SPDS table '{record.Key}' could not store its refresh state",saveEx);} Services.Log.Error($"Live SPDS table '{record.Key}' could not refresh; audit_spds_schedule reports its stale state",ex); } } } catch(Exception ex){Services.Log.Error("Live SPDS updater failed; the user's edit was kept",ex);} } + // LastRefreshUtc is when the refresh state was last recorded, not the time of every refresh. + private static void RecordState(DataStorage storage,LiveSpdsRecord record,string? error) + { + if(record.LastRefreshUtc!=null&&record.LastRefreshError==error)return; + record.LastRefreshError=error; + record.LastRefreshUtc=DateTime.UtcNow.ToString("o"); + LiveSpds.Save(storage,record); + } } internal static class LiveSpds diff --git a/docs/audit-fixes-2026-10-20.md b/docs/audit-fixes-2026-10-20.md index 1ef55d0..3c5a0d9 100644 --- a/docs/audit-fixes-2026-10-20.md +++ b/docs/audit-fixes-2026-10-20.md @@ -9,7 +9,7 @@ The supplied interim report contains 168 entries (1 critical, 50 high, 103 mediu - Transactional logical failures roll back before responding. Failed items in `run_batch` roll back their subtransaction while successful items remain in the batch. Script transaction groups also roll back on logical failures. This protects Revit model changes, not external file/network effects of arbitrary code. - Results and aged history stay one JSON value. Truncation preserves failure, preview, commit and rollback control fields. Large diagnostics retain an archive reference. - `get_full_result` returns JSON pages: `content`, `offset`, `next_offset`, `has_more`, `total_chars`. Follow the returned offset. Each serialized page fits the response ceiling, including escaped characters and emoji. -- Archive entries retain up to 2,000,000 characters; `archive_truncated` explicitly identifies larger originals. Memory and disk are bounded. Archives are historical evidence, not a substitute for re-querying edited model elements. +- Archive entries retain up to 2,000,000 characters; `archive_truncated` explicitly identifies larger originals. Memory and disk are bounded. The in-memory cache evicts the least recently used entry (a plain dictionary evicted the newest one after its first removal, so later lookups re-read the archive file). Archives are historical evidence, not a substitute for re-querying edited model elements. ## Model correctness and deletion @@ -24,10 +24,12 @@ The supplied interim report contains 168 entries (1 critical, 50 high, 103 mediu ## Context and responsiveness - Direct API discovery reveals individual tools, at most 12 per search; exact names reveal one. Ambiguous Russian grid/section-size stems no longer prewarm reinforcement or family authoring. Disabled tools are excluded. +- Tools picked out by the query's own words are ranked, and matches beyond the 12 are listed by name so one exact-name search loads any of them. A group named only by a keyword is loaded whole: tool descriptions are English, so a Russian request ("хомуты по зонам") gives no word to rank by, and the first 12 in registration order would hide the rest of the group. Every keyword group in a prompt is loaded, so "арматура и разрез" gets both. +- Group keywords match at the start of a word or after a common Russian verb prefix ("образмерь", "замаркируй", "сгруппируй", "заармируй"). Keys are split like the text, so "технико-эконом" and "СП 1.13130" match, and quotes, guillemets and brackets separate words. - Subscription MCP discovery keeps its existing compact gateway; API discovery and subscription timings are different execution profiles. - Codex driving rules are included in the initial prompt, not duplicated in every resumed prompt. MCP initialization continues to deliver current session instructions. - Catalog lists retain complete cached data and expose bounded pages/counts, section selection, family-category and substring filters. External edits invalidate the affected document cache. -- Cyrillic headless results use Unicode serialization and a serialized-size budget. +- Cyrillic headless results are re-encoded without `\uXXXX` escapes before the budget is measured, so the limit counts real text and the kept head is readable. Callers still serialize with the default encoder; the test now builds its input the same way. - Prepared PNG attachments fit the actual inline base64 budget. Markdown throttles continuous deltas instead of postponing rendering until streaming stops. - Optional closing suggestions do not trigger automatic continuation. Benchmark API round limits use the benchmark's configured budget rather than a smaller pane setting. @@ -37,10 +39,12 @@ Every changed call returns a receipt with exact change counts, a paged archive o A receipt proves only the readback scope. Before claiming dimensional or parameter correctness, use `verify_model_result` with declared expected values; it already performs independent native checks. No generic success flag certifies engineering adequacy or drawing compliance. -Task journals expose authorization-process and API-provider request counts/times plus post-write readback time, in addition to existing total, queue, Revit, worker and token/cache metrics. Phase sums may overlap; provider time includes network/server/streaming and must not be presented as pure model reasoning time. Failed journal writes are surfaced in diagnostics. SPDS updater failures/incomplete sources persist in the model and are exposed by `audit_spds_schedule`. +Task journals expose authorization-process and API-provider request counts/times plus post-write readback time, in addition to existing total, queue, Revit, worker and token/cache metrics. Phase sums may overlap; provider time includes network/server/streaming and must not be presented as pure model reasoning time. Failed journal writes are surfaced in diagnostics. SPDS updater failures/incomplete sources persist in the model and are exposed by `audit_spds_schedule`. The updater writes that state only when it changes, so ordinary edits do not modify the table's DataStorage element (in a workshared model, writing it on every edit would hand its ownership to whoever edited last and fail collaborators' edits). `refresh_utc` is the time the state was last recorded. ## Validation +Follow-up review fixes: 426 unit tests pass, 14 of them new; each new test fails on the previous revision. Builds for Revit 2025/2026/2027, MCP, CLI and the 51 ModPlus checks pass (run on Linux with Windows targeting). + Local checks on Windows: **412 unit tests passed**, isolated pane/MCP/CLI checks passed, **51 ModPlus adapter checks passed**, builds for Revit 2025/2026/2027 and native-harness compilation succeeded (existing compiler warnings remain). No production project was modified, no live model calls were billed, and native Revit execution remains pending. Pure regression tests exercise failure preservation, escaped/Unicode paging, archive retention, Cyrillic budgeting, density including light materials, integer parsing, selective tool discovery, catalog paging, aged result structure and scoped phase accounting. WPF checks use the real markdown renderer and a dense image fixture. Transport/CLI checks use isolated fake clients/processes and no paid model calls. From e2d0aad341a10371f7e49e9cbfbd5bff34d9dda9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 05:28:45 +0000 Subject: [PATCH 3/3] Fix remaining review findings in result handling, catalog, undo, cleanup and model tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Exceptions and repeated-error notes stay one JSON value, so aged results keep their failure state; text "Error…" results count as failures. - Aged summaries copy fewer, shorter fields and never replace a result with something larger. - get_full_result pages carry completion fields up front, so they are not re-capped; the test now runs completion then cap. - System prompt describes the JSON aged summary and next_offset paging. - post_write_readback timing passes the task id from the Revit thread. - Pre-loading uses the user's text only and pre-loads each group once per conversation, avoiding repeated prompt-cache rewrites. - Catalog: overview with family type counts by default, paging first, limit halved until the response fits, offset limited to a single list. - Benchmark API runs that reach the round budget are marked budget-stopped. - undo_last: "steps": null undoes the whole action; an unconfirmed Redo of the user's step is reported as such; no empty change archives. - clean_model: one purge pass, one batch rehearsal with per-candidate fallback only on cascades/errors, bounded failure and cascade samples. - unload_family refuses when types reference the family via a parameter. - NameResolve prefers an exact-case match and accepts "Family: Type" for system types. - derive_parameters fails once on a dimension mismatch, bounds failures, documents factor; calculate_weight lets material density win over the fallback and reports parameter write failures. - Native density case uses 1234 kg/m3 converted by Revit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RCGB6xWx3ZtmWZRQEJtN6B --- ClaudeRevit.NativeTests/ReviewFixtures.cs | 16 +++-- ClaudeRevit.Tests/AuditSafetyTests.cs | 64 ++++++++++++++++- ClaudeRevit.Tests/BenchmarkRunnerStubs.cs | 2 + ClaudeRevit.Tests/BenchmarkRunnerTests.cs | 11 ++- ClaudeRevit.Tests/ToolSearchLogicTests.cs | 4 +- ClaudeRevit/Services/BenchmarkRunner.cs | 2 + ClaudeRevit/Services/CatalogPage.cs | 37 +++++++++- ClaudeRevit/Services/ChatService.cs | 34 ++++++--- ClaudeRevit/Services/TaskJournal.cs | 2 + ClaudeRevit/Services/ToolResult.cs | 19 ++++- ClaudeRevit/Services/ToolResultAging.cs | 8 +-- ClaudeRevit/Services/ToolResultCap.cs | 16 +++-- ClaudeRevit/Tools/CalculateWeight.cs | 27 ++++++-- ClaudeRevit/Tools/CleanModel.cs | 84 +++++++++++++++-------- ClaudeRevit/Tools/DeriveParameters.cs | 12 +++- ClaudeRevit/Tools/GetProjectCatalog.cs | 11 +-- ClaudeRevit/Tools/ModelChangeCapture.cs | 9 ++- ClaudeRevit/Tools/NameResolve.cs | 14 ++-- ClaudeRevit/Tools/ToolDispatcher.cs | 6 +- ClaudeRevit/Tools/UndoLast.cs | 5 ++ ClaudeRevit/Tools/UnloadFamily.cs | 10 +++ docs/audit-fixes-2026-10-20.md | 30 +++++++- 22 files changed, 336 insertions(+), 87 deletions(-) diff --git a/ClaudeRevit.NativeTests/ReviewFixtures.cs b/ClaudeRevit.NativeTests/ReviewFixtures.cs index 0cee0ec..bf3fb97 100644 --- a/ClaudeRevit.NativeTests/ReviewFixtures.cs +++ b/ClaudeRevit.NativeTests/ReviewFixtures.cs @@ -26,7 +26,7 @@ void Check(string name, Func body) Check("set_parameter_reaches_material_named_like_a_type", () => WithProject(app, MaterialBeatsSameNamedType)); Check("audit_total_wall_thickness", () => WithProject(app, TotalThickness)); Check("audit_derived_length_internal_units", () => WithProject(app, DerivedLength)); - Check("audit_material_density_7850", () => WithProject(app, MaterialDensity)); + Check("audit_material_density_1234", () => WithProject(app, MaterialDensity)); Check("audit_placed_unbounded_room_survives_cleanup", () => WithProject(app, PlacedRoom)); return new { passed = checks.Count(c => (bool)c.GetType().GetProperty("ok")!.GetValue(c)!), total = checks.Count, checks }; } @@ -72,10 +72,13 @@ private static object DerivedLength(UIApplication app, Document doc) private static object MaterialDensity(UIApplication app, Document doc) { + // 1234 kg/m3 is no default density in CalculateWeight, so only a correctly read material + // asset gives this mass; Revit converts the value itself, independently of Units.cs. using var tx = new Transaction(doc, "QA density"); tx.Start(); - var materialId = Material.Create(doc, "QA_Steel_7850"); + var materialId = Material.Create(doc, "QA_Density_1234"); var material = (Material)doc.GetElement(materialId); - var asset = new StructuralAsset("QA_Steel_Asset", StructuralAssetClass.Metal) { Density = 7850 * Math.Pow(.3048, 3) }; + var asset = new StructuralAsset("QA_Density_Asset", StructuralAssetClass.Generic) + { Density = UnitUtils.ConvertToInternalUnits(1234, UnitTypeId.KilogramsPerCubicMeter) }; material.SetMaterialAspectByPropertySet(MaterialAspect.Structural, PropertySetElement.Create(doc, asset).Id); var side = 1 / .3048; var loop = CurveLoop.Create(new List @@ -87,10 +90,11 @@ private static object MaterialDensity(UIApplication app, Document doc) new SolidOptions(materialId, ElementId.InvalidElementId)); var shape = DirectShape.CreateElement(doc, new ElementId(BuiltInCategory.OST_GenericModel)); shape.SetShape(new GeometryObject[] { solid }); doc.Regenerate(); - using var result = JsonDocument.Parse(new CalculateWeight().Execute(SyntheticFixtures.Args(new { element_ids = new[] { shape.Id.Value } }), app)); + // The fallback density must not override the material's own. + using var result = JsonDocument.Parse(new CalculateWeight().Execute(SyntheticFixtures.Args(new { element_ids = new[] { shape.Id.Value }, density_kg_m3 = 9999 }), app)); var mass = result.RootElement.GetProperty("total_kg").GetDouble(); - if (Math.Abs(mass - 7850) > .02) throw new InvalidOperationException($"A 1 m3 steel solid must weigh 7850 kg, got {mass}."); - tx.RollBack(); return new { volume_m3 = 1, expected_kg = 7850, actual_kg = mass }; + if (Math.Abs(mass - 1234) > .02) throw new InvalidOperationException($"A 1 m3 solid of a 1234 kg/m3 material must weigh 1234 kg, got {mass}."); + tx.RollBack(); return new { volume_m3 = 1, expected_kg = 1234, actual_kg = mass }; } private static object PlacedRoom(UIApplication app, Document doc) diff --git a/ClaudeRevit.Tests/AuditSafetyTests.cs b/ClaudeRevit.Tests/AuditSafetyTests.cs index 3ea043a..dd60f25 100644 --- a/ClaudeRevit.Tests/AuditSafetyTests.cs +++ b/ClaudeRevit.Tests/AuditSafetyTests.cs @@ -64,8 +64,8 @@ public void PagesRoundTripBeyondOldThirtyThousandLimit(string pattern) do { var page = ToolResultCap.Page("fixture", text, offset, 30_000); - // Pages must survive the production dispatcher cap unchanged. - Assert.Equal(page, ToolResultCap.Apply(page, "get_full_result")); + // Pages must survive the production dispatcher path (completion, then cap) unchanged. + Assert.Equal(page, ToolResultCap.Apply(ToolResult.Complete(page), "get_full_result")); using var parsed = JsonDocument.Parse(page); var r = parsed.RootElement; restored.Append(r.GetProperty("content").GetString()); if (!r.GetProperty("has_more").GetBoolean()) break; @@ -254,6 +254,66 @@ public void ExactDiscoveryLoadsOneSchemaAndVagueVerbLoadsNone() Assert.InRange(ToolSearchLogic.Search(catalog, "rebar").ToolNames.Count, 1, 12); } + [Fact] + public void AgedErrorTextAndNotedFailuresKeepTheirFailureState() + { + var text = "Error: Compilation error:\n" + new string('x', 2_000); + var aged = JsonNode.Parse(ToolResultCap.Summarize(text, "id1", "aged result", 400, aged: true))!; + Assert.False(aged["ok"]!.GetValue()); + Assert.Equal("failed", aged["status"]!.GetValue()); + + var failure = ToolResult.Failure("validation", "Type 'X' not found. Available: " + new string('y', 2_000)); + var noted = ToolResult.WithSystemNote(failure, "[SYSTEM: this exact error has now happened 2 times.]"); + var parsed = JsonNode.Parse(noted)!; // still one JSON value + Assert.Contains("happened 2 times", parsed["system_note"]!.GetValue()); + var agedNoted = JsonNode.Parse(ToolResultCap.Summarize(noted, "id2", "aged result", 400, aged: true))!; + Assert.False(agedNoted["ok"]!.GetValue()); + } + + [Fact] + public void AgingNeverMakesAResultLarger() + { + var receipt = new JsonObject { ["added"] = 1, ["modified"] = 0, ["deleted"] = 0, ["details_id"] = "changes_x", + ["readback"] = new JsonArray(new JsonObject { ["id"] = 1, ["category"] = "Walls", ["bounds_mm"] = new string('1', 120) }) }; + var content = new JsonObject { ["ok"] = true, ["id"] = 123, ["message"] = new string('w', 450), ["receipt"] = receipt } + .ToJsonString(ToolResult.Options); + Assert.InRange(content.Length, 701, 1_200); + var history = new List + { + new() { Role = "user", Blocks = { new ChatToolResultBlock("t1", content, false) } }, + new() { Role = "assistant", Blocks = { new ChatTextBlock("ok") } }, + new() { Role = "user", Blocks = { new ChatTextBlock("next") } }, + }; + ToolResultAging.AgeOlderTurns(history, 1); + var kept = ((ChatToolResultBlock)history[0].Blocks[0]).Content; + Assert.True(kept.Length <= content.Length, $"aged {kept.Length} > original {content.Length}"); + + var large = new JsonObject { ["ok"] = true, ["items"] = new string('z', 20_000), ["receipt"] = receipt.DeepClone() }.ToJsonString(ToolResult.Options); + var agedLarge = ToolResultCap.Summarize(large, "id3", "aged result", 400, aged: true); + Assert.True(agedLarge.Length < 1_500, $"aged summary is {agedLarge.Length} chars"); + } + + [Fact] + public void CatalogOverviewCountsFamilyTypesAndOffsetNeedsOneList() + { + var categories = new JsonObject(); + for (var c = 0; c < 60; c++) + categories["Category " + c] = new JsonArray(Enumerable.Range(0, 60).Select(i => (JsonNode?)JsonValue.Create($"Family {c}: A rather long type name number {i}")).ToArray()); + using var catalog = JsonDocument.Parse(new JsonObject { ["levels"] = new JsonArray(1, 2), ["family_types_by_category"] = categories, ["materials"] = new JsonArray("A", "B") }.ToJsonString()); + var overview = CatalogPage.SelectFitting(catalog.RootElement, null, null, 0, 40, null, 38_000); + Assert.True(overview.ToJsonString(ToolResult.Options).Length <= 38_000); + Assert.Equal(60, overview["catalog"]!["family_type_counts_by_category"]!["Category 7"]!.GetValue()); + Assert.NotNull(overview["paging"]!["materials"]); + + var names = CatalogPage.SelectFitting(catalog.RootElement, new[] { "family_types_by_category" }, null, 0, 40, null, 38_000); + Assert.True(names.ToJsonString(ToolResult.Options).Length <= 38_000); + Assert.NotNull(names["limit_reduced_to"]); + + Assert.Throws(() => CatalogPage.Select(catalog.RootElement, null, null, 40, 40, null)); + var one = CatalogPage.Select(catalog.RootElement, new[] { "family_types_by_category" }, null, 40, 40, "Category 7"); + Assert.Equal(20, one["catalog"]!["family_types_by_category"]!["Category 7"]!.AsArray().Count); + } + [Fact] public void CatalogPagingKeepsCountsAndDoesNotDiscardLateEntries() { diff --git a/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs b/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs index 6ca5532..ac082fa 100644 --- a/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs +++ b/ClaudeRevit.Tests/BenchmarkRunnerStubs.cs @@ -20,6 +20,8 @@ public ChatService(bool ephemeral) { } public Action? OnStatus { get; set; } public TurnTimings Timings { get; } = new() { ModelAndToolsSeconds = 0.01, ToolWaitSeconds = 0.004, QueueSeconds = 0.002, RevitExecutionSeconds = 0.001 }; public string? LastRunError => null; + public static bool StopAtRoundLimit { get; set; } + public bool StoppedAtRoundLimit => StopAtRoundLimit; public sealed record TaskMetrics(string Model, int Rounds, long InputTokens, long OutputTokens, string UsageScope = "turn"); public TaskMetrics? LastTask => new("test-model", 2, 100, 20); public static Func Send { get; set; } = _ => Task.CompletedTask; diff --git a/ClaudeRevit.Tests/BenchmarkRunnerTests.cs b/ClaudeRevit.Tests/BenchmarkRunnerTests.cs index 446c65a..57ec164 100644 --- a/ClaudeRevit.Tests/BenchmarkRunnerTests.cs +++ b/ClaudeRevit.Tests/BenchmarkRunnerTests.cs @@ -15,7 +15,7 @@ public BenchmarkRunnerTests() _dispatcher.Starts = _dispatcher.Ends = 0; _dispatcher.ScopeActive = _dispatcher.Family = false; _dispatcher.LevelCreated = false; _dispatcher.Tools.Clear(); ToolDispatcher.ForceSuppress = false; DocumentSessions.CurrentDocumentKey = "test-document"; - ChatService.Send = _ => Task.CompletedTask; ChatService.Grade = "{\"pass\":true,\"score\":90,\"reason\":\"objective test\"}"; + ChatService.Send = _ => Task.CompletedTask; ChatService.StopAtRoundLimit = false; ChatService.Grade = "{\"pass\":true,\"score\":90,\"reason\":\"objective test\"}"; } public void Dispose() { if (Directory.Exists(_root)) Directory.Delete(_root, true); } private Task Run(IReadOnlyList tasks, List rows, CancellationToken ct = default) => @@ -40,6 +40,15 @@ public async Task SkipsWrongDocumentAndWritesQualitySpeedAndFormulaWithoutInvent Assert.False(second.RootElement.GetProperty("graded").GetBoolean()); } [Fact] + public async Task ApiRunStoppedAtItsRoundBudgetIsMarkedBudgetStopped() + { + // The API path ends at the round limit by itself; nothing is cancelled. + ChatService.StopAtRoundLimit = true; + var rows = new List(); + await Run(new[] { BenchmarkTasks.All[0] }, rows); + Assert.StartsWith("[task budget reached]", rows[0].Reason); + } + [Fact] public async Task CancellationRollsBackScopeAndRestoresSuppression() { using var cancellation = new CancellationTokenSource(); diff --git a/ClaudeRevit.Tests/ToolSearchLogicTests.cs b/ClaudeRevit.Tests/ToolSearchLogicTests.cs index 6b40ff8..e8df85a 100644 --- a/ClaudeRevit.Tests/ToolSearchLogicTests.cs +++ b/ClaudeRevit.Tests/ToolSearchLogicTests.cs @@ -26,11 +26,11 @@ public class ToolSearchLogicTests }; [Fact] - public void Search_by_group_word_reveals_whole_group() + public void Search_by_group_word_loads_the_matching_tools() { var r = ToolSearchLogic.Search(Catalog(), "place rebar stirrups"); Assert.Contains("Rebar", r.Categories); - // All three rebar tools travel together, not just the one keyword hit. + // All three match the group word; a group larger than 12 is covered in AuditSafetyTests. Assert.Contains("create_rebar", r.Message); Assert.Contains("create_rebar_batch", r.Message); Assert.Contains("get_rebar_in_host", r.Message); diff --git a/ClaudeRevit/Services/BenchmarkRunner.cs b/ClaudeRevit/Services/BenchmarkRunner.cs index 2071126..ba23905 100644 --- a/ClaudeRevit/Services/BenchmarkRunner.cs +++ b/ClaudeRevit/Services/BenchmarkRunner.cs @@ -116,6 +116,8 @@ public static async Task RunAsync( // selections never consult the pane's subscription toggle or old overrides. await chat.SendAsync(conversation, execution.Tag, taskCts.Token, mcpSelection: execution.Agent); error = chat.LastRunError; + // The API path stops at the round budget by itself instead of being cancelled. + budgetStopped = chat.StoppedAtRoundLimit; } catch (OperationCanceledException) when (!ct.IsCancellationRequested) { budgetStopped = true; } catch (OperationCanceledException) { throw; } diff --git a/ClaudeRevit/Services/CatalogPage.cs b/ClaudeRevit/Services/CatalogPage.cs index 9d5166b..fe8c18c 100644 --- a/ClaudeRevit/Services/CatalogPage.cs +++ b/ClaudeRevit/Services/CatalogPage.cs @@ -33,10 +33,41 @@ public static JsonObject Select(JsonElement catalog, string[]? sections, string? } return node?.DeepClone(); } + if (sections?.Any(key => !source.ContainsKey(key)) == true) throw new ArgumentException("Unknown catalog section.", nameof(sections)); + // A call with no narrowing is the usual first call. Listing every loadable family type of + // every category would overflow the response, so it gets counts per category instead. + var overview = sections == null && string.IsNullOrWhiteSpace(familyCategory) && string.IsNullOrWhiteSpace(query); var selected = new JsonObject(); foreach (var (key, value) in source) - if (sections == null || sections.Contains(key, StringComparer.Ordinal)) selected[key] = Visit(value, key); - if (sections?.Any(key => !source.ContainsKey(key)) == true) throw new ArgumentException("Unknown catalog section.", nameof(sections)); - return new JsonObject { ["catalog"] = selected, ["paging"] = paging }; + { + if (sections != null && !sections.Contains(key, StringComparer.Ordinal)) continue; + if (overview && key == "family_types_by_category" && value is JsonObject byCategory) + selected["family_type_counts_by_category"] = new JsonObject(byCategory.Select(c => + KeyValuePair.Create(c.Key, (JsonNode?)JsonValue.Create(c.Value is JsonArray list ? list.Count : 0)))); + else selected[key] = Visit(value, key); + } + // One offset applies to every returned list, so continuing one list would silently skip + // the start of all the others. + if (offset > 0 && paging.Count > 1) + throw new ArgumentException("offset continues a single list: pass sections (and family_category for family types) " + + "so that only the list being continued is returned.", nameof(offset)); + var page = new JsonObject { ["paging"] = paging, ["catalog"] = selected }; + if (overview) + page["note"] = "Family type names are listed when sections includes family_types_by_category or family_category is given."; + return page; + } + + // Halve the per-list limit until the response fits the result ceiling, so the paging block + // and every section survive instead of being cut by the generic cap. + public static JsonObject SelectFitting(JsonElement catalog, string[]? sections, string? query, int offset, int limit, string? familyCategory, int maxChars) + { + var page = Select(catalog, sections, query, offset, limit, familyCategory); + while (limit > 1 && page.ToJsonString(ToolResult.Options).Length > maxChars) + { + limit = Math.Max(1, limit / 2); + page = Select(catalog, sections, query, offset, limit, familyCategory); + page["limit_reduced_to"] = limit; + } + return page; } } diff --git a/ClaudeRevit/Services/ChatService.cs b/ClaudeRevit/Services/ChatService.cs index 4e87acf..8c6d049 100644 --- a/ClaudeRevit/Services/ChatService.cs +++ b/ClaudeRevit/Services/ChatService.cs @@ -116,10 +116,11 @@ public class ChatService "in bounded batches and get_checkpoint_job before continuing. Preview does not advance a job. Reuse its key " + "and exact plan/revision; do not recreate completed steps. Save the RVT for crash durability. " + "For live schedules agree nesting_policy and source categories; use audit_spds_schedule and actual sheet exports.\n\n" + - "AGED RESULTS: To save tokens, tool results from earlier prompts are shown truncated with an " + - "'[aged to save tokens …]' marker carrying an id. This is normal. If you genuinely need a full old " + - "result AND it cannot have changed, call get_full_result with that id; if the model may have changed " + - "since, re-query the live model instead.\n\n" + + "AGED RESULTS: To save tokens, large tool results from earlier prompts are replaced by a JSON summary " + + "with \"aged\": true, its original ok/status fields, a short \"head\" excerpt and a full_result_id. The head " + + "is an excerpt of history, not a current result. If you genuinely need the full old result AND it cannot " + + "have changed, call get_full_result with that id and follow next_offset while has_more is true; if the " + + "model may have changed since, re-query the live model instead.\n\n" + "For destructive operations, briefly confirm with the user if intent is ambiguous. Otherwise just proceed. " + "If a tool returns an error, read it and adjust. Each completed batch/tool call has one undo entry. " + "No transaction stays open between calls.\n\n" + @@ -211,7 +212,13 @@ public sealed record TaskMetrics( // Core schemas plus explicitly selected tool names, in stable append order. // Search reveals at most 12 names; clearing history resets discovery. public int? MaxToolRoundsOverride { get; set; } + // Set when the last prompt ended because it used up its tool-call rounds. + public bool StoppedAtRoundLimit { get; private set; } private readonly List _revealedTools = new(); + // Groups already pre-loaded this session. A reveal appends tool schemas, and tools are the + // first cached prefix, so every reveal rewrites the whole prompt cache; a group is pre-loaded + // once, and later prompts naming it reuse what is there (find_tools reaches the rest). + private readonly HashSet _prewarmedGroups = new(); private static IEnumerable SearchableTools() { @@ -329,6 +336,7 @@ public void SwitchWorkspace(bool preserveHistory = false, DocumentSessions.Snaps _execCsharpOk = 0; _promoteNudged = false; _revealedTools.Clear(); + _prewarmedGroups.Clear(); } private static string? ReadSession(string path) { try { return File.Exists(path) ? File.ReadAllText(path).Trim() : null; } catch { return null; } } @@ -385,6 +393,7 @@ public void ClearHistory() _execCsharpOk = 0; _promoteNudged = false; _revealedTools.Clear(); + _prewarmedGroups.Clear(); _claudeCodeSessionId = null; PersistClaudeCodeSession(); _codexSessionId = null; @@ -817,8 +826,11 @@ private async Task SendCoreAsync( // ("add rebar", "make a section") so those tools are present on the first round and the // model doesn't have to spend a find_tools round first. It can still find_tools for the // rest. Purely additive to what's already revealed this session. - if (ToolCatalog.PrewarmCategories(lastUser).Count > 0) - foreach (var name in ToolCatalog.Search(SearchableTools(), lastUser).ToolNames) + // Only the user's own words count: the attachment manifest (paths, ".pdf", "inspect") + // would otherwise open groups the request never asked for. + var newGroups = ToolCatalog.PrewarmCategories(request.Text).Where(_prewarmedGroups.Add).ToHashSet(); + if (newGroups.Count > 0) + foreach (var name in ToolCatalog.Search(SearchableTools().Where(t => newGroups.Contains(ToolCatalog.CategoryOf(t))), request.Text).ToolNames) if (!_revealedTools.Contains(name)) _revealedTools.Add(name); var appendOnly = !alt && ClaudeModels.AppendOnlyHistory(model); @@ -905,6 +917,7 @@ List ClaudeTools() // Read once per prompt so a mid-turn Settings change doesn't shift the cap. var maxIterations = MaxToolRoundsOverride ?? SettingsStore.MaxToolRounds; + StoppedAtRoundLimit = false; // Auto-escalation to a stronger model: keep a cheap/fast model for routine work, switch // to a stronger one when the task looks hard up front, or once the fast model starts @@ -963,6 +976,7 @@ List ClaudeTools() { if (iter >= maxIterations) { + StoppedAtRoundLimit = true; await ui.InvokeAsync(() => conversation.Add(new ChatMessage { Role = "assistant", AssistantName = _assistantName, @@ -1249,7 +1263,7 @@ await ui.InvokeAsync(() => } catch (Exception ex) { - content = $"Error: {ex.Message}"; + content = ToolResult.Failure("tool_error", ex.Message); isError = true; var display = $"{FormatInput(inp)}\n✗ {ex.Message}"; await ui.InvokeAsync(() => @@ -1351,11 +1365,11 @@ await ui.InvokeAsync(() => { resultTurn.Blocks[bi] = b with { - Content = b.Content + - "\n\n[SYSTEM: this exact error has now happened " + n + " times. Retrying the " + + Content = ToolResult.WithSystemNote(b.Content, + "[SYSTEM: this exact error has now happened " + n + " times. Retrying the " + "same call will NOT work — do not repeat it. Instead: look up the EXACT name/id " + "with a list_* or get_* tool, or use a different tool or approach (execute_csharp " + - "is an available fallback), or ask the user how to proceed.]" + "is an available fallback), or ask the user how to proceed.]") }; } } diff --git a/ClaudeRevit/Services/TaskJournal.cs b/ClaudeRevit/Services/TaskJournal.cs index 2171518..82e0ef9 100644 --- a/ClaudeRevit/Services/TaskJournal.cs +++ b/ClaudeRevit/Services/TaskJournal.cs @@ -18,6 +18,8 @@ public sealed record PhaseMetric(int Count, double Seconds); public static string? LastWriteError => Volatile.Read(ref _lastWriteError); public static string? CurrentId => Current.Value; public static IDisposable MeasurePhase(string phase) => new PhaseScope(CurrentId, phase); + // The Revit API thread does not carry the chat flow's AsyncLocal, so code there passes the id. + public static IDisposable MeasurePhase(string? id, string phase) => new PhaseScope(id, phase); private sealed class PhaseScope(string? id, string phase) : IDisposable { private readonly long _started = System.Diagnostics.Stopwatch.GetTimestamp(); diff --git a/ClaudeRevit/Services/ToolResult.cs b/ClaudeRevit/Services/ToolResult.cs index 8ff1f61..fc47630 100644 --- a/ClaudeRevit/Services/ToolResult.cs +++ b/ClaudeRevit/Services/ToolResult.cs @@ -21,7 +21,8 @@ public static bool LooksSuccessful(string raw) if (!obj.ContainsKey("ok") && obj["error"] is { } error && error.ToJsonString() is not ("null" or "\"\"")) return false; return true; } - catch (JsonException) { return true; } // legacy text results + // Legacy text results; the chat loop reports an exception as "Error: …" text. + catch (JsonException) { return !raw.TrimStart().StartsWith("Error", StringComparison.Ordinal); } } private static bool IsFalse(JsonNode? node) => node is JsonValue v && v.TryGetValue(out var b) && !b; private static bool IsTrue(JsonNode? node) => node is JsonValue v && v.TryGetValue(out var b) && b; @@ -114,6 +115,22 @@ JsonValue v when v.TryGetValue(out var n) => n > 0, _ => false }; + // A note from the chat loop, kept inside the result so it stays one JSON value (text appended + // after the JSON would make it unparseable, and history aging would then lose its failure state). + public static string WithSystemNote(string raw, string note) + { + try + { + if (JsonNode.Parse(raw) is JsonObject obj) + { + obj["system_note"] = note; + return obj.ToJsonString(Options); + } + } + catch (JsonException) { } + return raw + "\n\n" + note; + } + public static string Failure(string code, string message) => new JsonObject { ["ok"] = false, ["error"] = message, ["warnings"] = new JsonArray(), diff --git a/ClaudeRevit/Services/ToolResultAging.cs b/ClaudeRevit/Services/ToolResultAging.cs index ad5a18a..add9f39 100644 --- a/ClaudeRevit/Services/ToolResultAging.cs +++ b/ClaudeRevit/Services/ToolResultAging.cs @@ -66,11 +66,11 @@ private static int AgeTurn(ApiTurn turn) } catch (System.Text.Json.JsonException) { } + var summary = ToolResultCap.Summarize(tr.Content, tr.ToolUseId, "aged result", KeepChars, aged: true); + // The envelope carries fixed fields; a result only a little over the threshold would grow. + if (summary.Length >= tr.Content.Length) continue; ToolResultArchive.Record(tr.ToolUseId, tr.Content); - turn.Blocks[i] = new ChatToolResultBlock( - tr.ToolUseId, - ToolResultCap.Summarize(tr.Content, tr.ToolUseId, "aged result", KeepChars, aged: true), - tr.IsError); + turn.Blocks[i] = new ChatToolResultBlock(tr.ToolUseId, summary, tr.IsError); aged++; } return aged; diff --git a/ClaudeRevit/Services/ToolResultCap.cs b/ClaudeRevit/Services/ToolResultCap.cs index 89cf81e..fd0a31a 100644 --- a/ClaudeRevit/Services/ToolResultCap.cs +++ b/ClaudeRevit/Services/ToolResultCap.cs @@ -37,20 +37,22 @@ public static string Summarize(string result, string id, string toolName, int he foreach (var key in Controls) if (source[key] is { } value && value.ToJsonString(ToolResult.Options).Length <= 1_000) envelope[key] = value.DeepClone(); - foreach (var key in new[] { "error", "warnings", "errors", "verification", "plan", "receipt" }) + // An aged summary is replayed on every later request, so its copies stay short; the + // receipt is already in the head. + foreach (var key in aged ? new[] { "error", "warnings", "errors" } : new[] { "error", "warnings", "errors", "verification", "plan", "receipt" }) if (source[key] is { } value) - envelope[key] = Bounded(value, key is "warnings" or "errors"); + envelope[key] = Bounded(value, key is "warnings" or "errors", aged ? 300 : 3_000); } } catch (JsonException) { } return WithBoundedText(envelope, "head", result, 0, Math.Min(headChars, result.Length)); } - private static JsonNode Bounded(JsonNode value, bool array) + private static JsonNode Bounded(JsonNode value, bool array, int limit) { var raw = value.ToJsonString(ToolResult.Options); - if (raw.Length <= 3_000) return value.DeepClone(); - var excerpt = raw[..400] + "… (see archive)"; + if (raw.Length <= limit) return value.DeepClone(); + var excerpt = raw[..Math.Min(400, limit / 2)] + "… (see archive)"; return array ? new JsonArray(new JsonObject { ["code"] = "details_archived", ["message"] = excerpt }) : JsonValue.Create(excerpt)!; } @@ -63,8 +65,10 @@ public static string Page(string id, string content, int offset = 0, int limit = throw new ArgumentOutOfRangeException(nameof(offset), "Do not split a Unicode surrogate pair; use the returned next_offset and a limit of at least 2."); return WithBoundedText(new JsonObject { + // The fields ToolResult.Complete would add are present already, so the dispatcher's + // completion cannot push a page fitted to the ceiling over it. ["ok"] = true, ["id"] = id, ["offset"] = offset, ["total_chars"] = content.Length, - ["historical"] = true + ["historical"] = true, ["warnings"] = new JsonArray(), ["errors"] = new JsonArray(), ["status"] = "success" }, "content", content, offset, Math.Min(limit, content.Length - offset)); } diff --git a/ClaudeRevit/Tools/CalculateWeight.cs b/ClaudeRevit/Tools/CalculateWeight.cs index 36ba203..54b58c9 100644 --- a/ClaudeRevit/Tools/CalculateWeight.cs +++ b/ClaudeRevit/Tools/CalculateWeight.cs @@ -70,6 +70,8 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var rows = new List<(long id, string type, double kg)>(); var noVolume = new List(); + var written = 0; + var writeFailures = new List(); foreach (var el in elements) { @@ -85,16 +87,23 @@ public string Execute(IReadOnlyDictionary input, UIApplicat if (!string.IsNullOrWhiteSpace(writeParam)) { var p = el.LookupParameter(writeParam); - if (p != null && !p.IsReadOnly) + string? problem = p == null ? "parameter not found" : p.IsReadOnly ? "read-only" : null; + if (problem == null) { try { - if (p.StorageType == StorageType.Double) p.Set(kg.Value); - else if (p.StorageType == StorageType.String) p.Set(Math.Round(kg.Value, 2).ToString("0.##")); - else if (p.StorageType == StorageType.Integer) p.Set((int)Math.Round(kg.Value)); + var set = p!.StorageType switch + { + StorageType.Double => p.Set(kg.Value), + StorageType.String => p.Set(Math.Round(kg.Value, 2).ToString("0.##", System.Globalization.CultureInfo.InvariantCulture)), + StorageType.Integer => p.Set((int)Math.Round(kg.Value)), + _ => false + }; + if (set) written++; else problem = "incompatible parameter type"; } - catch { } + catch (Exception ex) { problem = ex.Message; } } + if (problem != null) writeFailures.Add(new { id = el.Id.Value, reason = problem }); } } @@ -115,6 +124,9 @@ public string Execute(IReadOnlyDictionary input, UIApplicat by_type = byType, no_volume = noVolume.Take(50), wrote_parameter = writeParam, + parameter_written = written, + parameter_failed_count = writeFailures.Count, + parameter_failures = writeFailures.Take(50), sample = rows.Take(50).Select(r => new { id = r.id, type = r.type, kg = Math.Round(r.kg, 2) }) }); } @@ -136,12 +148,13 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var dM = Units.FeetToMm(diaFt) / 1000.0; var lM = Units.FeetToMm(totalLenFt) / 1000.0; var volM3 = Math.PI * dM * dM / 4.0 * lM; - return volM3 * (explicitDensity ?? DensityOf(doc, el) ?? SteelDensity); + // density_kg_m3 is the fallback for elements whose material has no density, as described. + return volM3 * (DensityOf(doc, el) ?? explicitDensity ?? SteelDensity); } var volFt3 = VolumeFt3(el); if (volFt3 == null || volFt3 <= 0) return null; - var density = explicitDensity ?? DensityOf(doc, el) ?? DefaultDensity(el); + var density = DensityOf(doc, el) ?? explicitDensity ?? DefaultDensity(el); return Units.CuFeetToCuM(volFt3.Value) * density; } diff --git a/ClaudeRevit/Tools/CleanModel.cs b/ClaudeRevit/Tools/CleanModel.cs index 5ab675c..6c7ecff 100644 --- a/ClaudeRevit/Tools/CleanModel.cs +++ b/ClaudeRevit/Tools/CleanModel.cs @@ -60,10 +60,12 @@ public string Execute(IReadOnlyDictionary input, UIApplicat : all.ToHashSet(); var report = new List(); + // Purge analysis is a whole-model pass; both default targets share one result per call. + var purgeable = new Lazy>(() => doc.GetUnusedElements(new HashSet())); var toDelete = new List(); - if (targets.Contains("unused_family_types")) report.Add(UnusedFamilyTypes(doc, toDelete)); - if (targets.Contains("unused_materials")) report.Add(UnusedMaterials(doc, toDelete)); + if (targets.Contains("unused_family_types")) report.Add(UnusedFamilyTypes(doc, toDelete, purgeable.Value)); + if (targets.Contains("unused_materials")) report.Add(UnusedMaterials(doc, toDelete, purgeable.Value)); if (targets.Contains("exploded_imports")) report.Add(ExplodedImports(doc, toDelete)); if (targets.Contains("unplaced_rooms")) report.Add(UnplacedRooms(doc, toDelete)); if (targets.Contains("empty_groups")) report.Add(EmptyGroups(doc, toDelete)); @@ -71,54 +73,83 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var deleted = 0; var failures = new List(); var cascades = new List(); - var candidates = toDelete.Distinct().ToHashSet(); - // Rehearse actual Revit deletion in a subtransaction even for dry runs. - // Never silently cascade outside the candidate set, including on apply. - foreach (var id in candidates) + var candidates = toDelete.Distinct().Where(id => doc.GetElement(id) != null).ToHashSet(); + // Rehearse actual Revit deletion in a subtransaction even for dry runs, and never cascade + // outside the candidate set, including on apply. The whole set is rehearsed in one delete + // first: one regeneration instead of one per candidate, which on a large model is the + // difference between seconds and minutes. Only when that shows a cascade or an error is + // each candidate rehearsed on its own, to find which ones to refuse. + var clean = false; + if (candidates.Count > 0) { ToolContext.ThrowIfCancelled(); - if (doc.GetElement(id) == null) continue; - using var sub = new SubTransaction(doc); - sub.Start(); + using var rehearsal = new SubTransaction(doc); + rehearsal.Start(); try { - var removed = doc.Delete(id).ToArray(); - var unexpected = removed.Where(x => !candidates.Contains(x)).ToArray(); - cascades.Add(new { id = id.Value, deleted_ids = removed.Select(x => x.Value).ToArray(), - unexpected_ids = unexpected.Select(x => x.Value).ToArray() }); - if (!apply || unexpected.Length > 0) sub.RollBack(); - else { sub.Commit(); deleted += removed.Length; } - if (unexpected.Length > 0) failures.Add(new { id = id.Value, reason = "Dependent elements outside the candidate set: deletion refused." }); + var removed = doc.Delete(candidates).ToArray(); + clean = removed.All(candidates.Contains); + if (clean && apply) { rehearsal.Commit(); deleted = removed.Length; } + else rehearsal.RollBack(); } catch (Autodesk.Revit.Exceptions.RegenerationFailedException) { throw; } catch (OperationCanceledException) { throw; } - catch (Exception ex) + catch (Exception) { - if (sub.GetStatus() == TransactionStatus.Started) sub.RollBack(); - failures.Add(new { id = id.Value, reason = Short(ex.Message) }); + if (rehearsal.GetStatus() == TransactionStatus.Started) rehearsal.RollBack(); } } + if (!clean) + foreach (var id in candidates) + { + ToolContext.ThrowIfCancelled(); + if (doc.GetElement(id) == null) continue; + using var sub = new SubTransaction(doc); + sub.Start(); + try + { + var removed = doc.Delete(id).ToArray(); + var unexpected = removed.Where(x => !candidates.Contains(x)).ToArray(); + if (!apply || unexpected.Length > 0) sub.RollBack(); + else { sub.Commit(); deleted += removed.Length; } + if (unexpected.Length > 0) + { + cascades.Add(new { id = id.Value, unexpected_ids = unexpected.Take(20).Select(x => x.Value).ToArray(), unexpected_count = unexpected.Length }); + failures.Add(new { id = id.Value, reason = "Dependent elements outside the candidate set: deletion refused." }); + } + } + catch (Autodesk.Revit.Exceptions.RegenerationFailedException) { throw; } + catch (OperationCanceledException) { throw; } + catch (Exception ex) + { + if (sub.GetStatus() == TransactionStatus.Started) sub.RollBack(); + failures.Add(new { id = id.Value, reason = Short(ex.Message) }); + } + } + // Samples stay bounded so the per-target details (what the user is shown before apply) + // are never pushed out of the response by long id lists. return Services.Json.Serialize(new { preview = !apply, mode = apply ? "applied" : "dry_run", - candidates = toDelete.Distinct().Count(), + candidates = candidates.Count, deleted_elements = apply ? deleted : 0, - failures, - cascades, details = report, + failures_total = failures.Count, + failures = failures.Take(50), + cascades_total = cascades.Count, + cascades = cascades.Take(50), note = apply - ? "Applied safe candidates only; inspect failures and cascades. Each tool call is one undo step." + ? "Applied safe candidates only; candidates whose deletion would also remove other elements were refused (see cascades). Each tool call is one undo step." : "DRY RUN — nothing was deleted. Show this to the user, then call again with apply=true to proceed." }); } // Native Purge Unused understands references in parameters, nested types, // painted faces and other model data that instance counting misses. - private static object UnusedFamilyTypes(Document doc, List sink) + private static object UnusedFamilyTypes(Document doc, List sink, ISet purgeable) { - var purgeable = doc.GetUnusedElements(new HashSet()); var candidates = new FilteredElementCollector(doc).OfClass(typeof(FamilySymbol)).Cast() .Where(s => purgeable.Contains(s.Id) && s.Family?.IsInPlace != true).ToList(); // Retain at least one type in every loaded family. @@ -134,9 +165,8 @@ private static object UnusedFamilyTypes(Document doc, List sink) return new { target = "unused_family_types", count = selected.Count, sample = selected.Take(50).Select(s => $"{s.Family.Name}: {s.Name}") }; } - private static object UnusedMaterials(Document doc, List sink) + private static object UnusedMaterials(Document doc, List sink, ISet purgeable) { - var purgeable = doc.GetUnusedElements(new HashSet()); var materials = new FilteredElementCollector(doc).OfClass(typeof(Material)).Cast() .Where(m => purgeable.Contains(m.Id)).ToList(); sink.AddRange(materials.Select(m => m.Id)); diff --git a/ClaudeRevit/Tools/DeriveParameters.cs b/ClaudeRevit/Tools/DeriveParameters.cs index c966765..aff25cf 100644 --- a/ClaudeRevit/Tools/DeriveParameters.cs +++ b/ClaudeRevit/Tools/DeriveParameters.cs @@ -22,7 +22,8 @@ public class DeriveParameters : IRevitTool "height_mm, elevation_mm, top_elevation_mm), identity (level_name, type_name, family_name, category, " + "host_name, host_id), or a TEMPLATE combining them.\n" + "template example: \"{level_name}-{type_name}\" — any source name in braces, plus {index} for a running " + - "number. Use `round` for numeric sources and `factor` to scale (e.g. tonnes).\n" + + "number. Use `round` for numeric sources and `factor` to scale (e.g. tonnes) into a Number/Text target; " + + "Length/Area/Volume targets are converted from mm/m²/m³ automatically.\n" + "Target a category (optionally on_level) or an explicit element_ids list. Writes text or numeric " + "parameters; Length/Area/Volume targets convert the declared source units to Revit internal units; mismatched dimensions are rejected. Number/Text targets retain source values."; @@ -50,7 +51,7 @@ public class DeriveParameters : IRevitTool description = "With source='template': text with {placeholders}, e.g. \"{level_name}-{type_name}-{index}\"." }), ["round"] = JsonSerializer.SerializeToElement(new { type = "integer", description = "Decimals for numeric sources. Default 1." }), - ["factor"] = JsonSerializer.SerializeToElement(new { type = "number", description = "Multiply numeric result by this. Default 1." }), + ["factor"] = JsonSerializer.SerializeToElement(new { type = "number", description = "Multiply the numeric result, in its source unit (mm, m², m³), by this. Default 1. Length/Area/Volume targets are converted to Revit units automatically: do not use factor for unit conversion." }), ["skip_if_filled"] = JsonSerializer.SerializeToElement(new { type = "boolean", description = "Don't overwrite a parameter that already has a value. Default false." @@ -119,6 +120,9 @@ public string Execute(IReadOnlyDictionary input, UIApplicat } written.Add(new { id = el.Id.Value, value = shown }); } + // A source/target dimension mismatch is the call's contract error, the same for every + // element: fail the call once instead of repeating it per element as a partial success. + catch (ToolInputException) { throw; } catch (Exception ex) { failed.Add(new { id = el.Id.Value, reason = ex.Message }); } } @@ -126,9 +130,11 @@ public string Execute(IReadOnlyDictionary input, UIApplicat { parameter = paramName, source, + ok = written.Count > 0 || failed.Count == 0, written = written.Count, skipped = elements.Count - written.Count - failed.Count, - failed, + failed_count = failed.Count, + failed = failed.Take(50), sample = written.Take(50) }); } diff --git a/ClaudeRevit/Tools/GetProjectCatalog.cs b/ClaudeRevit/Tools/GetProjectCatalog.cs index b006d9c..07a05f4 100644 --- a/ClaudeRevit/Tools/GetProjectCatalog.cs +++ b/ClaudeRevit/Tools/GetProjectCatalog.cs @@ -22,7 +22,9 @@ public class GetProjectCatalog : IRevitTool "kind/discipline), loadable family types grouped by category, system types (walls, " + "floors, roofs), materials, and the full reinforcement catalog (bar types with " + "diameters, rebar shapes, hook types, cover types, area/path reinforcement types). " + - "Request sections and a family_category/query for the current task; use paging offsets for additional entries. " + + "Without sections, family_category or query it returns an overview with family type counts per category; " + + "request sections and a family_category/query for the names. offset continues one list, so pass the single " + + "section (and family_category) named in paging. A response that would not fit reports limit_reduced_to. " + "The result is cached for the session; pass refresh=true after loading new families or " + "creating new types."; @@ -33,7 +35,7 @@ public class GetProjectCatalog : IRevitTool ["sections"] = JsonSerializer.SerializeToElement(new { type = "array", items = new { type = "string", @enum = new[] { "document", "levels", "view_templates", "family_types_by_category", "system_types", "materials", "reinforcement" } }, description = "Optional sections; request only assets needed for the current task." }), ["family_category"] = JsonSerializer.SerializeToElement(new { type = "string", description = "Optional category substring for family_types_by_category, e.g. Structural Framing." }), ["query"] = JsonSerializer.SerializeToElement(new { type = "string", description = "Optional substring filter within catalog lists." }), - ["offset"] = JsonSerializer.SerializeToElement(new { type = "integer", minimum = 0, description = "Default 0; list continuation offsets are in paging." }), + ["offset"] = JsonSerializer.SerializeToElement(new { type = "integer", minimum = 0, description = "Default 0. Continues one list: use next_offset from paging with the single section (and family_category) it belongs to." }), ["limit"] = JsonSerializer.SerializeToElement(new { type = "integer", minimum = 1, maximum = 300, description = "Default 40 per list." }), ["refresh"] = JsonSerializer.SerializeToElement(new { @@ -74,9 +76,10 @@ public string Execute(IReadOnlyDictionary input, UIApplicat } // One serialization site so the cached and fresh response shapes can't diverge. - var response = Services.CatalogPage.Select(catalog, + var response = Services.CatalogPage.SelectFitting(catalog, input.TryGetValue("sections", out var sections) ? sections.EnumerateArray().Select(s => s.GetString()!).ToArray() : null, - NativeToolUtil.Text(input, "query"), ToolInput.OptionalInt(input, "offset") ?? 0, ToolInput.OptionalInt(input, "limit") ?? 40, NativeToolUtil.Text(input, "family_category")); + NativeToolUtil.Text(input, "query"), ToolInput.OptionalInt(input, "offset") ?? 0, ToolInput.OptionalInt(input, "limit") ?? 40, NativeToolUtil.Text(input, "family_category"), + Services.ToolResultCap.MaxChars - 2_000); response["cached"] = cached; return response.ToJsonString(Services.ToolResult.Options); } diff --git a/ClaudeRevit/Tools/ModelChangeCapture.cs b/ClaudeRevit/Tools/ModelChangeCapture.cs index d022aa4..eda8042 100644 --- a/ClaudeRevit/Tools/ModelChangeCapture.cs +++ b/ClaudeRevit/Tools/ModelChangeCapture.cs @@ -84,16 +84,19 @@ public object Complete(bool rolledBack) added_ids = added, modified_ids = modified, deleted_ids = deleted, rolled_back = rolledBack, units = "Revit internal: ft, kg; bounds below in mm" }); - if (added.Length + modified.Length + deleted.Length > 0 || rolledBack) Services.ToolResultArchive.Record(receiptId, detail); + // A rolled-back or preview call has nothing to page through; archiving its empty list would + // only push real results out of the bounded archive. + var archived = added.Length + modified.Length + deleted.Length > 0; + if (archived) Services.ToolResultArchive.Record(receiptId, detail); Receipt = new { added = added.Length, modified = modified.Length, deleted = deleted.Length, - rolled_back = rolledBack, details_id = receiptId, + rolled_back = rolledBack, details_id = archived ? receiptId : null, details_truncated = detail.Length > Services.ToolResultArchive.MaxContentChars, verification = "Changed IDs re-read after execution; dimensional/parameter expectations require verify_model_result.", readback = added.Concat(modified).Take(8).Select(Readback).ToArray() }; - return new { receipt_id=receiptId, added_count=added.Length,modified_count=modified.Length,deleted_count=deleted.Length, + return new { receipt_id=archived ? receiptId : null, added_count=added.Length,modified_count=modified.Length,deleted_count=deleted.Length, added_by_category=ByCategory(added),modified_by_category=ByCategory(modified), added_by_representation=added.GroupBy(id=>InfoOf(id).Representation).ToDictionary(g=>g.Key,g=>g.Count()), added_ids=added.Take(IdLimit).ToArray(),modified_ids=modified.Take(IdLimit).ToArray(),deleted_ids=deleted.Take(IdLimit).ToArray(), diff --git a/ClaudeRevit/Tools/NameResolve.cs b/ClaudeRevit/Tools/NameResolve.cs index 250bdbb..287496b 100644 --- a/ClaudeRevit/Tools/NameResolve.cs +++ b/ClaudeRevit/Tools/NameResolve.cs @@ -21,6 +21,9 @@ public static T ByName(Document doc, string? name, string kind, var matches = all.Where(e => Matches(e, name)).ToArray(); if (matches.Length == 1) return matches[0]; + // An exact-case hit still decides between names that differ only in case, as before. + var exact = matches.Where(e => e.Name == name?.Trim()).ToArray(); + if (exact.Length == 1) return exact[0]; if (matches.Length > 1) throw new ToolInputException($"Ambiguous {kind} '{name}'; use Family: Type or an explicit id. Matches: {string.Join(", ", matches.Select(Display))}"); throw new InvalidOperationException(NotFound(name, kind, all.Select(Display))); } @@ -30,11 +33,14 @@ public static bool Matches(Element element, string? name) if (string.IsNullOrWhiteSpace(name)) return false; var requested = name.Trim(); if (string.Equals(element.Name?.Trim(), requested, StringComparison.OrdinalIgnoreCase)) return true; - if (element is not FamilySymbol symbol) return false; - return string.Equals($"{symbol.FamilyName}: {symbol.Name}", requested, StringComparison.OrdinalIgnoreCase) || - string.Equals($"{symbol.FamilyName}:{symbol.Name}", requested, StringComparison.OrdinalIgnoreCase); + // System types qualify too ("Basic Wall: Exterior" vs "Curtain Wall: Exterior"), so the + // form the ambiguity error suggests always resolves. + if (element is not ElementType type || string.IsNullOrEmpty(type.FamilyName)) return false; + return string.Equals($"{type.FamilyName}: {type.Name}", requested, StringComparison.OrdinalIgnoreCase) || + string.Equals($"{type.FamilyName}:{type.Name}", requested, StringComparison.OrdinalIgnoreCase); } - private static string Display(Element element) => element is FamilySymbol symbol ? $"{symbol.FamilyName}: {symbol.Name}" : element.Name; + private static string Display(Element element) => + element is ElementType type && !string.IsNullOrEmpty(type.FamilyName) ? $"{type.FamilyName}: {type.Name}" : element.Name; // A ready-made exception for call sites that already did their own lookup. public static ToolInputException Missing(string? name, string kind, IEnumerable available) => diff --git a/ClaudeRevit/Tools/ToolDispatcher.cs b/ClaudeRevit/Tools/ToolDispatcher.cs index 20cfa36..edf32f7 100644 --- a/ClaudeRevit/Tools/ToolDispatcher.cs +++ b/ClaudeRevit/Tools/ToolDispatcher.cs @@ -466,7 +466,9 @@ private void HandleTool(UIApplication app, ToolJob job) if (target == null) { job.Tcs.TrySetResult(Services.ToolResult.Failure("validation", "No bound document. Nothing was undone.")); job.Operation.Dispose(); return; } try { - int? steps = job.Input.ContainsKey("steps") ? ToolInput.RequiredInt(job.Input, "steps") : null; + // An explicit null is how strict/nullable schemas send "not given": undo the whole action. + int? steps = job.Input.TryGetValue("steps", out var stepsValue) && stepsValue.ValueKind != JsonValueKind.Null + ? ToolInput.RequiredInt(job.Input, "steps") : null; if (steps is <= 0) throw new ToolInputException("steps must be a positive integer."); Services.Log.Info($"tool → undo_last steps={steps?.ToString() ?? "action"}"); job.Tcs.Task.ContinueWith(_ => job.Operation.Dispose(), TaskScheduler.Default); @@ -680,7 +682,7 @@ private void HandleTool(UIApplication app, ToolJob job) try { object? delta; - using (Services.TaskJournal.MeasurePhase("post_write_readback")) delta = changes?.Complete(rolledBack); + using (Services.TaskJournal.MeasurePhase(job.TaskId, "post_write_readback")) delta = changes?.Complete(rolledBack); if (completedResult != null && changes?.Receipt is { } receipt && (changes.AddedCount + changes.ModifiedCount + changes.DeletedCount > 0 || executionRolledBack)) { diff --git a/ClaudeRevit/Tools/UndoLast.cs b/ClaudeRevit/Tools/UndoLast.cs index 34919fe..88a4964 100644 --- a/ClaudeRevit/Tools/UndoLast.cs +++ b/ClaudeRevit/Tools/UndoLast.cs @@ -174,6 +174,11 @@ private void Finish(string? problem) { _app.Idling -= OnIdling; _active = null; + // The step undone last was the user's own edit and Revit never confirmed restoring it: + // whatever stopped the sequence, that edit is most likely still undone, so say so first. + if (_foreign != null && !_redoConfirmed) + problem = $"The user's own step '{_foreign}' was undone and Revit did not confirm the Redo " + + $"({problem ?? "no confirmation"}). It is probably still undone: ask the user to press Redo (Ctrl+Y) once in Revit to restore it."; ActionHistory.Pop(_key, _done); var undoneEntries = _entries.Skip(_entries.Count - _done).ToList(); var leftovers = _doc.IsValidObject ? undoneEntries.SelectMany(x => x.AddedSample).Count(id => _doc.GetElement(new ElementId(id)) != null) : 0; diff --git a/ClaudeRevit/Tools/UnloadFamily.cs b/ClaudeRevit/Tools/UnloadFamily.cs index ea5e78b..5f3aab7 100644 --- a/ClaudeRevit/Tools/UnloadFamily.cs +++ b/ClaudeRevit/Tools/UnloadFamily.cs @@ -40,6 +40,16 @@ public string Execute(IReadOnlyDictionary input, UIApplicat var instances = new FilteredElementCollector(doc).OfClass(typeof(FamilyInstance)).Cast() .Where(i => symbols.Contains(i.GetTypeId())).Select(i => i.Id.Value).ToArray(); if (instances.Length > 0) throw new ToolInputException($"Family has {instances.Length} placed instances; nothing was deleted."); + // Profiles, grid and level heads and view titles are used through a type parameter, not + // through instances. Revit resets such a reference to instead of deleting the + // type, so the deletion rehearsal below cannot see it: look for it directly. + var referencing = new FilteredElementCollector(doc).WhereElementIsElementType() + .Where(t => !symbols.Contains(t.Id)) + .Where(t => t.Parameters.Cast().Any(p => p.StorageType == StorageType.ElementId && symbols.Contains(p.AsElementId()))) + .Select(t => $"{(t as ElementType)?.FamilyName}: {t.Name}".TrimStart(':', ' ')) + .Take(20).ToArray(); + if (referencing.Length > 0) + throw new ToolInputException($"Types reference this family's types through a parameter (profile, head, title…): {string.Join(", ", referencing)}. Nothing was deleted."); var allowed = symbols.Append(id).ToHashSet(); using (var preview = new SubTransaction(doc)) { diff --git a/docs/audit-fixes-2026-10-20.md b/docs/audit-fixes-2026-10-20.md index 3c5a0d9..cfd1c21 100644 --- a/docs/audit-fixes-2026-10-20.md +++ b/docs/audit-fixes-2026-10-20.md @@ -41,9 +41,35 @@ A receipt proves only the readback scope. Before claiming dimensional or paramet Task journals expose authorization-process and API-provider request counts/times plus post-write readback time, in addition to existing total, queue, Revit, worker and token/cache metrics. Phase sums may overlap; provider time includes network/server/streaming and must not be presented as pure model reasoning time. Failed journal writes are surfaced in diagnostics. SPDS updater failures/incomplete sources persist in the model and are exposed by `audit_spds_schedule`. The updater writes that state only when it changes, so ordinary edits do not modify the table's DataStorage element (in a workshared model, writing it on every edit would hand its ownership to whoever edited last and fail collaborators' edits). `refresh_utc` is the time the state was last recorded. +## Follow-up review of this change + +A review of the change itself found the issues below; all are fixed here, with regression tests where they can run without Revit. + +- **Failure state survives aging.** An exception is reported as a JSON failure, not "Error: …" text, and the repeated-error nudge is a `system_note` field inside the JSON instead of text after it. Non-JSON text starting with "Error" counts as a failure. +- **Aging never grows a result.** Aged summaries copy only `error`, `warnings` and `errors`, each bounded to 300 characters (the receipt is already in the head). A result whose summary would not be shorter is kept as is. +- **Pages survive completion.** `get_full_result` pages already carry the fields completion adds, so a page fitted to the ceiling is not re-capped. The test now runs the production path (completion, then cap). +- **System prompt** describes the JSON aged summary and `next_offset` paging instead of the removed text marker. +- **Post-write readback time** is recorded: the dispatcher passes the task id, since the Revit API thread does not carry the chat flow's AsyncLocal. +- **Pre-loading** runs on the user's own words, not the attachment manifest, and pre-loads each group once per conversation, so later prompts naming it do not append tools and rewrite the prompt cache. +- **Catalog.** A call without sections, family_category or query returns an overview with family type counts per category. The paging block comes first, and the per-list limit is halved until the response fits (`limit_reduced_to`). A nonzero `offset` needs a single list, since one offset applies to every returned list. +- **Benchmark.** An API run that ends at its round budget is marked `[task budget reached]`, as a cancelled run already was. `0` still means no limit, as documented. +- **Undo.** + - `"steps": null` means a whole-action undo again. + - When the Redo of the user's own step is not confirmed, the result says that step is probably still undone and asks for one Redo. + - Rolled-back and preview calls no longer archive empty change lists. +- **Cleanup.** + - The purge analysis runs once per call. + - All candidates are rehearsed in one delete; candidates are rehearsed one by one only when that shows a cascade or an error. + - Failures and cascades are bounded samples with totals, after the per-target details. +- **Unload.** `unload_family` also refuses when another type references the family's types through a parameter (profiles, grid and level heads, view titles), which Revit would silently reset instead of deleting. +- **Names.** A name that matches several types resolves to its exact-case match when there is one. System types accept the qualified "Family: Type" form (for example "Basic Wall: Exterior"), which is what the ambiguity error suggests. +- **Derived parameters.** A source/target dimension mismatch fails the call once instead of every element being reported as a partial success. Failures are bounded. The schema says `factor` scales in the source unit and is not for unit conversion. +- **Weight.** `density_kg_m3` is the fallback, as documented, and no longer overrides material densities. Parameter writes report `parameter_written` and bounded `parameter_failures` instead of failing silently. +- **Native density case.** It uses 1234 kg/m³ converted by Revit, which matches no default density, and passes a 9999 kg/m³ fallback that must not win. + ## Validation -Follow-up review fixes: 426 unit tests pass, 14 of them new; each new test fails on the previous revision. Builds for Revit 2025/2026/2027, MCP, CLI and the 51 ModPlus checks pass (run on Linux with Windows targeting). +Follow-up review fixes: 430 unit tests pass, 18 of them new (the 14 for discovery, matching, headless budget and archive fail on the previous revision). Builds for Revit 2025/2026/2027, MCP, CLI and the 51 ModPlus checks pass (run on Linux with Windows targeting). Local checks on Windows: **412 unit tests passed**, isolated pane/MCP/CLI checks passed, **51 ModPlus adapter checks passed**, builds for Revit 2025/2026/2027 and native-harness compilation succeeded (existing compiler warnings remain). No production project was modified, no live model calls were billed, and native Revit execution remains pending. @@ -55,7 +81,7 @@ Pure regression tests exercise failure preservation, escaped/Unicode paging, arc | --- | --- | | `audit_total_wall_thickness` | 20 + editable core + 30 mm totals 300 mm | | `audit_derived_length_internal_units` | A wall's measured length copied into a Length parameter remains the same number of internal feet | -| `audit_material_density_7850` | A 1 m³ solid with structural steel density weighs 7850 kg | +| `audit_material_density_1234` | A 1 m³ solid of a 1234 kg/m³ material (no default density, converted by Revit) weighs 1234 kg, even with a 9999 kg/m³ fallback given | | `audit_placed_unbounded_room_survives_cleanup` | A placed room with zero area remains after cleaning unplaced rooms | Compilation is not execution: these new native cases still require Revit 2027. Also run live acceptance for Python exception rollback, painted/nested references during purge, family unload cascades, Undo/Redo interleaved with manual edits/tab switching, and SPDS refresh recovery. Use disposable RVT/RFA fixtures, not production models.