Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3de0f72
ai generated caching system. todo: review and test
toloudis Feb 19, 2026
f005527
fix compilation
toloudis Feb 19, 2026
218e9f6
use cache on time slider change
toloudis Feb 19, 2026
9aa1175
add unit tests for cache mgr
toloudis May 2, 2026
7413032
improve cache tests
toloudis May 3, 2026
a6058c3
Merge branch 'main' into feature/volumecache
toloudis May 9, 2026
0feb6d8
Merge branch 'main' into feature/volumecache
toloudis May 11, 2026
d06adae
Merge branch 'main' into feature/volumecache
toloudis May 19, 2026
abd8c8d
give macos a fighting chance to get available memory
toloudis May 19, 2026
c53b029
Merge branch 'feature/release1.10cleanup' into feature/volumecache
toloudis May 23, 2026
0e583cd
Fix data race: lock m_mutex around evictIfNeeded in setConfig
toloudis May 23, 2026
9604a43
Guard clearDiskCache against deleting non-cache directories
toloudis May 23, 2026
443e797
Silence C4267 narrowing warning in cache manager test
toloudis May 23, 2026
39bc997
Invalidate cache entries when the source file changes
toloudis May 24, 2026
832b1a9
Move available memory/disk queries into renderlib (no Qt deps)
toloudis May 24, 2026
f4fea24
Disk cache: evict before write, and close eviction race
toloudis May 24, 2026
f7630a8
Add disk-tier and mtime-invalidation tests for CacheManager
toloudis May 24, 2026
715b5de
Document the stale-disk-lastAccess edge case for RAM-resident entries
toloudis May 24, 2026
e80498a
Normalize filepaths in cache keys so equivalent paths share an entry
toloudis May 24, 2026
b0a5271
Add UNC path test for cache key normalization
toloudis May 24, 2026
a3e34de
Rename CacheManager::clear -> clearMemoryCache for symmetry with clea…
toloudis May 24, 2026
e35f84d
ensure that bpp loaded from cache is 16bpp
toloudis May 27, 2026
bbd86bc
allow the timeline widget to keep reusing its cached reader
toloudis May 27, 2026
654bbe0
reuse existing reader
toloudis May 27, 2026
a096257
DRY: only call makeKey once
toloudis May 27, 2026
cca3550
move the cache dir validity check earlier
toloudis May 27, 2026
275d7ad
Merge branch 'feature/volumecache' of https://github.com/allen-cell-a…
toloudis May 27, 2026
86304d5
refactor a function
toloudis May 27, 2026
0b4f9b5
don't allow changing the cache settings during rendering from Render …
toloudis Jun 1, 2026
7a2605a
prefer scoped_lock
toloudis Jun 1, 2026
70e79b1
handle error
toloudis Jun 1, 2026
0ea24c4
better macos instructions
toloudis Jun 1, 2026
f25a1f5
don't let users select cache directory
toloudis Jun 3, 2026
55ccbd5
keep cache dir fixed for duration of app. it is not user-selectable
toloudis Jun 3, 2026
f67632b
allow cachemanager to be constructed
toloudis Jun 3, 2026
f6764d5
lint fixes
toloudis Jun 3, 2026
6e92347
Merge branch 'main' into feature/volumecache
toloudis Jun 3, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions .github/agents/reviewer.agent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
---
description: Code reviewer for the AGAVE codebase. Reviews diffs and individual files for correctness, style, and architectural fit.
tools:
[
"codebase",
"search",
"usages",
"problems",
"changes",
"githubRepo",
"editFiles",
]
---

# AGAVE Code Reviewer

You review code changes in the AGAVE repository (C++17/Qt6 desktop app + Python client + JS web client). Be concise, specific, and cite file paths with line numbers.

## Review Scope

When invoked, determine what to review:

1. If the user names files or a PR, review those.
2. If the reviewer names a branch, review all changes in that branch relative to its base branch
3. Otherwise, inspect uncommitted/staged changes via the changes tool.
4. For each changed file, read enough surrounding context to understand intent — do not review lines in isolation.

## What To Check

### Architecture

- `renderlib/` must not depend on Qt. Flag any `Q*` types, `QObject`, signals/slots, or Qt headers leaking into `renderlib/`.
- GUI logic belongs in `agave_app/`. Rendering, I/O, scene, and serialization belong in `renderlib/`. It is preferable to have anything not necessary for the GUI to be pushed down into `renderlib/`.
- New commands must be added to **all** of: `renderlib/command.h`, `renderlib/command.cpp`, `agave_app/commandBuffer.cpp`, `test/test_commands.cpp`, `agave_pyclient/agave_pyclient/commandbuffer.py`, `agave_pyclient/agave_pyclient/agave.py`, `webclient/src/commandbuffer.ts`, `webclient/src/agave.ts`. Verify the integer ID is unique and consistent, and that the argument list/order matches across all locations.

### C++ Style

- C++17 only — no later-standard features.
- Classes / methods / enums: `PascalCase`. Member variables: `m_` prefix.
- Headers use `#pragma once`.
- Include order: local project headers → standard C++ → third-party → Qt.
- Prefer `const`, `constexpr`, references, and RAII. Flag raw `new`/`delete` outside ownership-transfer patterns already in use.
- Watch for missing `override`, unnecessary copies in range-for, signed/unsigned comparisons, and narrowing conversions (especially `size_t` ↔ `int`).
- We use clang-format with Mozilla-style (see .clang-format). Code should already be autoformatted.
- We use clang-tidy with a custom config (see .clang-tidy). Flag any warnings that are not explicitly disabled there.

### Python Style

- PEP 8 / `snake_case`.
- Should pass `ruff check`, `ruff format`.

### Correctness & Safety

- Qt signal/slot connections: confirm sender/receiver lifetimes and that lambdas capturing `this` are safe.
- OpenGL / GPU code in `renderlib/graphics/`: check resource cleanup, context currency, and that GL calls aren't made from non-GL threads.
- File I/O in `renderlib/io/`: validate bounds, handle malformed input, and avoid blocking the UI thread.
- Command protocol: `parse()` / `write()` field order must match the `CMD_ARGS` declaration exactly.
- OWASP-relevant issues: unchecked input sizes, path traversal in file loaders, integer overflow in image dimension math.

### Tests

- New commands require a round-trip test in `test/test_commands.cpp` (see `AGENTS.md` for the pattern).
- Non-trivial logic in `renderlib/` should have a Catch2 test.
- Python client changes should have a corresponding test in `agave_pyclient/tests/`.

### Build & Versioning

- New source files must be added to the relevant `CMakeLists.txt`.
- Version bumps go through `tbump` — flag manual edits to version strings.

## Output Format

Group findings by severity:

- **Blocking** — bugs, protocol mismatches, broken builds, security issues.
- **Should fix** — style violations, missing tests, architectural drift.
- **Nits** — minor suggestions, naming, comments.

For each finding, cite the file and line(s) and give a short rationale plus a concrete suggested change. If a finding is speculative, say so.

End with a one-line overall recommendation: _approve_, _approve with comments_, or _request changes_.

## Constraints

- Do not rewrite the whole change. Suggest targeted edits.
- Do not run the build or tests unless asked.
- If you edit files, limit edits to the specific fixes you flagged; do not opportunistically refactor unrelated code.
6 changes: 3 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ aqt install-qt --outputdir ~/Qt mac desktop 6.9.3 -m qtwebsockets qtimageformats
export Qt6_DIR=~/Qt/6.9.3/macos

mkdir build && cd build
cmake ..
make
cmake .. -G Ninja
cmake --build .
```

### Windows
Expand Down Expand Up @@ -90,7 +90,7 @@ clang-tidy -p build --fix renderlib/RenderSettings.cpp
### Python

- PEP 8 / snake_case
- Tooling: black, flake8, pyright (see `pyrightconfig.json`)
- Tooling: - `ruff check`, `ruff format`.

## Conventions

Expand Down
6 changes: 6 additions & 0 deletions agave_app/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ target_sources(agaveapp PRIVATE
"${CMAKE_CURRENT_SOURCE_DIR}/CameraDockWidget.h"
"${CMAKE_CURRENT_SOURCE_DIR}/CameraWidget.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/CameraWidget.h"
"${CMAKE_CURRENT_SOURCE_DIR}/CacheSettings.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/CacheSettings.h"
"${CMAKE_CURRENT_SOURCE_DIR}/CacheSettingsDockWidget.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/CacheSettingsDockWidget.h"
"${CMAKE_CURRENT_SOURCE_DIR}/CacheSettingsWidget.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/CacheSettingsWidget.h"
"${CMAKE_CURRENT_SOURCE_DIR}/citationDialog.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/citationDialog.h"
"${CMAKE_CURRENT_SOURCE_DIR}/cgiparser.cpp"
Expand Down
148 changes: 148 additions & 0 deletions agave_app/CacheSettings.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
#include "CacheSettings.h"

#include "renderlib/CacheManager.h"
#include "renderlib/Logging.h"
#include "renderlib/SystemInfo.h"

#include <QDir>
#include <QFile>
#include <QStandardPaths>

#include <nlohmann/json.hpp>

namespace {

::CacheConfig
toRenderlibConfig(const CacheSettingsData& data)
{
::CacheConfig config;
config.enabled = data.enabled;
config.enableDisk = data.enableDisk;

std::uint64_t availableMem = SystemInfo::availableMemoryBytes();
if (availableMem > 0) {
config.maxRamBytes = std::min(data.maxRamBytes, availableMem);
if (config.maxRamBytes < data.maxRamBytes) {
LOG_WARNING << "Cache RAM limit reduced from requested " << data.maxRamBytes << " to " << config.maxRamBytes
<< " bytes to fit available memory.";
}
} else {
config.maxRamBytes = data.maxRamBytes;
}

// The cache root is owned and writability-checked by CacheManager; clamp the
// disk limit against whatever filesystem it actually lives on.
std::uint64_t availableDisk = SystemInfo::availableDiskBytes(CacheManager::instance().getCacheDirectory());
if (availableDisk > 0) {
config.maxDiskBytes = std::min(data.maxDiskBytes, availableDisk);
if (config.maxDiskBytes < data.maxDiskBytes && data.enableDisk) {
LOG_WARNING << "Cache disk limit reduced from requested " << data.maxDiskBytes << " to " << config.maxDiskBytes
<< " bytes to fit available disk space.";
}
} else {
config.maxDiskBytes = data.maxDiskBytes;
}

if (!config.enableDisk) {
config.maxDiskBytes = 0;
}

return config;
}

} // namespace

CacheSettings::CacheSettings() = default;

CacheSettingsData
CacheSettings::defaultSettings() const
{
// Tunable defaults come from CacheSettingsData's in-class initializers.
return {};
}

std::string
CacheSettings::configPath() const
{
QString baseDir = QStandardPaths::writableLocation(QStandardPaths::AppConfigLocation);
if (baseDir.isEmpty()) {
baseDir = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation);
}
if (baseDir.isEmpty()) {
baseDir = QDir::currentPath();
}
QDir().mkpath(baseDir);
return QDir(baseDir).filePath("cache_settings.json").toStdString();
}

CacheSettingsData
CacheSettings::load()
{
CacheSettingsData data = defaultSettings();
QString path = QString::fromStdString(configPath());
QFile file(path);
if (!file.exists()) {
return data;
}
if (!file.open(QIODevice::ReadOnly)) {
return data;
}

QByteArray raw = file.readAll();
try {
nlohmann::json doc = nlohmann::json::parse(raw.toStdString());
if (doc.contains("enabled")) {
data.enabled = doc["enabled"].get<bool>();
}
if (doc.contains("enableDisk")) {
data.enableDisk = doc["enableDisk"].get<bool>();
}
if (doc.contains("maxRamBytes")) {
data.maxRamBytes = doc["maxRamBytes"].get<std::uint64_t>();
}
if (doc.contains("maxDiskBytes")) {
data.maxDiskBytes = doc["maxDiskBytes"].get<std::uint64_t>();
}
} catch (...) {
return defaultSettings();
}

return data;
}

bool
CacheSettings::save(const CacheSettingsData& data) const
{
nlohmann::json doc;
doc["enabled"] = data.enabled;
doc["enableDisk"] = data.enableDisk;
doc["maxRamBytes"] = data.maxRamBytes;
doc["maxDiskBytes"] = data.maxDiskBytes;

QString path = QString::fromStdString(configPath());
QFile file(path);
if (!file.open(QIODevice::WriteOnly)) {
return false;
}
std::string out = doc.dump(2);
file.write(out.c_str(), static_cast<int>(out.size()));
return true;
}

void
CacheSettings::applyToRenderlib(const CacheSettingsData& data) const
{
// The cache directory (and its writability) is settled once at startup in
// CacheManager::initialize(); if it wasn't writable the manager left its root
// unset, so a disk-enabled config here is simply honored as RAM-only. We only
// push the runtime tunables.
::CacheConfig config = toRenderlibConfig(data);
LOG_INFO << "Cache config: enabled=" << (config.enabled ? 1 : 0) << " ram_bytes=" << config.maxRamBytes
<< " disk_enabled=" << (config.enableDisk ? 1 : 0) << " disk_bytes=" << config.maxDiskBytes
<< " cache_dir=" << CacheManager::instance().getCacheDirectory();
CacheManager::instance().setConfig(config);
auto applied = CacheManager::instance().getConfig();
LOG_INFO << "Cache config applied: enabled=" << (applied.enabled ? 1 : 0) << " ram_bytes=" << applied.maxRamBytes
<< " disk_enabled=" << (applied.enableDisk ? 1 : 0) << " disk_bytes=" << applied.maxDiskBytes
<< " cache_dir=" << CacheManager::instance().getCacheDirectory();
}
26 changes: 26 additions & 0 deletions agave_app/CacheSettings.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#pragma once

#include <cstdint>
#include <string>

struct CacheSettingsData
{
bool enabled = true;
bool enableDisk = true;
std::uint64_t maxRamBytes = 4ULL * 1024ULL * 1024ULL * 1024ULL;
std::uint64_t maxDiskBytes = 100ULL * 1024ULL * 1024ULL * 1024ULL;
};

class CacheSettings
{
public:
CacheSettings();

CacheSettingsData load();
bool save(const CacheSettingsData& data) const;

void applyToRenderlib(const CacheSettingsData& data) const;

CacheSettingsData defaultSettings() const;
std::string configPath() const;
};
8 changes: 8 additions & 0 deletions agave_app/CacheSettingsDockWidget.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#include "CacheSettingsDockWidget.h"

CacheSettingsDockWidget::CacheSettingsDockWidget(QWidget* parent)
: QDockWidget(parent)
{
setWindowTitle(tr("Advanced Cache Settings"));
setWidget(&m_settingsWidget);
}
18 changes: 18 additions & 0 deletions agave_app/CacheSettingsDockWidget.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#pragma once

#include <QDockWidget>

#include "CacheSettingsWidget.h"

class CacheSettingsDockWidget : public QDockWidget
{
Q_OBJECT

public:
explicit CacheSettingsDockWidget(QWidget* parent = nullptr);

CacheSettingsWidget* widget() { return &m_settingsWidget; }
Comment thread
toloudis marked this conversation as resolved.

private:
CacheSettingsWidget m_settingsWidget;
};
62 changes: 62 additions & 0 deletions agave_app/CacheSettingsWidget.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#include "CacheSettingsWidget.h"

#include "renderlib/CacheManager.h"

#include <QFormLayout>

CacheSettingsWidget::CacheSettingsWidget(QWidget* parent)
: QWidget(parent)
{
auto* layout = new QFormLayout(this);

m_enableCache = new QCheckBox(tr("Enable cache"), this);
m_enableDisk = new QCheckBox(tr("Enable disk cache"), this);

m_ramLimitMB = new QSpinBox(this);
m_ramLimitMB->setRange(0, 1024 * 1024);
m_ramLimitMB->setSuffix(tr(" MB"));
m_ramLimitMB->setSingleStep(256);

m_diskLimitGB = new QSpinBox(this);
m_diskLimitGB->setRange(0, 1024 * 1024);
m_diskLimitGB->setSuffix(tr(" GB"));
m_diskLimitGB->setSingleStep(10);

m_cacheDirLabel = new QLabel(this);
// The cache directory is fixed (registered at startup); display it read-only.
m_cacheDirLabel->setText(QString::fromStdString(CacheManager::instance().getCacheDirectory()));
m_cacheDirLabel->setTextInteractionFlags(Qt::TextSelectableByMouse);
m_cacheDirLabel->setWordWrap(true);

m_applyButton = new QPushButton(tr("Apply"), this);
m_clearDiskButton = new QPushButton(tr("Clear disk cache"), this);

layout->addRow(m_enableCache);
layout->addRow(m_enableDisk);
layout->addRow(tr("RAM limit"), m_ramLimitMB);
layout->addRow(tr("Disk limit"), m_diskLimitGB);
layout->addRow(tr("Cache directory"), m_cacheDirLabel);
layout->addRow(QString(), m_applyButton);
layout->addRow(QString(), m_clearDiskButton);
setLayout(layout);
}

void
CacheSettingsWidget::setSettings(const CacheSettingsData& data)
{
m_enableCache->setChecked(data.enabled);
m_enableDisk->setChecked(data.enableDisk);
m_ramLimitMB->setValue(static_cast<int>(data.maxRamBytes / (1024ULL * 1024ULL)));
m_diskLimitGB->setValue(static_cast<int>(data.maxDiskBytes / (1024ULL * 1024ULL * 1024ULL)));
}

CacheSettingsData
CacheSettingsWidget::getSettings() const
{
CacheSettingsData data;
data.enabled = m_enableCache->isChecked();
data.enableDisk = m_enableDisk->isChecked();
data.maxRamBytes = static_cast<std::uint64_t>(m_ramLimitMB->value()) * 1024ULL * 1024ULL;
data.maxDiskBytes = static_cast<std::uint64_t>(m_diskLimitGB->value()) * 1024ULL * 1024ULL * 1024ULL;
return data;
}
Loading
Loading