Skip to content

fix: debug server /tuning-history returns correct field names#429

Merged
eddycek merged 2 commits intomainfrom
fix/debug-server-tuning-history-fields
Apr 9, 2026
Merged

fix: debug server /tuning-history returns correct field names#429
eddycek merged 2 commits intomainfrom
fix/debug-server-tuning-history-fields

Conversation

@eddycek
Copy link
Copy Markdown
Owner

@eddycek eddycek commented Apr 9, 2026

Summary

  • Debug server /tuning-history endpoint was mapping non-existent field names (r.type instead of r.tuningType, r.appliedChanges instead of individual change arrays)
  • Verification metrics, convergence data, log IDs, and transfer function metrics were completely omitted from the response
  • This caused tooling (Claude Code) to incorrectly report verificationMetrics: null and tuningType: null for all history records

Test plan

  • Start dev server, run curl -s http://127.0.0.1:9300/tuning-history | jq '.records[0] | keys' — verify all fields present
  • Verify tuningType, appliedPIDChanges, verificationPidMetrics, convergence are populated

🤖 Generated with Claude Code

The endpoint was mapping non-existent field names (r.type instead of
r.tuningType, r.appliedChanges instead of r.appliedPIDChanges) and
omitting verification metrics, convergence data, and log IDs entirely.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 9, 2026 09:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the debug server’s /tuning-history response so it exposes the correct tuning history fields (matching stored CompletedTuningRecord data) for downstream tooling consumption.

Changes:

  • Corrects field mappings (e.g., typetuningType) and stops returning non-existent properties.
  • Adds additional tuning history fields to the response (verification metrics, applied change arrays, log IDs, convergence, etc.).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +483 to +490
transferFunctionMetrics: r.transferFunctionMetrics,
appliedFilterChanges: r.appliedFilterChanges,
appliedPIDChanges: r.appliedPIDChanges,
appliedFeedforwardChanges: r.appliedFeedforwardChanges,
filterLogId: r.filterLogId,
pidLogId: r.pidLogId,
verificationLogId: r.verificationLogId,
convergence: r.convergence,
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The history record type includes additional fields that are still omitted from this debug response (e.g., quickLogId and verificationTransferFunctionMetrics). Since the PR description calls out missing log IDs / transfer-function metrics, consider including these fields (or explicitly documenting why they’re excluded) so tooling can see the full data set.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — added quickLogId and verificationTransferFunctionMetrics in 07074b2.

Comment thread src/main/debug/DebugServer.ts Outdated
Comment on lines 474 to 477
records: records.map((r: any) => ({
id: r.id,
type: r.type,
tuningType: r.tuningType,
completedAt: r.completedAt,
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

records.map((r: any) => ...) leaves this endpoint untyped, which is how incorrect/nonexistent fields (e.g., r.type, r.appliedChanges) can slip in without compile-time errors. Consider typing records/r as CompletedTuningRecord (and the response shape) to make future field-name mismatches a TypeScript error.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — r is now typed as CompletedTuningRecord in 07074b2.

Address CodePilot feedback:
- Type `r` as CompletedTuningRecord (prevents future field-name mismatches)
- Add quickLogId and verificationTransferFunctionMetrics

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@eddycek eddycek merged commit 13e5d84 into main Apr 9, 2026
1 check passed
@eddycek eddycek deleted the fix/debug-server-tuning-history-fields branch April 9, 2026 10:38
eddycek added a commit that referenced this pull request Apr 9, 2026
3099→3105 tests, 144→145 files after #429/#430. Add new component
to renderer CLAUDE.md and TESTING.md inventory.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants