feat: Table sorting and target management for headless mode

[jsavin]

Summary

This PR implements comprehensive table sorting and target management functionality for headless CLI mode, including 16 integration tests covering all major code paths.

Key Features:

• Table sorting verbs (table.sortby, table.getsortorder) - Sort tables by name, value, or kind columns with case-insensitive matching
• Target management verbs (lang.settarget, lang.gettarget, lang.cleartarget) - Explicit control of current target table in headless mode
• String resources - Direction list and table column titles via YAML-based resource system (no CFBundle dependency in headless)
• Headless stubs - tablegettitlestring() for column name mapping, getstringlist() YAML lookup

Test Coverage

Integration Tests: 16/16 passing

All tests in tests/integration/test_cases/table_sorting_verbs.yaml:

• table.sortby() with all column types (name, value, kind)
• Case-insensitive column name matching (NAME, VaLuE, kind)
• table.getsortorder() - verify persisted sort order
• Sequential sort order changes
• lang.settarget/gettarget/cleartarget - target lifecycle
• Error handling (invalid column, no target set)

Unit Tests: Passing

Key Changes

Core Implementation:

• Common/source/resources.c - Guard macOS CFBundle getstringlist() with #ifndef FRONTIER_HEADLESS
• resources/strings/langerrorlist.yaml - Added directionlist (135) and tablestringlist (165) string resources

Headless Mode:

• tests/headless_table_stubs.c - Implemented tablegettitlestring() stub for column name mapping
• tests/headless_lang_runtime_more_stubs.c - Extended getstringlist() to handle directionlistnumber (135) and tablestringlist (165)

Tests:

• tests/integration/test_cases/table_sorting_verbs.yaml - 220 lines, 16 comprehensive integration test cases

Architectural Notes

Target System (Two-Tier)

• Selection context: Auto-set by lang.new() when table first becomes active
• Explicit lang target: Set via lang.settarget(), checked before selection context
• Selection context takes precedence (maintains backward compatibility with existing code)

String Resources

• Headless mode loads strings from YAML instead of macOS CFBundle
• Direction strings (up, down, left, right, flatup, flatdown, sorted, pageup, pagedown, pageleft, pageright)
• Table column titles (name, value, kind)

API Details

• lang.gettarget() returns addressvaluetype - caller must use ^ dereference to get table value
• table.sortby() uses case-insensitive column name matching
• table.getsortorder() returns lowercase string ("name", "value", or "kind")

Next Steps

1. Bot review feedback incorporation (if needed)
2. Monitor for integration test execution and coverage reports
3. Merge upon successful bot approval and all checks passing

🤖 Generated with Claude Code

[claude]

Code Review - PR #237: Table Sorting and Target Management

Overall Assessment: ✅ APPROVE WITH MINOR RECOMMENDATIONS

This is a solid, well-architected implementation of table sorting and target management for headless mode. The code demonstrates excellent engineering practices with comprehensive testing, proper abstraction, and careful attention to thread safety and resource management.

---

Strengths 🌟

1. Excellent Architecture

• Proper separation of concerns: Headless vs windowed mode dispatch is clean and maintainable
• Reference-counted context management: The table_selection_context_t design with atomic refcounting is production-ready
• Cleanup guard pattern: Using goto cleanup for resource management in sortbyfunc prevents context leaks (lines 1456-1490)

2. Comprehensive Test Coverage

• 220-line integration test suite with 16 test cases covering all major code paths
• Tests include edge cases: case-insensitive matching, invalid columns, error handling
• Both unit tests (test_table_sorting.c) and integration tests (table_sorting_verbs.yaml)

3. Robust Error Handling

• Proper validation of column indices (lines 1520-1525)
• NULL checks before context operations
• Logging for diagnostic purposes (log_error for invalid sort order)
• Clear user-facing error messages

4. Thread Safety Foundation

• table_selection_context_t uses _Atomic uint32_t refcount for concurrent access safety
• Context acquire/release pattern prevents dangling pointers
• Designed with Phase 6 collaborative editing in mind (reserved fields for CRDT metadata)

5. Documentation Quality

• Comprehensive PR description with implementation details, architectural notes, and API usage
• Inline code comments explain non-obvious logic
• Function-level documentation in headers (headless_selection.h)

---

Code Quality Observations

Positive Patterns

1. Cursor Preservation Logic (lines 1464-1482)

bigstring cursor_key;
boolean had_cursor = (ctx->cursor_key[0] > 0);
setemptystring(cursor_key);  /* Initialize to empty */

if (had_cursor) {
    copystring(ctx->cursor_key, cursor_key);
}

/* Resort table */
(**htable).sortorder = ixcol;
hashresort(htable, nil);

/* Restore cursor and recalculate flat index */
if (had_cursor) {
    ctx->current_table = htable;
    copystring(cursor_key, ctx->cursor_key);
    ctx->cursor_flat_index = table_selection_get_row_for_key(ctx, htable, cursor_key);
}

✅ Excellent: Preserves UX by maintaining cursor position across sort operations.

2. Context Leak Protection (lines 1486-1488)

cleanup:
    if (ctx != NULL)
        table_selection_release(ctx);

✅ Best practice: Defensive cleanup pattern ensures resources are freed even if future code adds error paths.

3. Bounds Validation (lines 1520-1525)

if (ixcol < namecolumn || ixcol > kindcolumn) {
    log_error(LOG_COMP_TABLE, "Invalid sort order: %d (valid range: %d-%d)",
              ixcol, namecolumn, kindcolumn);
    langerrormessage(BIGSTRING("\x1A" "Invalid sort order state"));
    return (false);
}

✅ Defensive: Protects against database corruption or uninitialized table state.

---

Minor Recommendations (Non-Blocking)

1. Test Infrastructure Robustness (Priority: LOW)

Issue: test_table_sorting.c had multiple segfaults during development that required extensive fixes:

• Missing test framework functions (init, summary, get_exit_code)
• Incomplete UserTalk runtime initialization
• Missing RUN_TEST macro

Observation: While these were fixed (commits 117fe0e, 9f88021), this pattern suggests test infrastructure fragility.

Recommendation:

• Consider creating a TEST_FRAMEWORK_TEMPLATE.md documenting the required initialization sequence
• Add a test that validates test framework setup to catch missing components early

Impact: Low - fixes are in place, but documentation would prevent similar issues in future test suites.

---

2. Portable Build Fixes (Priority: INFORMATIONAL)

Commits ecd4025, a035f6c, 6e58b5e added numerous portable type definitions and fixed include chains.

Observation: These were necessary fixes, but the number of commits (9 total) suggests portable builds may not be regularly tested.

Recommendation:

• Add CI job for -DFRONTIER_PORTABLE builds to catch regressions early
• Document portable build requirements in docs/BUILDING.md

Impact: Low - current fixes are correct, but CI would prevent future breakage.

---

3. Headless Stub Organization (Priority: MINOR)

Issue: headless_stubs.c was created to resolve ODR violation (commit 4498a12), but stub organization is split across multiple files:

• headless_stubs.h / headless_stubs.c - General stubs
• headless_table_stubs.c - Table-specific stubs
• headless_lang_runtime_more_stubs.c - Language runtime stubs

Recommendation:

• Consider consolidating into tests/headless_stubs/ directory with subdirectories by subsystem
• Pattern: tests/headless_stubs/table/, tests/headless_stubs/lang/, etc.

Impact: Very Low - current organization works, but future maintainability improvement.

---

4. String Resource Abstraction (Priority: OBSERVATION)

Files Modified:

• Common/source/resources.c - Guards macOS CFBundle with #ifndef FRONTIER_HEADLESS
• tests/headless_lang_runtime_more_stubs.c - YAML-based string lookup for headless

Observation: This creates two parallel string resource systems (CFBundle for Mac, YAML for headless).

Recommendation: (For future work, not this PR)

• Consider abstracting string resources behind a common API
• Both implementations could use the same backing YAML files
• This would eliminate CFBundle dependency on macOS builds too

Impact: None for this PR - observation for future refactoring opportunities.

---

Security & Performance

Security: ✅ No Issues

• No user input flows directly to unsafe operations
• Column name validation prevents injection attacks
• Bounds checking on array indices

Performance: ✅ Appropriate

• Context acquire/release adds minimal overhead (atomic increment/decrement)
• Table resort (hashresort) is expected to be O(n log n) - acceptable for typical table sizes
• Cursor preservation lookup is O(n) worst case - reasonable tradeoff for UX

---

Alignment with CLAUDE.md Standards

✅ Follows Project Conventions

1. Thread Safety (CLAUDE.md "Global Mutable State"):

   • Uses thread-local context instead of global state ✅
   • Reference counting for nested operation safety ✅

2. Logging Standards (CLAUDE.md "Logging Standards"):

   • Uses log_error(), log_warn(), log_debug() ✅
   • No fprintf(stderr) violations ✅

3. Multi-Session Stability (CLAUDE.md "Worktree Discipline"):

   • PR created from feature branch with proper workflow ✅
   • Multiple commits cleaned up via squash/fixup strategy ✅

4. Verb Implementation (CLAUDE.md "Implementing Kernel Verbs"):

   • Follows tyvaluerecord wrapping pattern ✅
   • Sets flnextparamislast correctly ✅

5. North Star Vision (CLAUDE.md "Collaborative ODB Editing"):

   • table_selection_context_t has void *reserved[4] for future CRDT metadata ✅
   • Design explicitly supports Phase 6 multi-user scenarios ✅

---

Testing Evidence

Integration Tests: 16/16 PASSING ✅

From PR description:

• table.sortby() with all column types (name, value, kind)
• Case-insensitive column name matching
• table.getsortorder() verification
• Sequential sort order changes
• lang.settarget/gettarget/cleartarget lifecycle
• Error handling (invalid column, no target set)

Unit Tests: PASSING ✅

• test_table_sorting builds successfully
• Full UserTalk runtime initialization
• 8 test cases covering core functionality

---

Final Verdict

✅ APPROVED

This PR demonstrates:

• Strong engineering fundamentals: Proper abstraction, error handling, resource management
• Future-proof design: Thread safety, reference counting, reserved fields for collaborative editing
• Excellent test coverage: 16 integration tests + unit tests
• Clean code: Readable, well-documented, follows project conventions

The minor recommendations are non-blocking observations for future improvements, not issues with this PR.

Recommended Actions:

1. ✅ Merge this PR
2. Consider CI job for portable builds (future work)
3. Document test framework initialization sequence (future work)

---

Great work on this implementation! 🎉

The table sorting and target management features are production-ready and align perfectly with the project's collaborative ODB vision.