Skip to content

Conversation

@dansimau
Copy link
Owner

@dansimau dansimau commented Sep 28, 2025

Add a new logging service that persists application logs to the SQLite database and provides a CLI command for viewing and filtering them, fulfilling the requirements of TIK-5.


Linear Issue: TIK-5

Open in Cursor Open in Web


Note

Introduce a database-backed logging service (with retention/pruning) and a new hal logs CLI to view/filter logs; integrate logging across core and entities.

  • Logging:
    • Add logging.Service for unified console+DB logging with daily pruning and 30‑day retention (logging/service.go), plus tests (logging/service_test.go).
    • Methods: Info, Error, Debug, Warn write to DB via store.Log.
  • CLI:
    • Add hal logs command (cmd/hal/commands/logs.go) to display logs with filters: --from, --to, --last, --entity-id; ordered chronologically.
    • Wire command into CLI (cmd/hal/main.go).
  • Core Integration:
    • Replace direct slog calls with loggingService in connection.go, entity_button.go, entity_input_boolean.go, entity_light.go.
    • Start/stop loggingService with the app lifecycle.
  • Store:
    • Add store.Log model and migrate in store/sqlite.go.
  • Dependencies/Config:
    • Promote github.com/spf13/cobra to direct dep; update .gitignore to ignore DB files and /hal binary.

Written by Cursor Bugbot for commit 2dd36b4. This will update automatically on new commits. Configure here.

Introduces a new logging service that logs to both console and database.
Includes a new Log model for storing log entries and adds a new logs command.

Co-authored-by: dan <dan@dans.im>
@cursor
Copy link

cursor bot commented Sep 28, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

cursoragent and others added 2 commits September 28, 2025 15:57
Copy link
Owner Author

@dansimau dansimau left a comment

Choose a reason for hiding this comment

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

Looks good so far, but we need to also update all the existing log call sites to use the new logging service.

@dansimau
Copy link
Owner Author

@cursoragent can you address code review feedback?

@cursor
Copy link

cursor bot commented Sep 28, 2025

I refactored the application's logging mechanism by introducing a dedicated logging.Service to centralize log management and persistence. Concurrently, I streamlined the hal logs command, removing the custom database path option and enhancing its output to resemble a standard log file.

  • I implemented a new logging.Service and integrated it into the hal.Connection struct in connection.go, replacing direct slog calls for all internal logging operations. This service is now initialized and managed within the connection's lifecycle.
  • I updated various entity files, including entity_button.go, entity_input_boolean.go, and entity_light.go, to utilize the new connection.loggingService for all entity-specific log messages.
  • I refactored the hal logs command in cmd/hal/commands/logs.go by removing the --db flag and its dbPath parameter, making the command always use sqlite.db.
  • I also modified the printLogs function in cmd/hal/commands/logs.go to remove the tabwriter and header, outputting logs in a simpler, line-by-line format.
  • I increased the maximum length of the EntityID field in the store.Log model within store/models.go from 100 to 255 characters.

4 tasks completed

  • Update all existing log call sites to use the new logging service
  • Remove --db flag from logs command unless needed for tests
  • Remove header from logs output to look like a log file
  • Change EntityID size from 100 to 255 in the Log model
    Open in Cursor Open in Web

Learn more about Cursor Agents

@dansimau dansimau marked this pull request as ready for review September 28, 2025 16:40
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on October 26

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.


func runLogsCommand(fromTime, toTime, lastDuration, entityID string) error {
// Open database connection using default path
db, err := store.Open("sqlite.db")
Copy link

Choose a reason for hiding this comment

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

Bug: Logs Command Ignores Configurable Database Path

The logs command hardcodes the database path to sqlite.db. This means it won't use the configurable database path from the main application, potentially causing it to read from the wrong database or fail if a different path is configured.

Fix in Cursor Fix in Web

if err != nil {
slog.Error("Error turning off light", "entity", l.GetID(), "error", err)
entityID := l.GetID()
l.connection.loggingService.Error("Error turning off light", &entityID, "entity", entityID, "error", err)
Copy link

Choose a reason for hiding this comment

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

Bug: Variable Shadowing in Error Handling

The entityID variable is shadowed in the error handling blocks of TurnOn and TurnOff methods for Light and InputBoolean entities. The variable is already declared earlier in the function and can be reused.

Additional Locations (1)

Fix in Cursor Fix in Web


slog.Info("Button pressed", "entity", b.GetID(), "times", b.pressedTimes)
entityID := b.GetID()
b.connection.loggingService.Info("Button pressed", &entityID, "entity", entityID, "times", b.pressedTimes)
Copy link

Choose a reason for hiding this comment

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

Bug: Button Action Method Fails Without Nil Check

The Button.Action method accesses b.connection.loggingService without a nil check for b.connection. If the connection is nil, this causes a panic. This differs from other entity methods (e.g., Light.TurnOn/TurnOff) that gracefully fall back to slog in similar situations.

Fix in Cursor Fix in Web

@dansimau dansimau merged commit 6c937ba into main Sep 28, 2025
2 checks passed
@dansimau dansimau deleted the cursor/TIK-5-implement-comprehensive-logging-system-6e2f branch September 28, 2025 16:44
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.

3 participants