Tests/phase c deeper coverage

.clang-tidy

@@ -25,6 +25,13 @@ Checks: >
   -cert-dcl37-c,
   -cert-dcl51-cpp,
   -cert-dcl16-c,
+  -cert-dcl03-c,
+  # cert-dcl03-c suggests replacing assert() with static_assert. Disabled
+  # because DASHER_ASSERT (now mapped to assert() in Debug builds per
+  # Tier 1 #2) is used for runtime invariants that cannot be evaluated
+  # at compile time (e.g. DASHER_ASSERT(false) for "should never happen"
+  # branches, DASHER_ASSERT(p != nullptr) for runtime pointers). The
+  # check fires on every such site.
   clang-analyzer-*,
   -clang-analyzer-core.NullDereference,
   -clang-analyzer-core.CallAndMessage,

.github/workflows/ci.yml

@@ -108,9 +108,18 @@ jobs:
 
       - name: Check for warnings
         run: |
-          # Filter out compiler warnings ([-Wxxx]) and command-line noise;
-          # only fail on actual clang-tidy findings ([check-name] pattern)
-          TIDY_WARNINGS=$(grep "warning:" clang-tidy-output.txt | grep -v '\[-W' | grep -v 'command-line option' || true)
+          # Filter out compiler warnings and command-line noise; only fail
+          # on actual clang-tidy findings ([check-name] pattern).
+          # Both forms of compiler warning are excluded:
+          #   [-Wfoo]               — direct from the compiler
+          #   [clang-diagnostic-foo] — same warnings, surfaced via clang-tidy
+          # The latter is what clang-tidy emits when it wraps a -W warning
+          # into its own [check-name] reporting format.
+          TIDY_WARNINGS=$(grep "warning:" clang-tidy-output.txt \
+            | grep -v '\[-W' \
+            | grep -v '\[clang-diagnostic-' \
+            | grep -v 'command-line option' \
+            || true)
           if [ -n "$TIDY_WARNINGS" ]; then
             echo "::error::clang-tidy found issues"
             echo "$TIDY_WARNINGS"

.gitignore

@@ -33,6 +33,7 @@
 
 # Build directories
 build/
+build-*/
 build-sanitize/
 Build/
 out/
@@ -42,6 +43,13 @@ Testing/
 
 # Build logs
 build_log.txt
+*.log.txt
+
+# Local helper scripts and scratch files (prefixed with underscore)
+_*.ps1
+_*.bat
+_*.sh
+_*.py
 
 # IDE
 .autotools
@@ -70,6 +78,12 @@ Data/dasher_settings.xml
 *.settings.xml
 dasher_settings.xml
 dasher.log
+
+# Training files written by the engine to CWD when contexts are destroyed.
+# This is a real bug (Tier 1 item: library should not write to CWD) — for
+# now we ignore the leaked files so they don't pollute git status.
+training_*.txt
+
 build-san/
 build-tidy/
 build-san/

CMakeLists.txt

@@ -35,6 +35,12 @@ add_compile_options(
 add_compile_options(
     $<$<CXX_COMPILER_ID:MSVC>:/W4>
     $<$<CXX_COMPILER_ID:MSVC>:/permissive->
+    # C4100 (unre-ferenced formal parameter) — disabled globally. The legacy
+    # codebase has hundreds of inherited virtual method overrides where
+    # parameters are intentionally unused in specific subclasses. This is a
+    # style issue, not a bug. The bug-finding warnings (C4701 uninitialized,
+    # C4244 narrowing) remain enabled.
+    $<$<CXX_COMPILER_ID:MSVC>:/wd4100>
 )
 
 ###############################
@@ -133,14 +139,6 @@ list(REMOVE_ITEM SOURCE_FILES
     "${CMAKE_CURRENT_LIST_DIR}/src/DasherCore/Common/Unicode/original_harness.c"
 )
 
-# Still in the library but not actively used anymore. Also not supported on all platforms
-list(REMOVE_ITEM HEADER_FILES "${CMAKE_CURRENT_LIST_DIR}/src/DasherCore/CannaConversionHelper.h")
-list(REMOVE_ITEM HEADER_FILES "${CMAKE_CURRENT_LIST_DIR}/src/DasherCore/SocketInputBase.h")
-list(REMOVE_ITEM HEADER_FILES "${CMAKE_CURRENT_LIST_DIR}/src/DasherCore/SocketInput.h")
-list(REMOVE_ITEM SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/src/DasherCore/CannaConversionHelper.cpp")
-list(REMOVE_ITEM SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/src/DasherCore/SocketInputBase.cpp")
-list(REMOVE_ITEM SOURCE_FILES "${CMAKE_CURRENT_LIST_DIR}/src/DasherCore/SocketInput.cpp")
-
 ADD_library(DasherCore STATIC
 	${HEADER_FILES}
 	${SOURCE_FILES}
@@ -203,22 +201,40 @@ if(BUILD_CAPI)
             set(TEST_DATA_DIR "${CMAKE_CURRENT_LIST_DIR}")
         endif()
 
+        # doctest is the test framework. Vendored as a single header in Thirdparty/.
+        set(DOCTEST_INCLUDE_DIR "${CMAKE_CURRENT_LIST_DIR}/Thirdparty/doctest")
+
+        # Default per-test timeout. Tests routinely take 15-30s because each
+        # dasher_create() loads every alphabet XML file from Data/. 300s
+        # gives headroom on slow filesystems (WSL+9P, Windows CI) and for
+        # Debug builds with asserts active, while still catching real hangs.
+        # (CI can override via `ctest --timeout` on the command line.)
+        set(DASHER_TEST_TIMEOUT 300 CACHE STRING "Per-test timeout in seconds")
+
         # Helper function for adding a test executable
         function(dasher_add_test name source)
             add_executable(${name} ${CMAKE_CURRENT_LIST_DIR}/tests/${source})
-            target_include_directories(${name} PRIVATE ${CMAKE_CURRENT_LIST_DIR}/src/ ${CMAKE_CURRENT_LIST_DIR}/tests/)
+            target_include_directories(${name} PRIVATE
+                ${CMAKE_CURRENT_LIST_DIR}/src/
+                ${CMAKE_CURRENT_LIST_DIR}/tests/
+                ${DOCTEST_INCLUDE_DIR})
             target_link_libraries(${name} PRIVATE dasher)
             target_compile_definitions(${name} PRIVATE TEST_DATA_DIR="${TEST_DATA_DIR}")
             add_test(NAME ${name} COMMAND ${name})
+            set_tests_properties(${name} PROPERTIES TIMEOUT ${DASHER_TEST_TIMEOUT})
         endfunction()
 
         # Variant for tests that need internal DasherCore symbols (e.g. ConvertUTF)
         function(dasher_add_test_internal name source)
             add_executable(${name} ${CMAKE_CURRENT_LIST_DIR}/tests/${source})
-            target_include_directories(${name} PRIVATE ${CMAKE_CURRENT_LIST_DIR}/src/ ${CMAKE_CURRENT_LIST_DIR}/tests/)
+            target_include_directories(${name} PRIVATE
+                ${CMAKE_CURRENT_LIST_DIR}/src/
+                ${CMAKE_CURRENT_LIST_DIR}/tests/
+                ${DOCTEST_INCLUDE_DIR})
             target_link_libraries(${name} PRIVATE DasherCore pugixml)
             target_compile_definitions(${name} PRIVATE TEST_DATA_DIR="${TEST_DATA_DIR}")
             add_test(NAME ${name} COMMAND ${name})
+            set_tests_properties(${name} PROPERTIES TIMEOUT ${DASHER_TEST_TIMEOUT})
         endfunction()
 
         dasher_add_test(dasher_capi_tests test_capi.cpp)
@@ -243,16 +259,40 @@ if(BUILD_CAPI)
         dasher_add_test(dasher_training_tests test_training.cpp)
         dasher_add_test(dasher_node_tree_tests test_node_tree.cpp)
 
+        # Phase B (characterization) — closes coverage gaps noted in the review.
+        #   - buffer_lifetime: the C API "valid until next call" contract
+        #   - lm_correctness: training, LP_UNIFORM/LP_LM_*/BP_LM_ADAPTIVE effects
+        #   - view_geometry:   LP_ORIENTATION, BP_NONLINEAR_Y, LP_GEOMETRY
+        #   - input_filters:   all 14 registered filters (12 previously untested)
+        #   - xml_error_paths: malformed alphabet/color/settings XML
+        dasher_add_test(dasher_capi_buffer_lifetime_tests test_capi_buffer_lifetime.cpp)
+        dasher_add_test(dasher_lm_correctness_tests test_lm_correctness.cpp)
+        dasher_add_test(dasher_view_geometry_tests test_view_geometry.cpp)
+        dasher_add_test(dasher_input_filter_tests test_input_filters.cpp)
+        dasher_add_test(dasher_xml_error_path_tests test_xml_error_paths.cpp)
+
+        # Phase C (deeper coverage) — end-to-end + perf + property tests.
+        #   - control_actions_navigation: fires callback via actual node nav
+        #   - spell_word: drives mouse via probabilities to enter children
+        #   - benchmarks: dasher_frame p99 latency baseline (catches regressions)
+        #   - property_invariants: normalization holds across random training
+        dasher_add_test(dasher_control_actions_navigation_tests test_control_actions_navigation.cpp)
+        dasher_add_test(dasher_spell_word_tests test_spell_word.cpp)
+        dasher_add_test(dasher_benchmark_tests test_benchmarks.cpp)
+        dasher_add_test(dasher_property_invariant_tests test_property_invariants.cpp)
+
         # Control action system tests — needs internal DasherCore classes (ActionRegistry)
         # AND C API functions, so we compile CAPI.cpp directly and link DasherCore
         add_executable(dasher_control_action_tests
             ${CMAKE_CURRENT_LIST_DIR}/tests/test_control_actions.cpp
             ${CMAKE_CURRENT_LIST_DIR}/src/CAPI.cpp)
         target_include_directories(dasher_control_action_tests PRIVATE
             ${CMAKE_CURRENT_LIST_DIR}/src/
-            ${CMAKE_CURRENT_LIST_DIR}/tests/)
+            ${CMAKE_CURRENT_LIST_DIR}/tests/
+            ${DOCTEST_INCLUDE_DIR})
         target_link_libraries(dasher_control_action_tests PRIVATE DasherCore pugixml)
         target_compile_definitions(dasher_control_action_tests PRIVATE TEST_DATA_DIR="${TEST_DATA_DIR}")
         add_test(NAME dasher_control_action_tests COMMAND dasher_control_action_tests)
+        set_tests_properties(dasher_control_action_tests PROPERTIES TIMEOUT ${DASHER_TEST_TIMEOUT})
     endif()
 endif()

CONTRIBUTING.md

@@ -130,6 +130,33 @@ Write readable code first. Optimize only after profiling proves a bottleneck.
 - Prefer pass-by-`const`-reference over copy for large types
 - Don't inline complex logic — let the compiler decide
 
+### Rule 7: Naming Convention for New Code
+
+DasherCore has 25 years of accumulated naming styles. We do NOT rename
+existing code (that would be a massive churn PR with zero functional
+benefit). Instead, all **new** code — new classes, new functions, new
+files — should follow these conventions:
+
+| Element | Convention | Example | Notes |
+|---------|-----------|---------|-------|
+| Classes | `PascalCase` | `class AlphabetManager` | Not `CAlphabetManager` (the C-prefix is legacy MFC style) |
+| Methods | `PascalCase` | `void GetProbs()` | Not `getProbs()` or `get_probs()` |
+| Member variables | `snake_case_` (trailing underscore) | `int symbol_count_;` | Not `m_iSymbolCount` (legacy Hungarian) |
+| Free functions | `snake_case` | `int get_root_child_count()` | C API functions use `dasher_` prefix |
+| Constants/enums | `PascalCase` or `UPPER_SNAKE` | `NORMALIZATION`, `EDIT_OUTPUT` | Match surrounding code |
+| Files | `PascalCase.h` / `PascalCase.cpp` | `AlphabetManager.h` | Not `alphabet_manager.h` |
+| Namespaces | `PascalCase` | `namespace Dasher` | |
+
+**Existing conventions you'll encounter (do not change these):**
+- `CFoo` — MFC-style class prefix (~70 classes). The dominant legacy style.
+- `m_iFoo`, `m_bFoo` — Hungarian-notation member prefix. Common in legacy.
+- `Get_node_under_crosshair` — snake_case methods (rare, in DasherModel).
+- `dasher_foo_bar()` — C API functions in `dasher.h` (frozen, do not change).
+
+**Rule of thumb:** When you touch a file, match the file's existing style
+for that edit. When you create a new file, use the conventions above.
+When in doubt, ask in the PR.
+
 ---
 
 ## Testing

Scripts/generate_parameters.py

@@ -13,10 +13,12 @@
 import os
 import sys
 import argparse
+import shutil
+import subprocess
 
 AUTOGEN_HEADER = """\
 // =============================================================================
-// AUTOGENERATED FILE — DO NOT EDIT DIRECTLY
+// AUTOGENERATED FILE - DO NOT EDIT DIRECTLY
 // Generated by: python3 Scripts/generate_parameters.py
 // Source:        settings_manifest.json
 // =============================================================================
@@ -204,6 +206,17 @@ def generate(manifest_path, output_path):
     with open(output_path, 'w') as f:
         f.write(content)
 
+    # Run clang-format on the generated file so it matches the project's
+    # .clang-format style. The template above intentionally uses compact
+    # tab-indented formatting (easier to maintain in Python); clang-format
+    # normalizes it to the project standard. If clang-format is not
+    # installed, the raw output is still valid C++ - just not formatted.
+    clang_format = shutil.which("clang-format")
+    if clang_format:
+        subprocess.run([clang_format, "-i", output_path], check=True)
+    else:
+        print("WARNING: clang-format not found; output is unformatted.", file=sys.stderr)
+
     print(f"Generated {output_path} from {manifest_path}")
     print(f"  {len(manifest['parameters'])} parameters")
 

Strings/strings_en.json

@@ -104,8 +104,6 @@
   "LP_GEOMETRY.enum.Squish and Log": "Squish and Log",
   "LP_LM_WORD_ALPHA.label": "Word Alpha",
   "LP_LM_WORD_ALPHA.description": "Alpha value for word-based model.",
-  "LP_USER_LOG_LEVEL_MASK.label": "User Log Level",
-  "LP_USER_LOG_LEVEL_MASK.description": "Controls level of user logging, 0 = none, 1 = short, 2 = detailed, 3 = both.",
   "LP_ZOOMSTEPS.label": "Zoom Steps",
   "LP_ZOOMSTEPS.description": "Defines how many discrete steps are needed to zoom from the outer edge to the center.",
   "LP_B.label": "B-Parameter",
@@ -130,16 +128,6 @@
   "LP_NONLINEAR_X.description": "Nonlinear compression of X-axis (0 = none, higher = more extreme).",
   "LP_AUTOSPEED_SENSITIVITY.label": "Auto-Speed Sensitivity",
   "LP_AUTOSPEED_SENSITIVITY.description": "Sensitivity of automatic speed control (percent).",
-  "LP_SOCKET_PORT.label": "Socket Port",
-  "LP_SOCKET_PORT.description": "UDP/TCP socket to use for network socket input.",
-  "LP_SOCKET_INPUT_X_MIN.label": "Socket X Min",
-  "LP_SOCKET_INPUT_X_MIN.description": "Bottom of range of X values expected from network input.",
-  "LP_SOCKET_INPUT_X_MAX.label": "Socket X Max",
-  "LP_SOCKET_INPUT_X_MAX.description": "Top of range of X values expected from network input.",
-  "LP_SOCKET_INPUT_Y_MIN.label": "Socket Y Min",
-  "LP_SOCKET_INPUT_Y_MIN.description": "Bottom of range of Y values expected from network input.",
-  "LP_SOCKET_INPUT_Y_MAX.label": "Socket Y Max",
-  "LP_SOCKET_INPUT_Y_MAX.description": "Top of range of Y values expected from network input.",
   "LP_CIRCLE_PERCENT.label": "Circle Start Size",
   "LP_CIRCLE_PERCENT.description": "Percentage of nominal vertical range to use for radius of start circle.",
   "LP_TWO_BUTTON_OFFSET.label": "Two Button Offset",
@@ -166,12 +154,6 @@
   "LP_STATIC1B_TIME.description": "The time in milliseconds for static mode to scan from top to bottom.",
   "LP_STATIC1B_ZOOM.label": "Static Zoom Step",
   "LP_STATIC1B_ZOOM.description": "The amount of zoom applied per step in static mode.",
-  "LP_DEMO_SPRING.label": "Demo Springiness",
-  "LP_DEMO_SPRING.description": "A physics parameter for the automated cursor's 'springiness' in demo mode.",
-  "LP_DEMO_NOISE_MEM.label": "Demo Noise Memory",
-  "LP_DEMO_NOISE_MEM.description": "A physics parameter controlling the 'memory' or consistency of random movements in demo mode.",
-  "LP_DEMO_NOISE_MAG.label": "Demo Noise Magnitude",
-  "LP_DEMO_NOISE_MAG.description": "Controls the magnitude or intensity of random movements in demo mode.",
   "LP_MAXZOOM.label": "Zoom Sensitivity / Max Zoom",
   "LP_MAXZOOM.description": "Controls zoom mechanics in click mode.",
   "LP_DYNAMIC_SPEED_INC.label": "Auto-Speed Increment",