feat(icons): adopt the ADR-077 semantic icon vocabulary - #2206
Merged
rubenvdlinde merged 31 commits intoJul 30, 2026
Conversation
…wiring, schema dedup)
Snapshot of the local working tree taken before reconciling with
origin/development, which had moved 69 commits ahead.
Much of this tree is already upstream (27 of 36 new PHP files and 13 of 49
modified files are byte-identical to origin/development). The genuinely new
work preserved here is:
- lib/AppHost/{Controller,Service}: generic settings plane
(GenericSettingsControllerBase, GenericSettingsService, RegisterConfigResolver)
- lib/Command/DedupCollidedSchemasCommand.php
- unit tests for the above plus HandlesExceptionsTrait
- four openspec changes (apphost-settings-plane, apphost-schedule-flow-action,
app-declared-credential-providers, or-flow-object-write-node)
- lib/Settings/flow_register.json and flow e2e coverage
Committed on a stale base on purpose so the subsequent merge of
origin/development is a real three-way merge rather than a tree overwrite.
…026-07-28 # Conflicts: # lib/Capabilities/IntegrationsCapability.php # lib/Service/Flow/FlowRunService.php # lib/Service/Flow/FlowTriggerService.php # lib/Service/Flow/IFlowResolver.php # lib/Service/Flow/Nodes/SubFlowNode.php # lib/Service/Flow/OpenRegisterFlowResolver.php # lib/Settings/flow_register.json # tests/Unit/Db/SchemaAnnotationVocabularyTest.php # tests/e2e/api-direct/openconnector-flow-nodes.spec.ts
…backlog Measured on the shared dev instance: 108,151 of 227,063 audit rows (48%) carry no hash, interleaved across the whole id range (31,518..290,493 = min..max id). Three defects, all evidenced: 1. insertHashChained() seals per row under a global exclusive advisory lock (3 attempts x 50ms, fail-soft). Under any concurrency each insert pays up to 150ms and then abandons the seal anyway. Measured ~152 rows/min during the maintenance:repair that had to be killed after 74 minutes with ~12h left. The batched sealRows() path already exists but single inserts never reach it. 2. The backfill that specs/audit-hash-chain/spec.md:105 requires does not exist. harden-audit-seal-concurrency (12/12 complete) made the lock fail-soft on the explicit promise that a "later seal pass chains them" -- that pass was never built, so every contended write permanently degrades the chain. 3. verifyChain() skips ANY null hash and still returns valid: true. The comment claims "pre-migration entries" but no cutover marker exists in lib/, so the tamper-evidence check currently passes over a table that is 48% unverified. The change specifies a windowed driver around sealRows() (one lock per window, not per row), a two-phase read so sealed rows do not have their ~5.3KB payloads fetched just to contribute a chain link, a partial index for the backlog cursor (today it pkey-scans with a filter at 784ms/2000 ids), a hard-capped background job plus an occ command, and a cutover marker so unsealed rows stop hiding behind a passing verification. Design records why a naive bulk call is impossible: sealRowsLocked() SELECTs * over [min,max], which for this backlog is ~227k rows x 5,270B ~= 1.14GB in PHP.
…on drift
A single-object create currently takes 13-99s on the dev instance (six runs on
larpingapp/character, two-field payload each time: 13.6/17.8/20.4/41.0/62.8/99.1s).
This is NOT the CloudEvent storm — that was openconnector's inert recursion
guard, fixed separately, and a create now emits 1 event rather than 255. The
remainder is our own write path.
Counter deltas across one HTTP 201 create (the 41.0s run):
sequential scans of oc_openregister_schemas 5,135
sequential scans of oc_openregister_registers 6
transactions committed 12,541
Four measured costs:
1. 5,135 schema resolutions against a 1,917-row table. SchemaMapper::find()
HAS a request cache and is a shared service, so this is either a cache
key too specific (rbac/multitenancy flags multiply it 4x) or an uncached
sibling on the hot path. The query is a seq scan by construction —
SELECT * hydrates a ~2KB properties blob and LOWER(slug) defeats any
index ('Rows Removed by Filter: 1916'). ~4ms x 5,135 = ~20s, half the run.
2. Resolving an object reference whose table is unknown emits a UNION ALL
with one branch per magic table. At 2,728 tables that is 690KB of SQL:
planning 3,404.9ms, execution 546.1ms. 86% of the cost is PARSING a
statement that returns zero rows, so no index can help — and it is
usually avoidable, since character's six relation properties each
already declare their target schema.
3. 12,541 commits for one create: essentially every statement autocommits,
and the request waits on each fsync.
4. CloudEvent fan-out, audit-trail sealing (228,932 rows), notification
history and oc_activity all run before the response is returned.
Target p95 <500ms with the 2,728-table shape unchanged — fixing the write
path, not shrinking the dataset. Task 1 is deliberately 'attribute the 5,135
calls before changing anything': guessing at a hot path is how the CloudEvent
guard stayed inert for so long.
Also fixes an unrelated red gate: openspec/specs/saved-search-views/spec.md was
missing the blank line before '## Requirements', so the features-manifest
generator swallowed the heading into the feature summary and docs/features.json
drifted. 'quality / Features Check' fails on development because of it.
Conduction's standard requires named arguments on internal calls, forbids inline IFs, and has its own spacing///end conventions — the command shipped with 21 violations and turned 'quality / PHP Quality (phpcs)' red. 14 fixed by phpcbf; the rest by hand: setName/setDescription/addOption -> named arguments (four calls) splitOne/splitOneLocked/findCollisions/pickOwner call sites -> named arguments the two ternaries in execute() -> explicit if blocks All 11 PHP files changed on this branch now pass phpcs.
lib/ was 15 errors over 9 files before this branch, so 'quality / PHP Quality
(phpcs)' failed on development and on every PR opened against it.
6x curl_close($ch) deprecated since PHP 8.0 and a no-op — CurlHandle is
an object freed when it leaves scope, not a resource
needing an explicit close. Removed, with a comment so
nobody re-adds them.
3x missing @PARAM FlowRunController::__construct($userSession),
FederatedConfigService::publish($private),
FlowScheduleService::fire($owner)
6x file header ReconcileDeclaredBackgroundJobs.php put its docblock
AFTER declare(strict_types=1), so phpcs read it as a
stray inline block rather than the file header; tag
order was @author/@license/@copyright. Moved above the
declare and reordered to @author/@copyright/@license.
phpcs now reports 0 errors across all 72 files in lib/. phpstan is unchanged —
the same 7 findings exist with and without this commit (verified by stashing),
so nothing here introduced or masked one.
ObjectService::find() takes register+schema, does a scoped lookup, and on a
miss retries. The retry dropped BOTH the register and the schema, so a
legitimate 'not in this register' answer was produced by scanning every magic
table on the instance.
That scan is a UNION with one branch per magic table. At 2,728 tables it is
690 KB of SQL, and its cost is almost entirely PLANNING:
Planning Time: 3404.926 ms
Execution Time: 546.145 ms <- returns zero rows
No index can reduce parse time, so the only fix is to not emit it.
The fallback exists for a stale or sibling SCHEMA inside a register the caller
named correctly (openbuild#75 / openregister#1520) — it was never meant to
search other registers. It now keeps the register and drops only the schema,
and MagicMapper::find()/findAcrossAllSources() accept a registerIdScope that
filters candidate tables by register id.
Why this mattered on the write path: the flow-resolver registry asks every
resolver in turn whether a flow is theirs. Each non-owning resolver called
find(register: <its own>, schema: 'flow'), missed correctly, and paid a full
instance-wide scan to say 'not mine'. Measured 2026-07-29, that single widened
fallback was ~1.9s of a ~3.0s create.
event dispatch inside the create 1,900ms -> 80-187ms
wall (median of 5) 3.2s -> 1.55s
Also adds the measurement half of the change:
- tests/perf/object-create.sh reports min/median/p95 plus per-write schema
seq scans and commit count, and separately measures the INSTANCE FLOOR (an
authenticated request doing no object work). Nextcloud boots every enabled
app per request; on this instance, with 92 apps, that floor is ~864ms —
larger than the whole 500ms budget — so wall time alone cannot tell you
whether the write path regressed.
- SchemaMapper::traceRead() attributes every uncached schema read to its
caller (gated on the perf_trace_schema_reads app-config flag); this is
what identified the
1,471-call DocuDesk path.
- WritePhaseProbe times the write path's phases; this is what showed the cost
was event dispatch rather than the INSERT (76ms).
Current state: wall p95 1,421ms = 864ms instance floor + 557ms write path.
Refs openspec/changes/object-write-sub-500ms tasks 0, 1, 5.
FlowResolverRegistry::resolveFlow() asks every registered resolver in turn. A resolver that does not own the flow answers by looking the id up in its own register — so an unowned flow pays the entire chain, and every answer costs a database round trip. FlowTriggerService::fire() calls it once per queued run, and it calls it from runInline() BEFORE checking whether the flow is even synchronous, so an async flow pays a full resolution for nothing. A save that cascades fires the same triggers again for each child, so one object write resolved the same ids several times over. Memoised per request, misses included: 'no app owns this flow' is the expensive answer, since producing it requires every resolver to look and fail. Refs openspec/changes/object-write-sub-500ms.
… write ObjectCreatedEvent has ~22 listeners across the fleet. Dispatched inline the caller waits for all of them: measured 2026-07-29 it was 234-501ms of a ~530ms create, against a 76ms insert. None of it is needed to tell the caller its object was saved. Opt-in via 'occ config:app:set openregister defer_object_events --value=1'. Off by default, because deferring changes an observable contract: a 2xx stops meaning 'and every side effect has been applied'. A flow declaring executionMode: sync exists precisely so its effects land before the save returns, so this cannot be flipped on for everyone by fiat. event dispatch inside the create 234-501ms -> 6-8ms write path p95 (wall minus floor) 557ms -> 300ms CARRIES THE ACTING USER, and that is not incidental. The first version did not, and it was a perfectly green no-op: the job ran, logged nothing, threw nothing — and produced ZERO CloudEvents where the inline path produced one. A background job has no session, and OpenRegister reads are organisation- filtered against the session user, so every listener that consults the register (the CloudEvent firehose gate most visibly) saw an empty instance and skipped. Deferring side effects without carrying identity does not move the work, it deletes it. Verified after the fix: deferred dispatch produces exactly 1 CloudEvent, matching inline. The impersonation is released in a finally so it cannot leak into whatever job the worker runs next from the same process, and a missing acting user is logged at WARNING (not INFO — the default loglevel is 2, and a side effect that silently vanished is exactly what must not be filtered out).
Two corrections the measurements forced, both of which change what the requirement can mean: 1. The budget must be wall time MINUS THE INSTANCE FLOOR. An authenticated request doing no object work costs 864-1,099ms on this instance, because Nextcloud boots all 92 enabled apps per request. That is larger than the whole 500ms budget, it is PHP-side (boot issues almost no queries, and opcache is healthy: 0 OOM restarts, 91% hit rate), and no work in the write path can remove it. A measurement that does not subtract it reports how many apps are installed. 2. Deferred dispatch must carry the acting user. Added as an explicit requirement and scenario after the first implementation shipped a flawless no-op — zero CloudEvents, no exception, no log. Results recorded: write path 12.8s -> 183ms median / 300ms p95, inside budget. Wall 13.7s -> 1.28s. Task 5 is ticked but was solved by a DIFFERENT mechanism than specified — the fan-out was reached through the cross-schema fallback, not through untyped relation properties — so the originally-specified work is explicitly folded into task 6 rather than quietly counted as done.
# Conflicts: # openspec/changes/object-write-sub-500ms/specs/object-write-performance/spec.md # openspec/changes/object-write-sub-500ms/tasks.md
Adds WritePhaseProbe::stamp() — absolute offsets from REQUEST_TIME_FLOAT, distinct from mark()'s durations, because the question is 'how much of the request had already elapsed before our code ran at all'. Stamped at OpenRegister's register(), its boot() entry and exit, the create controller, and the end of the write. One create, wall 1,464ms: or.register.in 122ms NC core + apps registering before us or.boot.in 903ms +781ms of OTHER APPS registering or.boot.out 928ms our own boot: 25ms ctrl.create.in 964ms +36ms routing and middleware flush 1,357ms +393ms the actual write So 964ms of a 1,464ms request elapses before the controller is entered, and 781ms of that is apps registering. Bracketed directly on the same instance with the same auth: status.php (no app boot) 47ms capabilities (full app boot) 970ms ~920ms is booting 92 enabled apps, ~10ms each. 24 of them are Conduction fleet apps; a deployment running OpenRegister plus a handful of leaves boots a fraction of that. This is why the 500ms budget is specified against the write path with the instance floor subtracted: no change to the write path can move the 920ms, and a wall-clock measurement of it is a measurement of the app count. Off unless /tmp/or-trace-write-phases exists, checked once per process.
# Conflicts: # lib/Service/WritePhaseProbe.php
Adds or.register.out, closing the one gap in the request timeline that still allowed 'maybe OpenRegister's own registration is the pig'. It is not: or.register.in 235ms or.register.out 235ms <- our register() is 0ms or.boot.in 1,355ms <- +1,120ms of other apps + NC phases or.boot.out 1,376ms <- our boot() is 21ms ctrl.create.in 1,451ms OpenRegister has 268 registerService/registerEventListener calls across a 4,248-line Application.php and they cost nothing measurable, because they are lazy closures — which is the point of the API and worth having proof of. So the per-request app-boot cost (~16ms/app across 92 apps, measured by disabling 8 and restoring them) is Nextcloud's own registration and boot machinery plus the other 91 apps, and no change inside this app can reduce it.
BUDGET MET. Measured on larpingapp/character with tests/perf/object-create.sh: wall p95 13,700ms -> 469ms wall median 13,688ms -> 385ms write path 12,800ms -> 93ms median / 177ms p95 Requires an app token, defer_object_events=1, and PHP JIT disabled (Conduction/.github#75). With deferral off the p95 is 650ms. CORRECTS MY OWN EARLIER ANALYSIS. This change previously documented an 'instance floor' of 864-1,099ms attributed to Nextcloud booting 92 apps, and concluded wall-clock under 500ms was unreachable without disabling apps. That was an artefact of the benchmark, not a property of Nextcloud. Every sample authenticated with HTTP Basic auth carrying the ACCOUNT PASSWORD, which Nextcloud bcrypt-verifies on every request. Same endpoint, same instance, back to back: account password median 1,058ms app token median 456ms ~600ms per request was password hashing. No real client authenticates that way - browsers carry a session cookie, integrations use app tokens - so the benchmark was measuring bcrypt and charging it to the application. The true floor is ~240-290ms. The harness now warns when NC_AUTH looks like a password, and the spec makes token auth a precondition of the measurement with its own scenario. Worth stating plainly: I disabled apps to derive a per-app cost, checked opcache, checked APCu, and measured a slope - all real work, all answering the wrong question, because I never questioned the instrument. A floor assumed to be structural deserves the same attribution discipline as the code under test.
The code moved back to GitHub but this composer VCS repository did not, so every 'composer install' in CI still cloned from codeberg.org. When Codeberg returned 504s today, openregister's Newman suite failed in 'Install composer deps' on a dependency fetch — a full CI outage caused by a host we no longer publish to. ConductionNL/sapp on GitHub carries the same history: the pinned commit 5c406e91254d6936f44372db35f1cc15e5a06c56 and its branch feat/chained-filter-text-replace both resolve there. The lock now references the identical commit via GitHub, and no other package moved. Verified with a cold, unauthenticated 'composer install' against an empty cache: ddn/sapp downloads and extracts from GitHub with no Codeberg contact. Note for whoever picks this up: the .github submodule's origin is still https://codeberg.org/Conduction/.github.git even though ConductionNL/.github exists on GitHub. That is why the JIT change had to be raised as Conduction/.github#75 on Codeberg rather than a GitHub PR.
object-write-sub-500ms took a create from 13,688ms to 322ms median / 476ms p95.
The 500ms budget is met and is no longer the binding constraint: the instance
floor — an authenticated request doing no object work — is 172-213ms, so an
absolute wall budget mostly measures how many apps are installed.
This change targets the write path costing <=50ms above that floor, and the
budget in the spec becomes floor-relative for the same reason.
What the remaining cost is, from full PostgreSQL statement logging of one
create scoped to its backend and time window (326 statements, 176.8ms):
57 x 44.7ms SELECT * FROM oc_openregister_schemas
WHERE uuid=? OR LOWER(slug)=? OR id=?
24 x 3.2ms register lookups, same shape
18 x 0.8ms SELECT lastval()
9 x 4.7ms SELECT 1 FROM information_schema.tables
-- 2 audit rows + hash-chain UPDATE, 2 notification rows
~135 committed transactions where there should be 1
The schema query seq-scans by construction: SELECT * hydrates a ~2KB properties
blob and LOWER(slug) cannot use an index (Rows Removed by Filter: 1916 of 1929).
19 tasks in 5 phases, ordered by measured payoff: identity map for schemas and
registers, cheap miss path, stop probing information_schema, one transaction,
finish the deferral set, then delete the 2,728-branch fan-out via a
uuid->(register,schema) index rather than optimise it.
Phase 4 covers per-request work outside the write, in scope because the write is
measured against the floor (ADR-076):
pipelinq iterates the 3.4MB appstore catalogue on EVERY request
(resolveDependencyStatuses -> buildAppStoreLookup -> AppFetcher::get). It is
free here ONLY because has_internet_connection=false returns an empty set. It
also computes provideInitialState() on API requests that render no UI.
openconnector invokes a repair step from boot(). Correctly gated AND persists
its key, so free today — and one cleared config key from a repair step per
request. ADR-076 rule 4 puts that fallback in a TimedJob.
31 cron jobs, 8 at 60s. An idle instance does 18 schema seq scans and 356
commits per 4 seconds: the noise floor every measurement here fights.
Two measurement hazards written into the tasks because both cost me real time
this session: benchmarking with the account password adds ~600ms of bcrypt per
request (app token: 456ms median vs 1,058ms), and pg_stat_* counters are
database-global so cron pollutes them — the statement-log method is
authoritative.
Task 8 is flagged as a product decision, not a performance one: whether
deferral becomes default depends on what executionMode:sync promises.
…t median
I re-measured before starting on this plan, and the numbers it was written
against are stale. Acting on them would have meant a large refactor for a small
gain.
wall min 220ms median 249ms p95 394ms
instance floor 207ms
WRITE PATH min 13ms median 42ms p95 187ms
The <=50ms target is met at median (42ms). p95 is over, but the host was at load
4.6-6.0 from unrelated work — noise, not code.
What moved, from statement logs of one create before and after the DocuDesk
fixes landed:
before now
statements 326 251
schema lookups 57 (44.7ms) 15 (11.3ms)
register lookups 24 (3.2ms) 24 (3.5ms)
information_schema 9 (4.7ms) 9 (3.5ms)
SELECT lastval() 18 9
Phase 1 is therefore worth ~11ms, not ~45ms. Tasks 1-4 are DEFERRED, not
cancelled — they earn their keep again if the schema count grows or a caller
reintroduces a hot loop, and the reasoning stays in the file so nobody has to
rediscover it.
Also recording a hypothesis of mine that did NOT hold, because it looked
compelling: the re-measurement showed the non-lazy appconfig load at 80.6ms,
42% of all DB time, and app_versions stores 9.1MB across 86 non-lazy keys
(appstore.payload.*, up to 3.2MB for mail). Marking them lazy made creates
MARGINALLY SLOWER (275 -> 299ms median) because memcache.local is APCu and the
config is cached across requests — the 80ms was a cold-cache event, once per PHP
worker, not per request. Reverted. Still worth doing as hygiene; not a
performance task.
Revised priority: p95 stability first (establish whether it moves at all on an
unloaded host before calling it a code problem), then the phase-4 latent items
(pipelinq's appstore walk is free ONLY because has_internet_connection=false;
openconnector is one config key from a repair step per request; the 60s cron
fleet is the noise floor), then task 6 (one transaction) on correctness grounds
rather than latency.
…xamples/
PdfExtractorTest read vendor/ddn/sapp/examples/testdoc.pdf. That path only ever
resolved because composer happened to install ddn/sapp from SOURCE: the package
declares
/examples export-ignore
in its .gitattributes, so every distribution archive omits the directory. A git
clone keeps it; a zipball does not.
Switching the package to its GitHub dist (7ac9c92, to get Codeberg off the CI
critical path after an outage took the suite down) made that latent dependency
visible as exactly one failing assertion out of 15,504 tests:
Failed asserting that file ".../vendor/ddn/sapp/examples/testdoc.pdf" exists.
So the regression was mine, and the underlying fault is older: a test must not
depend on a dependency's examples directory, because the dependency has
explicitly declared it not part of what it ships. The fixture is now committed
at tests/fixtures/pdf/testdoc.pdf (51,269 bytes, byte-identical) and the test is
independent of how composer chooses to install anything.
Verified: 3 tests, 7 assertions, green.
…se 1 deferral
I had only ever measured creates, and generalised from that to defer the schema
identity map as "worth ~11ms". Measuring the rest of the object API shows that
was the create-only figure and the wrong call for everything else.
tests/perf/object-crud.sh covers create/read/search/update/delete, reports every
figure both as wall time and as wall minus the instance floor measured in the
same run, and prints host load so a bad sample is visible.
Wall (5 runs each, host load 61.7 from unrelated work — the ABSOLUTE numbers are
badly inflated, the RATIOS are the finding):
create 1,477ms 525ms above floor
read 1,447ms 495ms above floor
search 659ms 0ms above floor <- never leaves the floor
update 9,080ms 8,128ms above floor <- 15x a create
delete 4,243ms 3,291ms above floor <- 6x a create
Statement counts, which do NOT inflate with load (statement log scoped to the
request's backend and time window):
statements DB time
create 251 194ms
update 716 2,672ms
delete 595 1,781ms
Same shape in both slow paths — it is repeated resolution, not the write:
update delete create
register lookups 66 64 24
schema lookups (uuid OR slug OR id) 51 51 15
schema lookups (slug + id IN) 42 42 9
information_schema.tables probes 27 27 9
getLiveMagicTables() (all 2,728) 12 - ~3
So tasks 1-5 move back to the top. Task 3 (the REGISTER map) now matters more
than task 1, because registers are re-resolved more often than schemas.
Search being free is worth noting too: whatever the list path does, it is
already right.
The lesson is mine to own — I measured one operation and generalised. The
budget in the spec is per-operation for a reason.
getLiveMagicTables() lists EVERY magic table from information_schema and then fetches the full register and schema id lists to discard orphans. On this instance that is 2,728 tables, ~60ms a call. Measured 2026-07-30 (statement log, scoped to the request): an object UPDATE called it 12 TIMES. The answer cannot change mid-request unless this request creates a table, which is handled below. getLiveMagicTables enumerations per update: 12 -> 3 Not 1, because several MagicMapper instances participate in one update; the memo is per-instance. Getting to 1 needs the instances shared, which is a DI change and out of scope here. Also memoises checkTableExistsInDatabase(), but ONLY POSITIVE answers. A negative can legitimately become positive within the request — ensureTableExists() creates a table and then writes to it — and caching "no" would break that write against a table that now exists. Creating a table invalidates both memos via invalidateTableMemos(). HONEST LIMITATION: this did NOT reduce the 27 information_schema existence probes an update issues; that count is unchanged, so those come from a third path (most likely Doctrine's IDBConnection::tableExists() through another caller, not through this method). RegisterService::magicTableExists() already documents the same class of bug costing 76 SECONDS on a stats endpoint, so the pattern is known and solved in one place and not others. Tracked as task 5 of object-write-at-instance-floor; finding the third caller is the next step. Verified: 15,509 tests / 34,667 assertions green. phpcs clean.
# Conflicts: # openspec/changes/object-write-at-instance-floor/tasks.md
… — caveat them The per-operation statement counts (create 251, update 716, delete 595) came from taking every statement on the request's PostgreSQL backend within a time window. A backend is a pooled connection serving consecutive requests, so the window sweeps in unrelated traffic. Tight enough for a ~500ms create; useless for an update that took 30s under host load 21-62 — 24 distinct backends issued probes during that capture. This also retracts the inference I drew from it: '27 information_schema probes per update, each table probed 3x, therefore 3 MagicMapper instances'. The 3x is far more likely 3 REQUESTS reusing one pooled connection. What stands: the wall-clock ratios (update 15x a create, delete 6x, search free) time individual HTTP requests and are unaffected by pooling, and the direction of the finding — update/delete do far more repeated resolution than a create — is visible regardless of the multiplier. What does not: the counts themselves, and the breakdowns derived from them. Correct method for next time: bracket on a marker the request emits in its own SQL, add a per-request id to log_line_prefix, or count from inside PHP where 'this request' is unambiguous.
…d-log counts
Adds WritePhaseProbe::count() and instruments the three lookups the CRUD work
implicated. Counting inside the request makes "this request" unambiguous, which
the PostgreSQL statement log cannot: a backend is a pooled connection serving
consecutive requests, so bracketing by wall-clock sweeps in unrelated traffic.
The real numbers, and they are much lower than the log suggested:
schema reads tableExists full enumerations
create 6 3 0
update 13 7 1
delete 12 7 1
Against the log-derived figures I published (create 15 / update 51 schema
lookups, 27 probes), these are 2-4x smaller. Update and delete do roughly TWICE
a create's schema reads and each performs one full 2,728-table enumeration a
create does not — a real gap, and a far more modest one than I reported.
Note this also means the wall-clock ratios (update 15x a create, delete 6x) are
NOT explained by round-trip counts alone. Those ratios came from timing HTTP
requests and stand; the gap between them and these counts points at PHP-side
work, which is the next thing to measure rather than assume.
The magic-table memoisation holds enumerations at 1 per request — the floor
without sharing mapper instances, and better than the "12 -> 3" I claimed from
the log.
Limitation: read and search produce no counts, because flush() is only reached
from the write path. Instrumenting the read path is open.
…orphaned Inserting count() above stamp() left stamp()'s docblock attached to count() and stamp() with none — the third time this session that anchoring an insertion on a function SIGNATURE rather than on the docblock above it produced exactly this. Worth remembering: anchor on the docblock opener, not the signature.
Gives every app one honest answer to "which flows are running right now", scoped to the caller's organisation, so a dashboard widget can show it without each app building the same surface again. GET /api/flow-runs/active returns the NON-TERMINAL runs — queued, running and suspended, defined once as FlowRun::ACTIVE. Filtering to literally `running` would be empty almost always: a run holds that status only for the duration of a worker pass, while queued and suspended are where a live run actually waits. Three things had to change for that read to exist: - `organisation` is now STAMPED on queue(). The column has existed since the table was created and nothing ever wrote to it, so no tenant filter was possible at all. Resolution is lazy through the container: the cron worker builds this service on every pass and must not drag the RBAC graph in to fill a column it usually cannot fill. A run queued with no session is recorded unattributed rather than guessed at. - Scoping is STRICT. A run with no organisation goes to nobody. This feeds a widget every app renders to every user; attributing an unattributed run to the reader's tenant would put one tenant's activity on another's dashboard. - The rows are SUMMARISED — uuid, flow id AND resolved flow NAME, status, trigger, who started it, subject, current step, timestamps. Not the marking, not the items (which can hold the subject's own record data), not the step log: kilobytes per run a list never renders. The single-run endpoint stays the place to ask for a run's contents. `GET /api/flow-runs` is deliberately unchanged. It is the history surface with existing e2e coverage; a separate endpoint is what lets the tenant boundary be strict here without changing what existing callers see. Also indexes (organisation, status, id). Measured before the index on a dev instance with 48,058 runs: the planner walked the primary key backwards and filtered, reading 48,048 rows to return 1 (294ms in postgres, ~21s over HTTP) — on a surface a widget polls every 15 seconds. Tests: 32 green (5 new — no organisation reads nothing and never queries the store, scoping passes the caller's org through, rows carry the resolved name and step, an unresolvable flow falls back to its id, the row limit is capped). phpcs / phpmd / phpstan / psalm clean on the changed files; the two pre-existing StaticAccess findings and TooManyFields on the entity are now carried as reasoned suppressions rather than left failing.
Menu icons across the fleet had drifted into meaninglessness: a scan of 21 manifest-shipping apps found 120 distinct icons for 262 distinct labels, with one glyph standing for as many as 18 unrelated concepts (`icon-category-monitoring`) and the same concept drawn differently per app — Store was `icon-category-integration` in one app and `icon-category-organization` in another. Moves this app's menu onto the shared vocabulary: MDI PascalCase names, one concept to one icon. Tier A entries (Dashboard, Documentation, Settings, Store, Features & roadmap) now match every other Conduction app, which is the whole point — a glyph should mean the same thing wherever a user meets it. Two defect classes are fixed along the way: * Icon names that do not exist in vue-material-design-icons at all. They could never resolve — rendering a help-circle at best, nothing at all in the navigation. * Menu entries that rendered with NO icon, because CnAppNav resolves an MDI name only through the registry `registerIcons()` populates, with no fallback for a name the app never registered. Apps that relied on legacy `icon-*` classes registered nothing at all and were fine until the first MDI name appeared. src/icons.js is generated from the app's own manifests and register files, so every name the app references is registered and the migration stands on its own against the CURRENTLY RELEASED @conduction/nextcloud-vue — it does not wait on the library-side vocabulary (ConductionNL/nextcloud-vue#563). Verified: 0 menu entries render without an icon (was 51 fleet-wide), every icon import resolves against the app's own node_modules, and hydra's gate-60 icon-vocabulary check passes with no failures or warnings. Spec: ADR-077 (ConductionNL/hydra#408).
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 555/555 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-07-30 18:43 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Menu icons across the fleet had drifted into meaninglessness. A scan of all 21 manifest-shipping apps (366 menu entries) found 120 distinct icons for 262 distinct labels:
icon-category-monitoringicon-commenticon-category-organization…and the same concept drawn differently per app — Store was
icon-category-integrationin hermiq andicon-category-organizationin openbuild; Dashboard had three glyphs, Documentation three.What
Moves this app onto the shared vocabulary defined in ADR-077: MDI PascalCase names, one concept to one icon. Tier A entries (Dashboard, Documentation, Settings, Store, Features & roadmap) now match every other Conduction app — which is the point: a glyph should mean the same thing wherever a user meets it.
Two defect classes fixed along the way:
vue-material-design-iconsat all (LedgerOutline,FileSignOutline,GavelOutline,BankTransferOutline, …). They could never resolve — a help-circle at best, nothing in the navigation.CnAppNavresolves an MDI name only through the registryregisterIcons()populates, with no fallback and no CSS class for non-icon-*values. Apps that relied on legacyicon-*registered nothing at all and were fine right up until the first MDI name appeared. Live-verified on hrmq before this change: 29 of 72 nav entries rendered blank.src/icons.jsis generated from this app’s own manifests and register files, so every referenced name is registered and the migration stands on its own against the currently released@conduction/nextcloud-vue— it does not wait on the library-side vocabulary (ConductionNL/nextcloud-vue#563).Verification
node_modules(1,247 checked fleet-wide, 0 unresolvable).icon-vocabularypasses: 0 failures, 0 warnings.Spec: ADR-077 — ConductionNL/hydra#408.