fix: instanceof bug, partial flush clearing, strict type params, events-only sum

- extractGroupByInterval: match by method string, not instanceof (parsed queries are base Query)
- flush(): selectively clear buffer on per-batch success (retry preserved on failure)
- collect(): use TYPE_EVENT constant instead of string literal
- addBatch(): require explicit \$type param (no default)
- sum(): events-only by default (summing gauges is meaningless)
- sumDaily*: document as events-only (daily MV has only events)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: lohanidamodar

src/Usage/Adapter.php

@@ -106,18 +106,23 @@ abstract public function count(array $queries = [], ?string $type = null): int;
     /**
      * Sum metric values using Query objects.
      *
+     * Events-only by default because summing gauges is semantically meaningless
+     * (adding point-in-time snapshots doesn't produce a useful total).
+     *
      * @param array<\Utopia\Query\Query> $queries
      * @param string $attribute Attribute to sum (default: 'value')
-     * @param string|null $type Metric type: 'event', 'gauge', or null (sum both)
+     * @param string $type Metric type: 'event' or 'gauge'
      * @return int
      */
-    abstract public function sum(array $queries = [], string $attribute = 'value', ?string $type = null): int;
+    abstract public function sum(array $queries = [], string $attribute = 'value', string $type = Usage::TYPE_EVENT): int;
 
     /**
-     * Sum event metrics from the pre-aggregated daily table.
+     * Find event metrics from the pre-aggregated daily table.
      *
      * Queries the SummingMergeTree daily materialized view for fast billing/analytics.
-     * Only works for event metrics (gauges are not pre-aggregated).
+     *
+     * Note: Daily MV only stores event metrics. This method always queries
+     * the daily events table — gauges are never pre-aggregated.
      *
      * @param array<\Utopia\Query\Query> $queries  Filters (metric, time range, resource, etc.)
      * @return array<Metric>
@@ -127,6 +132,9 @@ abstract public function findDaily(array $queries = []): array;
     /**
      * Sum event metric values from the pre-aggregated daily table.
      *
+     * Note: Daily MV only stores event metrics. This method always queries
+     * the daily events table — gauges are never pre-aggregated.
+     *
      * @param array<\Utopia\Query\Query> $queries
      * @param string $attribute Attribute to sum (default: 'value')
      * @return int
@@ -136,6 +144,9 @@ abstract public function sumDaily(array $queries = [], string $attribute = 'valu
     /**
      * Sum multiple event metrics from the pre-aggregated daily table in one query.
      *
+     * Note: Daily MV only stores event metrics. This method always queries
+     * the daily events table — gauges are never pre-aggregated.
+     *
      * @param array<string> $metrics List of metric names
      * @param array<\Utopia\Query\Query> $queries Additional filters (e.g. date range)
      * @return array<string, int> Metric name => sum value

src/Usage/Adapter/ClickHouse.php

@@ -1334,7 +1334,7 @@ private function validateMetricsBatch(array $metrics, string $type): void
      * @param  int  $batchSize  Maximum number of metrics per INSERT statement
      * @throws Exception
      */
-    public function addBatch(array $metrics, string $type = Usage::TYPE_EVENT, int $batchSize = self::INSERT_BATCH_SIZE): bool
+    public function addBatch(array $metrics, string $type, int $batchSize = self::INSERT_BATCH_SIZE): bool
     {
         if (empty($metrics)) {
             return true;
@@ -1683,23 +1683,19 @@ private function countFromTable(array $queries, string $type): int
     /**
      * Sum metric values using Query objects.
      *
+     * Events-only by default — summing gauges is semantically meaningless.
+     *
      * @param array<Query> $queries
      * @param string $attribute Attribute to sum (default: 'value')
-     * @param string|null $type 'event', 'gauge', or null (both)
+     * @param string $type 'event' or 'gauge'
      * @return int
      * @throws Exception
      */
-    public function sum(array $queries = [], string $attribute = 'value', ?string $type = null): int
+    public function sum(array $queries = [], string $attribute = 'value', string $type = Usage::TYPE_EVENT): int
     {
         $this->setOperationContext('sum()');
 
-        if ($type !== null) {
-            return $this->sumFromTable($queries, $attribute, $type);
-        }
-
-        // Sum from both tables
-        return $this->sumFromTable($queries, $attribute, Usage::TYPE_EVENT)
-             + $this->sumFromTable($queries, $attribute, Usage::TYPE_GAUGE);
+        return $this->sumFromTable($queries, $attribute, $type);
     }
 
     /**

src/Usage/Adapter/Database.php

@@ -108,7 +108,7 @@ protected function getColumnDefinition(string $id, string $type = 'event'): stri
      * @return bool
      * @throws \Exception
      */
-    public function addBatch(array $metrics, string $type = Usage::TYPE_EVENT, int $batchSize = 1000): bool
+    public function addBatch(array $metrics, string $type, int $batchSize = 1000): bool
     {
         $this->db->getAuthorization()->skip(function () use ($metrics, $type, $batchSize) {
             $documents = [];
@@ -256,12 +256,14 @@ public function getTotalBatch(array $metrics, array $queries = [], ?string $type
     /**
      * Sum metric values.
      *
+     * Events-only by default — summing gauges is semantically meaningless.
+     *
      * @param array<Query> $queries
      * @param string $attribute
-     * @param string|null $type
+     * @param string $type 'event' or 'gauge'
      * @return int
      */
-    public function sum(array $queries = [], string $attribute = 'value', ?string $type = null): int
+    public function sum(array $queries = [], string $attribute = 'value', string $type = Usage::TYPE_EVENT): int
     {
         /** @var array<Metric> $results */
         $results = $this->find($queries, $type);

src/Usage/Usage.php

@@ -84,13 +84,16 @@ public function setup(): void
     /**
      * Add metrics in batch (raw append).
      *
+     * Callers must explicitly pass the metric type so event and gauge
+     * writes are never confused at the call site.
+     *
      * @param array<array{metric: string, value: int, tags?: array<string,mixed>}> $metrics
      * @param string $type Metric type: 'event' or 'gauge'
      * @param int $batchSize Maximum number of metrics per INSERT statement
      * @return bool
      * @throws \Exception
      */
-    public function addBatch(array $metrics, string $type = self::TYPE_EVENT, int $batchSize = 1000): bool
+    public function addBatch(array $metrics, string $type, int $batchSize = 1000): bool
     {
         return $this->adapter->addBatch($metrics, $type, $batchSize);
     }
@@ -183,14 +186,23 @@ public function count(array $queries = [], ?string $type = null): int
     /**
      * Sum metric values using Query objects.
      *
+     * Defaults to events because summing gauges (point-in-time snapshots)
+     * is semantically meaningless — it averages/accumulates snapshots rather
+     * than producing a useful total. Callers that truly want a gauge sum
+     * must opt in explicitly.
+     *
      * @param array<\Utopia\Query\Query> $queries
      * @param string $attribute Attribute to sum (default: 'value')
-     * @param string|null $type Metric type: 'event', 'gauge', or null (sum both)
+     * @param string $type Metric type: 'event' or 'gauge'
      * @return int
      * @throws \Exception
      */
-    public function sum(array $queries = [], string $attribute = 'value', ?string $type = null): int
+    public function sum(array $queries = [], string $attribute = 'value', string $type = self::TYPE_EVENT): int
     {
+        if ($type !== self::TYPE_EVENT && $type !== self::TYPE_GAUGE) {
+            throw new \InvalidArgumentException("Invalid type '{$type}'. Allowed: " . self::TYPE_EVENT . ', ' . self::TYPE_GAUGE);
+        }
+
         return $this->adapter->sum($queries, $attribute, $type);
     }
 
@@ -199,6 +211,9 @@ public function sum(array $queries = [], string $attribute = 'value', ?string $t
      *
      * Queries the SummingMergeTree daily MV for fast billing/analytics.
      *
+     * Note: Daily MV only stores event metrics. This method always queries
+     * the daily events table — gauges are never pre-aggregated.
+     *
      * @param array<\Utopia\Query\Query> $queries
      * @return array<Metric>
      * @throws \Exception
@@ -214,6 +229,9 @@ public function findDaily(array $queries = []): array
      * Use this for billing queries — reads pre-aggregated daily rows
      * instead of scanning billions of raw events.
      *
+     * Note: Daily MV only stores event metrics. This method always queries
+     * the daily events table — gauges are never pre-aggregated.
+     *
      * @param array<\Utopia\Query\Query> $queries
      * @param string $attribute Attribute to sum (default: 'value')
      * @return int
@@ -227,6 +245,9 @@ public function sumDaily(array $queries = [], string $attribute = 'value'): int
     /**
      * Sum multiple event metrics from the pre-aggregated daily table in one query.
      *
+     * Note: Daily MV only stores event metrics. This method always queries
+     * the daily events table — gauges are never pre-aggregated.
+     *
      * @param array<string> $metrics List of metric names
      * @param array<\Utopia\Query\Query> $queries Additional filters (e.g. date range)
      * @return array<string, int> Metric name => sum value
@@ -302,7 +323,7 @@ public function collect(string $metric, int $value, string $type, array $tags =
         $tagsHash = !empty($tags) ? md5(json_encode($tags, JSON_THROW_ON_ERROR)) : '';
         $key = $metric . ':' . $type . ':' . $tagsHash;
 
-        if ($type === 'event') {
+        if ($type === self::TYPE_EVENT) {
             // Additive: sum values for the same metric + tags combination
             if (isset($this->buffer[$key])) {
                 $this->buffer[$key]['value'] += $value;
@@ -333,9 +354,15 @@ public function collect(string $metric, int $value, string $type, array $tags =
      * Flush the in-memory buffer to storage.
      *
      * Separates buffered metrics into events and gauges, then writes each batch
-     * to the appropriate table via addBatch().
+     * to the appropriate table via addBatch(). Only entries whose batch write
+     * succeeds are removed from the buffer, so a partial failure preserves
+     * the unwritten metrics for retry on the next flush.
      *
-     * @return bool True if flush succeeded (or buffer was empty)
+     * If addBatch() throws mid-flush, any earlier successful batches have
+     * already been cleared from the buffer — the exception is allowed to
+     * propagate so the caller can observe the failure.
+     *
+     * @return bool True if all batches succeeded (or buffer was empty)
      * @throws \Exception
      */
     public function flush(): bool
@@ -345,35 +372,51 @@ public function flush(): bool
             return true;
         }
 
-        // Separate events and gauges
+        // Separate events and gauges; keep track of buffer keys so we can
+        // selectively unset only the entries whose write succeeded.
+        $eventKeys = [];
+        $gaugeKeys = [];
         $events = [];
         $gauges = [];
 
-        foreach ($this->buffer as $entry) {
+        foreach ($this->buffer as $key => $entry) {
             if ($entry['type'] === self::TYPE_EVENT) {
                 $events[] = $entry;
+                $eventKeys[] = $key;
             } else {
                 $gauges[] = $entry;
+                $gaugeKeys[] = $key;
             }
         }
 
-        $result = true;
+        $overallResult = true;
 
-        // Flush events to events table
+        // Flush events — clear buffer entries only on success.
         if (!empty($events)) {
-            $result = $this->adapter->addBatch($events, self::TYPE_EVENT);
+            if ($this->adapter->addBatch($events, self::TYPE_EVENT)) {
+                foreach ($eventKeys as $key) {
+                    unset($this->buffer[$key]);
+                }
+            } else {
+                $overallResult = false;
+            }
         }
 
-        // Flush gauges to gauges table
+        // Flush gauges — clear buffer entries only on success.
         if (!empty($gauges)) {
-            $result = $this->adapter->addBatch($gauges, self::TYPE_GAUGE) && $result;
+            if ($this->adapter->addBatch($gauges, self::TYPE_GAUGE)) {
+                foreach ($gaugeKeys as $key) {
+                    unset($this->buffer[$key]);
+                }
+            } else {
+                $overallResult = false;
+            }
         }
 
-        $this->buffer = [];
-        $this->bufferCount = 0;
+        $this->bufferCount = count($this->buffer);
         $this->lastFlushTime = microtime(true);
 
-        return $result;
+        return $overallResult;
     }
 
     /**

src/Usage/UsageQuery.php

@@ -91,13 +91,16 @@ public static function isGroupByInterval(Query $query): bool
     /**
      * Extract the groupByInterval query from an array of queries, if present.
      *
+     * Queries parsed via `Query::parse()` are base `Query` objects rather than
+     * `UsageQuery` instances, so we match on the method string alone.
+     *
      * @param array<Query> $queries
-     * @return self|null The groupByInterval query, or null if not present
+     * @return Query|null The groupByInterval query, or null if not present
      */
-    public static function extractGroupByInterval(array $queries): ?self
+    public static function extractGroupByInterval(array $queries): ?Query
     {
         foreach ($queries as $query) {
-            if ($query instanceof self && $query->getMethod() === self::TYPE_GROUP_BY_INTERVAL) {
+            if ($query->getMethod() === self::TYPE_GROUP_BY_INTERVAL) {
                 return $query;
             }
         }

tests/Usage/UsageQueryTest.php

@@ -62,7 +62,23 @@ public function testExtractGroupByInterval(): void
         $extracted = UsageQuery::extractGroupByInterval($queries);
 
         $this->assertNotNull($extracted);
-        $this->assertInstanceOf(UsageQuery::class, $extracted);
+        $this->assertInstanceOf(Query::class, $extracted);
+        $this->assertEquals(UsageQuery::TYPE_GROUP_BY_INTERVAL, $extracted->getMethod());
+        $this->assertEquals('1h', $extracted->getValue());
+    }
+
+    public function testExtractGroupByIntervalFromParsedQuery(): void
+    {
+        // Queries created via Query::parse() are base Query objects, not UsageQuery.
+        $parsedGroupBy = new Query(UsageQuery::TYPE_GROUP_BY_INTERVAL, 'time', ['1h']);
+        $equalQuery = Query::equal('metric', ['bandwidth']);
+
+        $queries = [$equalQuery, $parsedGroupBy];
+
+        $extracted = UsageQuery::extractGroupByInterval($queries);
+
+        $this->assertNotNull($extracted);
+        $this->assertEquals(UsageQuery::TYPE_GROUP_BY_INTERVAL, $extracted->getMethod());
         $this->assertEquals('1h', $extracted->getValue());
     }