kg: review follow-ups — observation-age stats, stats/health parity, embedding scan, hub install reconciliation by bawoodruff · Pull Request #4 · Cortexa-LLC/mcp

bawoodruff · 2026-08-25T22:04:05Z

Summary

Clears the four acknowledged-but-unfiled follow-ups from the PR #3 review rounds and the 2026-08-23 audit:

kg health observation-age stats (the ADR-009 scope cut; see ADR-009) — newest/oldest/median stored created_at, excluding zero-timestamp rows, rendered as ages in the human report. The median is positional and pinned by fixture rows dated 2020/2022, so a query returning the first, last, or a legacy row fails the test.
kg stats / kg health scope parity — both now share resolveScopeDB; an explicitly named scope that cannot be loaded errors instead of silently reporting the legacy database.
Embedding scan hardening (kglib) — the HNSW build converts float64 components and rejects vectors with unhandled component types; the old bare type assertion silently zeroed them, distorting every distance the vector participated in.
Hub install crash-window reconciliation — a hard kill between the current-symlink rename and the registry write left a graph answering 200-with-zero-results permanently. Server construction now rolls current back to the registered commit (the registry write is install's commit point); the orphaned commit directory is left for the next install's prune.

All four fixes are mutation-verified; go build/vet/test green in src/kg and src/kglib.

🤖 Generated with Claude Code

…nk repair, drop visibility Review of #4 found the scope-parity change regressed a case it did not intend. resolveScopeDB could not tell a scope named with --scope from one inherited out of config.json, so a stale defaultScope naming nothing — reachable, since SetDefaultScope does not verify the scope exists and config.json travels with a repo — turned `kg stats` from a legacy-database report into an error. The resolver now takes the requested scope separately: a NAMED scope that cannot load is still an error, while an inherited default falls back to the legacy database when the project has no scope configs at all. Two more from the same review: - The hub's reconcile pass collapsed "readlink failed" into "nothing to do", so a `current` symlink missing entirely — a partial copy, a restored backup, an operator — was left broken with every search 500ing, strictly worse than the split reconcile exists to fix. Missing links are now recreated from the registry; other readlink errors are logged, not silently skipped, as are registry entries failing path validation. - Dropping an embedding vector with unhandled component types is correct but as invisible as the zeroing it replaced, so the build logs the count once. New tests, each mutation-verified: kg stats routed through its own RunE (a test of the shared helper alone cannot show the caller still uses it) covering both the named-scope error and the inherited-default fallback; a missing-current reconcile case; humanAge's thresholds; and the report's age line asserted for order and real values rather than its label. Also from review: the median query builds its SKIP with strconv rather than embedding a shared constant in a format string, and the reconcile comment records the same-commit re-push residual it cannot detect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Re-review of #4 caught a severity escalation the last commit introduced. reconcileInstalls validates registry entries before joining them into paths, but dereferenced info.Commit to do it — and registry.json is hand-editable, where `{"graphs":{"g":null}}` unmarshals to a nil entry. Because the pass runs in the constructor, that panic aborted `kg hub serve` outright; before, the same entry only panicked inside a request, which net/http recovers per connection, so the hub kept serving every other graph. Nil entries are now skipped and logged. Also from the same review: - kg health no longer hard-fails when the observation count shifts mid-report. The median's SKIP offset came from counts taken by two earlier queries, so a concurrent kg index deleting rows made the offset overshoot and failed the entire run; collectObservationAge now takes its own count over the same predicate as the median query, and a vanished row set reports no age rather than an error. - reconcile logs the one state it cannot repair: current naming the registered commit whose directory is gone (searches fail, and the registry has no other answer). - printStats writes to an io.Writer like runHealth does, so the stats tests assert what was read — the previous fallback test passed even with stats bypassing the shared resolver entirely, which the reviewer demonstrated. - humanAge names negative durations instead of calling them "just now", and its year boundary is 12 30-day months so the ladder no longer prints "12mo" before "1y". - Fixture comments corrected to the six observations they now create. New tests for the null entry and the negative duration; the nil guard is mutation-verified (removing it panics the suite). Left as filed follow-ups, both from the reviewer: buildIndex's drop-on-reject branch stays uncovered until the loop body is extracted into a pure function, and the health/stats projectIDFromCwd divergence is provably cosmetic (findProjectRoot is idempotent) rather than a bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Four real runs since the reviewer went live show 40 is too snug, and that turn count tracks how far the reviewer wanders rather than how big the diff is: PR lines turns outcome #8 584 24 approved #5 2,704 35 posted a review <- largest diff, fewest turns #4 848 41 hit the 40 cap #7 989 41 hit the 40 cap The largest PR by a factor of three used the fewest turns, so sizing the cap to the diff is the wrong model. Both failures also stopped exactly AT the cap rather than near it, so the real ceiling is unknown — a snug cap will keep catching runs that were about to finish. Treat the cap as a runaway guard rather than a budget. Reviews run on a subscription OAuth token, so the reported total_cost_usd is notional rather than billed, and the binding limit is the job's timeout-minutes. At the observed ~6s/turn, 120 turns is roughly 12 minutes against a 30-minute wall clock, so the timeout still catches a genuine runaway. This matters more than a short review would, because exceeding the cap fails as a non-zero exit with an empty result, not as a truncated review — the run produces nothing at all. #4 and #7 currently have no review for that reason. num_turns is logged on every run, so if reviews start landing near 120 the signal is there rather than showing up as an unexplained silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Four acknowledged-but-unfiled items from this week's reviews: - kg health gains ADR-009's observation-age stats: newest/oldest/median stored created_at over timestamped observations (zero/NULL rows excluded — they are the zero-timestamp count, and would report every legacy-bearing graph as centuries old), rendered as ages in the human report. Median is positional (SKIP (n-1)/2), pinned by a fixture with known 2020/2022 rows so a query grabbing first, last, or a legacy row fails the test. - kg stats shares resolveScopeDB with kg health: an explicitly named scope that cannot be loaded is now an error, never a silent fallback to the legacy database. - kglib's HNSW build accepts float64 embedding components (converted) and rejects vectors with any other component type instead of silently zeroing them — a zeroed component distorted every distance the vector participated in, invisibly. - The hub reconciles installs at construction: a hard kill between repointing `current` and writing the registry left search reading one commit's database with another commit's ProjectID — 200 with zero results, permanently. The registry write is the commit point, so an unregistered `current` target now rolls back to the registered commit; the orphaned directory is left for the next install's prune. All four are mutation-verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…nk repair, drop visibility Review of #4 found the scope-parity change regressed a case it did not intend. resolveScopeDB could not tell a scope named with --scope from one inherited out of config.json, so a stale defaultScope naming nothing — reachable, since SetDefaultScope does not verify the scope exists and config.json travels with a repo — turned `kg stats` from a legacy-database report into an error. The resolver now takes the requested scope separately: a NAMED scope that cannot load is still an error, while an inherited default falls back to the legacy database when the project has no scope configs at all. Two more from the same review: - The hub's reconcile pass collapsed "readlink failed" into "nothing to do", so a `current` symlink missing entirely — a partial copy, a restored backup, an operator — was left broken with every search 500ing, strictly worse than the split reconcile exists to fix. Missing links are now recreated from the registry; other readlink errors are logged, not silently skipped, as are registry entries failing path validation. - Dropping an embedding vector with unhandled component types is correct but as invisible as the zeroing it replaced, so the build logs the count once. New tests, each mutation-verified: kg stats routed through its own RunE (a test of the shared helper alone cannot show the caller still uses it) covering both the named-scope error and the inherited-default fallback; a missing-current reconcile case; humanAge's thresholds; and the report's age line asserted for order and real values rather than its label. Also from review: the median query builds its SKIP with strconv rather than embedding a shared constant in a format string, and the reconcile comment records the same-commit re-push residual it cannot detect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Re-review of #4 caught a severity escalation the last commit introduced. reconcileInstalls validates registry entries before joining them into paths, but dereferenced info.Commit to do it — and registry.json is hand-editable, where `{"graphs":{"g":null}}` unmarshals to a nil entry. Because the pass runs in the constructor, that panic aborted `kg hub serve` outright; before, the same entry only panicked inside a request, which net/http recovers per connection, so the hub kept serving every other graph. Nil entries are now skipped and logged. Also from the same review: - kg health no longer hard-fails when the observation count shifts mid-report. The median's SKIP offset came from counts taken by two earlier queries, so a concurrent kg index deleting rows made the offset overshoot and failed the entire run; collectObservationAge now takes its own count over the same predicate as the median query, and a vanished row set reports no age rather than an error. - reconcile logs the one state it cannot repair: current naming the registered commit whose directory is gone (searches fail, and the registry has no other answer). - printStats writes to an io.Writer like runHealth does, so the stats tests assert what was read — the previous fallback test passed even with stats bypassing the shared resolver entirely, which the reviewer demonstrated. - humanAge names negative durations instead of calling them "just now", and its year boundary is 12 30-day months so the ladder no longer prints "12mo" before "1y". - Fixture comments corrected to the six observations they now create. New tests for the null entry and the negative duration; the nil guard is mutation-verified (removing it panics the suite). Left as filed follow-ups, both from the reviewer: buildIndex's drop-on-reject branch stays uncovered until the loop body is extracted into a pure function, and the health/stats projectIDFromCwd divergence is provably cosmetic (findProjectRoot is idempotent) rather than a bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

github-actions

Major Issues (should fix)

src/kg/internal/knowledge/health.go:157-182 (collectObservationAge, the max/min query) — Inconsistent race handling with the median query three lines below it. The function explicitly reasons that "a concurrent kg index deleting rows in between [the count and a follow-up query] would make the offset overshoot the result set" and handles that for the median query by returning (nil, nil) when !result.HasNext(). The max/min query is exposed to the exact same race (it also runs after the initial timestamped count, against the same non-transactional read connection), but its if result.HasNext() { ... } block has no else: if the aggregate returns no row in that narrow window, age.Newest and age.Oldest silently stay at Go's zero time.Time{} (year 1) instead of erroring or degrading to nil. That zero time then flows straight into printHealth's humanAge(gen.Sub(oa.Newest)), rendering something like "newest ~2053y" — reproducing, in a report command whose entire point (ADR-009, per this PR's own summary) is to stop exactly this "reports every legacy-bearing graph as centuries old" failure mode. This is a live concurrency concern for a tool explicitly meant to be used by multiple agents/processes against the same graph. Fix: mirror the median query's handling — if !result.HasNext() after the max/min query, return (nil, nil) (or propagate a clear error) rather than falling through with age still zero-valued.

Minor Issues (optional)

src/kglib/hnsw_index.go:126-127 — The dropped-vectors log line ("dropped %d of %d vectors read") computes the denominator as dropped+len(nodes), which excludes rows skipped earlier for missing/empty rawEmb (if !ok || len(rawEmb) == 0 { continue }). The message therefore understates the true number of rows the query returned whenever some entities have no embedding at all, making the "of %d" figure not actually "vectors read" but "vectors read that had a non-empty embedding column." Consider tracking a separate read counter incremented once per row, or rewording to "of %d vectors with embedding data."

Positive Observations

resolveScopeDB's split between "user-named scope" (always errors) and "inherited default scope" (falls back to legacy when no scope configs exist) is implemented correctly and is exercised by both the new named-scope-error test and the inherited-default-fallback regression test — matches the documented parity contract exactly, including the subtle case where a named scope must error even when no scope configs exist at all.
embeddingFromRaw correctly replaces the old silent-zeroing type assertion with an explicit float32/float64 accept-list and drop-on-unknown-type, with solid unit coverage of the float64/mixed/reject cases.
reconcileInstalls is careful and well-reasoned: it guards against nil registry entries and invalid path components before ever joining them into a filesystem path, distinguishes "current missing" from "current pointing at an unregistered commit" for logging purposes without a functional difference in the repair path, and correctly leaves data alone (rather than guessing) when the registered commit directory itself is missing or current is an unreadable non-symlink. The accompanying tests (interrupted seed, missing symlink, null registry entry) exercise real crash-window scenarios rather than just the happy path.
printStats/runStatsTo refactor cleanly threads an io.Writer through for testability, with no stale callers left on the old signature.

…eady handles Both findings from #4's first review. collectObservationAge runs two queries after its initial count, on the same non-transactional read connection, and both are exposed to a writer deleting rows in between. The median query says so in a comment and returns (nil, nil) when the row is gone. The max/min query above it had `if result.HasNext()` with no else, so in that window it fell through with Newest and Oldest still at the zero time. printHealth renders those as humanAge(gen.Sub(oa.Newest)) — roughly two thousand years. That is the "every legacy-bearing graph reads as centuries old" symptom this metric was added to expose, so the failure would have been indistinguishable from the bug it reports on. Mirror the median's handling. Hoisting the row read out of the if-block left the median's `row, err :=` declaring nothing new, so it becomes `=` with a note saying why. Also fixed the dropped-vector log's denominator. It computed dropped+len(nodes), but rows whose embedding column is missing or empty are skipped before `dropped` is incremented, so they appeared in neither term — the "of %d vectors read" figure silently excluded them. Count reads per row instead, so the number means what it says. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions

Review Summary

Files reviewed: src/kg/health.go, src/kg/health_test.go, src/kg/internal/hub/install_test.go, src/kg/internal/hub/server.go (diff + surrounding install/checkGraphOwnership/handleGraphSearch/expandLayers/handleFederatedSearch context), src/kg/internal/knowledge/health.go, src/kg/stats.go, src/kg/stats_test.go, src/kglib/hnsw_index.go, src/kglib/hnsw_index_test.go
Overall verdict: REQUEST CHANGES-worthy issues not present; Major issues found → comment
The two Major/Minor items flagged in the prior review round (median/max-min race asymmetry in collectObservationAge, and the dropped/read denominator in the hnsw log line) are both fixed correctly in this revision and are not re-raised.

Major Issues (should fix)

src/kg/internal/knowledge/health.go:176-190 (collectObservationAge, the max/min query) — The new if !result.HasNext() { return nil, nil } guard almost certainly never fires for this query, so it doesn't actually close the race it's meant to close. RETURN max(...), min(...) with no GROUP BY is a full-aggregation query: under standard Cypher semantics (which Kuzu follows), aggregation over zero matched rows still returns exactly one row, with max/min bound to NULL — it does not return zero rows the way the row-returning median query (ORDER BY ... SKIP ... LIMIT 1) does. So when the race actually happens (a concurrent kg index deletes the timestamped rows between the count and this query), result.HasNext() is still true, row.GetValue() returns a nil interface for both columns, and timeCell (line 244-254) hits its v.(time.Time) type-assertion failure, returning an error like timestamp cell has unexpected type <nil>. That error propagates up through CollectHealthMetrics (fmt.Errorf("collect observation age: %w", err)) and fails the whole kg health run — a command whose own doc comment says it "always exits 0 — it is a report, not a gate." So the race this code believes it closed instead trades "silently wrong age" for "the entire health report errors out," in the narrow window it's meant to guard. Fix: after getting the row, treat a nil/non-time.Time value from GetValue the same as the median query's !HasNext() case (return (nil, nil)) rather than routing it through timeCell's hard-error path — or have timeCell accept an "ok" return so the caller can distinguish "really missing" from "wrong type."
src/kg/internal/hub/server.go:348, 393-394, 419, 480-484, 578-579, 730-731 — This PR adds a nil-check for registry.Graphs entries in the newly-added reconcileInstalls (server.go:132-136) with a test proving {"graphs":{"g":null}} is a real, hand-editable state. But every other consumer of reg.Graphs[name] in this same file only checks ok, not whether the value itself is nil, and several dereference it unconditionally on the request path: handleGraphSearch (line 419 info, ok := ... → line 393 info.ProjectID inside searchGraph), expandLayers (line 480 info, ok := ... → line 484 info.Layers, reachable from handleGraphSearch's layer expansion), handleFederatedSearch (line 578 info := reg.Graphs[name] with no nil check at all, used at 579/info.Commit), and checkGraphOwnership (line 730-731 existing, ok := ...; existing.Repo, on the seed/push path). A registry hand-edited to hold a null entry for an existing graph name (not just the newly-added-and-not-yet-served case this PR tests) will nil-pointer-panic one of these request handlers instead of erroring cleanly — the same hazard this PR just spent a test and a fix on, one call away. Fix: give Registry (or these call sites) a helper like reg.Graphs[name] → (*GraphInfo, bool) that also treats a nil pointer as "not found," and use it at all five sites, or add info == nil checks alongside the existing !ok checks.

Minor Issues (optional)

None beyond what's already fixed from the prior review round.

Security Findings

None new. The nil-registry-entry panic above is a robustness/availability issue (self-inflicted via hand-edited registry.json), not something remotely triggerable — not rated as a security finding.

Positive Observations

The collectObservationAge median query's own race handling (its !result.HasNext() branch) is correct as written — that query is a genuine row-returning query, unlike the max/min aggregate above it, so the asymmetry the prior review flagged is now resolved for that half.
embeddingFromRaw and the dropped/read counters in src/kglib/hnsw_index.go are correctly fixed from the prior round; the new TestEmbeddingFromRaw covers float32/float64/mixed/reject cases well.
resolveScopeDB's split between "user-named scope" (always errors when unloadable) and "inherited default scope" (falls back to legacy only when no scope configs exist at all) is a real behavior fix over the old stats.go, which used to fall back to legacy for any scope (including explicitly-named ones) whenever len(configs) == 0 — the new logic and its tests (TestStatsErrorsOnUnloadableNamedScope, TestStatsFallsBackWhenInheritedDefaultScopeHasNoConfigs) correctly close that gap.
reconcileInstalls's handling of null/invalid registry entries, missing current, and unregistered current targets is careful, and the accompanying install_test.go tests exercise real crash-window scenarios (interrupted seed, missing symlink, null registry entry) rather than just the happy path.
The printStats/runStatsTo writer-threading refactor is clean and leaves no stale fmt.Print* callers behind.

…is time Two Majors from the second review round. The first is a correction to my own previous fix. The max/min guard I added last round never fires. `RETURN max(...), min(...)` has no GROUP BY, so it is a full aggregation: over zero matched rows Kuzu returns exactly ONE row with both columns NULL, not an empty result set. Verified directly against the engine rather than reasoned about — a probe against an empty store returns 1 row with nil in both columns. So !HasNext() is not what the race looks like, and the NULLs went straight to timeCell, whose type assertion turned them into "timestamp cell has unexpected type <nil>". That error propagates out of CollectHealthMetrics and fails the whole run — for a command whose own doc says it always exits 0, being a report and not a gate. The previous fix therefore traded a silently wrong age for a dead report in exactly the window it meant to protect. Handle the NULL case where it actually occurs, in a new ageBoundsFromRow that reports presence separately from error. A NULL means the rows went away, so it degrades to no age like the median query does; a non-NULL value of the wrong type stays a hard error, since that is a real defect and silent zeroing is the bug this metric exists to catch. timeCell now delegates to a value-level timeValue so an already-read cell can reuse it. Tested through the existing row interface seam, covering all three outcomes, and mutation-checked. Second: this PR added a nil guard to reconcileInstalls with a test proving `{"graphs":{"g":null}}` is a real hand-editable state, but every other lookup in server.go checked only `ok`. Graphs is map[string]*GraphInfo, so a null entry is a present key with a nil value — ok is true and the next dereference panics. Five request-path sites were exposed, one of them (federated search) with no check at all before reading info.Commit. Route every read through a Registry.graph helper that treats nil as absent. The test for it needed a second attempt worth recording: asserting merely "not 200" passed even with the guard removed, because net/http recovers a handler panic into a 500. Asserting 404 specifically is what makes it real — the mutation now fails with 500 and the panic's error body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions

Review Summary

Files reviewed: src/kg/health.go, src/kg/health_test.go, src/kg/stats.go, src/kg/stats_test.go, src/kg/internal/knowledge/health.go, src/kg/internal/knowledge/health_test.go, src/kg/internal/hub/server.go, src/kg/internal/hub/registry.go, src/kg/internal/hub/install_test.go, src/kglib/hnsw_index.go, src/kglib/hnsw_index_test.go (plus surrounding context: install, checkGraphOwnership, handleGraphSearch/handleFederatedSearch/expandLayers, resolveScopeDB call graph, and module go.mod/replace wiring)
Both Major issues raised in the prior review rounds are verified fixed in this revision and are not re-raised:
- collectObservationAge/ageBoundsFromRow (src/kg/internal/knowledge/health.go:176-291) now correctly treats Kuzu's one-row-of-NULLs full-aggregation result as "bounds absent" (checking newestRaw == nil || oldestRaw == nil) rather than relying on !HasNext(), which — as the second review round correctly diagnosed — never fires for a RETURN max(...), min(...) query. This is covered by a new direct unit test (TestAgeBoundsFromRow) exercising present/NULL/wrong-type cases without needing to race a real writer.
- The null-registry-entry hazard (src/kg/internal/hub/registry.go:30-45, server.go) is fixed with a Registry.graph(name) helper that treats a nil pointer as absent, and every request-path read site (handleGetGraph, handleGraphSearch, expandLayers (both the outer and inner lookups), handleFederatedSearch (both the enumerate-all and named-list paths), checkGraphOwnership) now goes through it. reconcileInstalls also guards its own loop. I traced all remaining direct reg.Graphs[...] accesses and confirmed they are either writes (install, handleSeed) or test-only.

Critical Issues

None.

Major Issues

None.

Minor Issues (optional)

None beyond what was already fixed from prior rounds.

Security Findings

None.

Positive Observations

reconcileInstalls's doc comment carefully scopes what it can and cannot repair (explicitly calling out the one residual gap — a same-commit re-push interrupted before the registry write — as a known, deliberately-unhandled case) rather than overclaiming; the accompanying tests (TestReconcileRollsBackInterruptedSeed, TestReconcileRestoresMissingCurrent, TestReconcileSurvivesNullRegistryEntry, TestRequestHandlersSurviveNullRegistryEntry) exercise real crash-window states, not just happy paths.
resolveScopeDB's consolidation of kg health's and kg stats' previously-duplicated scope-resolution logic is a genuine simplification, with the intentional named-vs-inherited-default asymmetry preserved and independently tested for both callers.
humanAge's threshold ladder and the median/oldest/newest ordering in the human-readable report are pinned with precise boundary tests (TestHumanAge, TestHealthHumanOutputAgeLine) rather than loose substring checks.
embeddingFromRaw's float32/float64 accept-list with explicit rejection of other types (instead of silently zeroing unknown components) is correctly reasoned and unit-tested for the float64/mixed/reject cases, and the dropped/read counter fix from the prior review round is applied correctly.

bawoodruff closed this Aug 28, 2026

bawoodruff reopened this Aug 28, 2026

This was referenced Aug 28, 2026

ci: raise the reviewer turn cap to 120 #11

Merged

ci: enforce gofmt, and format the files that were failing it #12

Merged

bawoodruff and others added 3 commits August 29, 2026 14:53

bawoodruff force-pushed the chore/review-followups branch from 2392fcf to 03cb73d Compare August 29, 2026 21:54

github-actions Bot reviewed Aug 29, 2026

View reviewed changes

github-actions Bot approved these changes Aug 29, 2026

View reviewed changes

bawoodruff merged commit c9af86e into main Aug 29, 2026
6 checks passed

bawoodruff deleted the chore/review-followups branch August 29, 2026 22:26

Navigation Menu

kg: review follow-ups — observation-age stats, stats/health parity, embedding scan, hub install reconciliation - #4

kg: review follow-ups — observation-age stats, stats/health parity, embedding scan, hub install reconciliation#4
bawoodruff merged 5 commits into
mainfrom
chore/review-followups

bawoodruff commented Aug 25, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bawoodruff commented Aug 25, 2026

Summary

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Major Issues (should fix)

Minor Issues (optional)

Positive Observations

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review Summary

Major Issues (should fix)

Minor Issues (optional)

Security Findings

Positive Observations

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review Summary

Critical Issues

Major Issues

Minor Issues (optional)

Security Findings

Positive Observations

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant