Add load latest data button, quiet mode default, and fetch citation from GitHub

[LukasWallrich]

Summary

• Add "Load latest data" button to Explorer sidebar that fetches fresh data from OSF
• Change verbose default to FALSE in load_fred_data() and read_fred() for quieter output
• Modify create_citation() to fetch citation from GitHub instead of regenerating dynamically

Test plan

• R CMD check passes with 0 errors, 0 warnings, 0 notes
• Tested refresh button in Explorer app - successfully downloads fresh data from OSF

Closes #85
Closes #53
Closes #98

🤖 Generated with Claude Code

[claude]

Claude finished @LukasWallrich's task —— View job

---

Code Review Progress

I'll review this pull request focusing on code quality, best practices, potential bugs, performance, security, and test coverage.

Review Tasks:

• Read repository setup instructions (CLAUDE.md)
• Review R package code changes (data_management.R, fred.R)
• Review Shiny app changes (global.R, server.R, ui.R)
• Review documentation updates
• Analyze code quality and best practices
• Check for potential bugs and issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage

---

Code Review Summary

This pull request implements three main improvements to the FReD package: a data refresh button, quiet mode defaults, and GitHub-based citation fetching. Here's my comprehensive review:

✅ Strengths

1. Clean Architecture: The data refresh functionality is well-implemented with proper reactive patterns using reactiveValues() to manage data state
2. Good Error Handling: The refresh mechanism includes proper try-catch blocks with user notifications
3. Consistent API Changes: The verbose = FALSE default change is applied consistently across both read_fred() and load_fred_data() functions
4. Documentation Updates: All documentation files properly reflect the parameter changes

⚠️ Issues & Recommendations

1. Potential Data Consistency Issues (server.R:24-37)

# Force fresh download by temporarily disabling offline mode
was_offline <- get_param("FRED_OFFLINE")
if (was_offline) use_FReD_offline(FALSE)

# Delete cached data file to force fresh download from OSF
cached_file <- get_param("FRED_DATA_FILE")
if (file.exists(cached_file)) {
  file.remove(cached_file)
}

Issue: This approach could cause data inconsistency if multiple users/sessions are accessing the same cached file, or if the operation fails partway through.
Recommendation: Consider using a session-specific temporary file or implement file locking.

2. Missing Input Validation (fred.R:82-83)

download.file(citation_url, temp, quiet = TRUE)
cit <- readLines(temp, warn = FALSE) %>% paste(collapse = "\n")

Issue: No validation that the download succeeded or that the file contains valid citation data.
Recommendation: Add validation:

if (!file.exists(temp) || file.size(temp) == 0) {
  stop("Failed to download citation file")
}

3. Potential Memory Concerns (server.R:33-35)
The data refresh doesn't clean up the old data before loading new data, which could cause temporary memory spikes for large datasets.

4. Network Dependency (fred.R:71)
The new GitHub-based citation approach introduces a network dependency that wasn't present in the original dynamic generation method. Consider implementing a more robust fallback strategy.

5. Silent File Removal (server.R:29-31)
File deletion happens without user confirmation, which could be unexpected behavior.

🔒 Security Assessment

• Low Risk: No obvious security vulnerabilities
• The download.file() calls use HTTPS URLs which is good practice
• No user input is directly used in file operations

⚡ Performance Considerations

• Positive: Quiet mode default reduces console output overhead
• Concern: Data refresh involves full dataset reprocessing which could be slow for large datasets
• Suggestion: Consider implementing incremental updates where possible

🧪 Test Coverage

Missing Test Areas:

1. Data refresh functionality under various network conditions
2. Citation URL fetch failure scenarios
3. File system errors during cache deletion
4. Concurrent access scenarios

📋 Minor Issues

1. Code Style: Some inconsistent spacing in server.R around reactive observers
2. Documentation: The changelog reference in fred.R:291 uses the old function signature

🎯 Overall Assessment

This is a solid improvement that adds valuable functionality while maintaining backward compatibility. The code quality is good with proper separation of concerns. The main risks are around network reliability and concurrent access to shared cached files.

Recommendation: ✅ APPROVE with suggestions for minor improvements around error handling and validation.

---

[claude]

Claude finished @LukasWallrich's task —— View job

---

Code Review Summary

I've completed a comprehensive review of this pull request. The changes implement three main improvements: a data refresh button, quiet mode defaults, and GitHub-based citation fetching. Here's my detailed analysis:

✅ Strengths

1. Well-implemented Data Refresh: The server.R:20-44 implementation uses proper reactive patterns with reactiveValues() to manage data state and includes appropriate error handling with user notifications.

2. Consistent API Changes: The verbose = FALSE default is consistently applied across both read_fred() and load_fred_data() functions (data_management.R:91, fred.R:143).

3. Enhanced Citation Function: The new GitHub-based approach (fred.R:70-104) includes improved validation and caching mechanisms.

4. Clean User Interface: The refresh button integration in ui.R:89-90 is well-positioned and styled appropriately.

⚠️ Issues & Recommendations

1. Potential Data Consistency Issues (server.R:24-37)

# Force fresh download by temporarily disabling offline mode
was_offline <- get_param("FRED_OFFLINE")
if (was_offline) use_FReD_offline(FALSE)

# Delete cached data file to force fresh download from OSF
cached_file <- get_param("FRED_DATA_FILE")
if (file.exists(cached_file)) {
  file.remove(cached_file)
}

Issue: This approach could cause data inconsistency if multiple users/sessions access the same cached file, or if the operation fails partway through.

Recommendation: Consider using session-specific temporary files or implement file locking mechanisms.

2. Missing Error Handling (fred.R:82-89)
The download validation is good, but could be more robust:

# Current validation
if (!file.exists(temp) || file.size(temp) == 0) {
  stop("Failed to download citation file")
}

Recommendation: Add network timeout handling and more specific error messages for different failure modes.

3. Potential Memory Concerns (server.R:33-35)
The data refresh doesn't explicitly clean up old data before loading new data, which could cause temporary memory spikes for large datasets.

4. Network Dependency Risk (fred.R:70)
The new GitHub-based citation approach introduces a network dependency that wasn't present in the original dynamic generation. While there's a fallback to return_inbuilt("citation"), consider documenting this dependency clearly.

🔒 Security Assessment

• Low Risk: No obvious security vulnerabilities
• The download.file() calls use HTTPS URLs which is good practice
• No user input is directly used in file operations
• Temporary file handling follows secure practices

⚡ Performance Considerations

• Positive: Quiet mode default (verbose = FALSE) reduces console output overhead
• Concern: Data refresh involves full dataset reprocessing which could be slow for large datasets
• Good: Caching mechanism in citation function (fred.R:77-79) improves repeated access performance

🧪 Test Coverage Assessment

Missing Test Areas:

1. Data refresh functionality under various network conditions
2. Citation URL fetch failure scenarios
3. File system errors during cache deletion
4. Concurrent access scenarios
5. Integration tests for the Shiny app refresh functionality

The existing test suite (tests/testthat/test-replication_outcomes.R) focuses on replication outcome assessments but doesn't cover the new features.

📋 Code Quality Notes

1. Good: Consistent code style and error handling patterns
2. Good: Proper documentation updates in all .Rd files
3. Minor: Some inconsistent spacing in server.R around reactive observers
4. Good: Global data loading function (global.R:15-22) enables clean refresh implementation

🎯 Overall Assessment

This is a solid improvement that adds valuable functionality while maintaining backward compatibility. The code quality is good with proper separation of concerns and appropriate error handling. The main risks are around network reliability and concurrent access to shared cached files.

Recommendation: ✅ APPROVE with suggestions for minor improvements around concurrent access handling and expanded test coverage.

The changes successfully address the stated objectives and follow good R/Shiny development practices. The quiet mode default is a user-friendly improvement, and the data refresh functionality provides valuable utility for keeping the application current.

---