What Happened?
Summary
These are three independent defects that happen to compound. Observed on a production
Docker deployment; the numbers are reproducible.
1. Series and file instruments are never imported
Observed
A repository containing lib/series/* syncs successfully and reports no error, but no
series instrument ever appears. Nothing is logged, because nothing is attempted.
Root cause
discoverInstrumentDirs (apps/api/src/instrument-repos/instrument-repos.service.ts:174)
iterates a hardcoded category list:
for (const category of ['forms', 'interactive']) {
const libDir = path.join(repoDir, 'lib', category);
lib/series/ and lib/file/ are skipped before discovery, even though both are real kinds —
$InstrumentKind is ['FILE', 'FORM', 'INTERACTIVE', 'SERIES']
(packages/schemas/src/instrument/instrument.base.ts:31) and the built-in library has a
directory for each (packages/instrument-library/src/{file,forms,interactive,series}/).
This is currently documented as intended behaviour in
.agents/docs/architecture/instrument-pipeline.md:49-51:
Repo discovery only scans lib/forms and lib/interactive. A lib/file or
lib/series directory in an external repo is silently ignored.
Why this is more than a one-line change
validateSeriesInstrument (apps/api/src/instruments/instruments.service.ts:691) resolves
every series item to an existing instrument id and rejects the entire series if any is
missing (Cannot find instrument '<name>' with edition '<edition>'). Consequences:
- Ordering becomes load-bearing. Series must be created after the scalars they
reference. Appending 'series' to the category array happens to get this right, since
forms and interactive are pushed first — but nothing enforces or documents that.
- Cross-repo series cannot work today. A series in repo A referencing a form in repo B
only imports if B was synced first. There is no dependency resolution and no retry, so
the result depends on the order repos were added. Making this reliable needs a second
resolution pass after all repos are imported.
- Group ownership is undecided. Repo imports pass no
seriesGroupId, so a series would
be created with seriesGroup: null and access would flow through repo assignment like
other repo-sourced instruments. Consistent, but should be a deliberate choice.
Suggested fix
Add 'series' (and 'file') to the scanned categories, make the ordering explicit rather
than incidental, log a clear reason when a series is skipped for a missing item, and update
the two doc locations pinning this behaviour (instrument-pipeline.md:47-51 and :219).
Decide explicitly whether cross-repo series references must work.
2. Import silently under-counts instruments
Observed
Found 9 instrument directories in DouglasNeuroInformatics/ODC_Restricted_Instruments
Imported 8 instruments from DouglasNeuroInformatics/ODC_Restricted_Instruments
Two distinct causes were found in the logs.
Cause A — an instrument that fails to evaluate is reported only as a generic message
Failed to import instrument from SHEEHAN_DISABILITY_SCALE:
UnprocessableEntityException: Failed to interpret instrument bundle
The instrument bundles, then fails when the API evaluates it
(instruments.service.ts:93-99). The underlying reason is logged separately via
this.loggingService.error(result.error) and is not correlated with the instrument, so an
operator sees only "Failed to interpret" with no indication of which file or why. If the
cause is module-scope DOM access, the bundler's GLOBALS proxies already produce a message
naming the offending file — that detail should reach the import log.
Cause B — check-then-act race drops instruments already present
Failed to import instrument from GAD_7: PrismaClientKnownRequestError:
Invalid `prisma.instrument.create()` invocation:
Unique constraint failed on the constraint: `_id_`
Also seen for OLDER_AMERICANS_RESOURCES_AND_SERVICES, TEN_ITEM_PERSONALITY_INVENTORY,
TMB_DIGIT_SYMBOL_MATCHING, TMB_FLANKER_ATTENTION, TMB_VERBAL_PA_STUDY — all names
provided by more than one repository.
create() guards with instrumentModel.exists({ id }) at instruments.service.ts:111,
then inserts at :137. Between those two points another import can insert the same id, so
the insert collides. The ConflictException handler in importInstrumentFromDir
(instrument-repos.service.ts:310-313) recovers the existing id from the message and still
associates the instrument with the repo — but a PrismaClientKnownRequestError is not a
ConflictException, so it escapes to the outer catch and the instrument is dropped from
that repo's instrumentIds entirely.
Note: the concurrency trigger is inferred from the log pattern (duplicate names across
repos) and has not been confirmed by reproduction.
There is also a genuinely silent path
importInstrumentFromDir returns null and importInstruments does
if (!result) continue; (:338) with no log above debug. One of those branches —
ConflictException where the id regex fails to match (:313) — would drop an instrument
with no trace at default log level.
Suggested fix
Treat a Prisma unique-constraint violation the same as ConflictException (recover the id,
count the instrument). Attach the underlying cause to the per-instrument failure log.
Replace the silent null/continue path with an explicit warning, or make it unreachable.
3. "View instruments" dialog hides all but the latest edition
Observed
A repo whose column reads 8 instruments opens a dialog listing 4.
Root cause
The column reads the stored count — row.instrumentIds.length
(apps/web/src/routes/_app/admin/instrument-repos/index.tsx:184) — while the dialog is
populated from useInstrumentInfoQuery() called with no arguments (:114) and filtered by
sourceRepo.id (:276).
With no arguments, allEditions defaults to false
(apps/api/src/instruments/instruments.service.ts:362), and that path keys results by
instrument name, keeping only the highest edition
(instruments.service.ts:427-432):
2. Import silently under-counts instruments
Observed
Found 9 instrument directories in DouglasNeuroInformatics/ODC_Restricted_Instruments
Imported 8 instruments from DouglasNeuroInformatics/ODC_Restricted_Instruments
Two distinct causes were found in the logs.
Cause A — an instrument that fails to evaluate is reported only as a generic message
Failed to import instrument from SHEEHAN_DISABILITY_SCALE:
UnprocessableEntityException: Failed to interpret instrument bundle
The instrument bundles, then fails when the API evaluates it
(instruments.service.ts:93-99). The underlying reason is logged separately via
this.loggingService.error(result.error) and is not correlated with the instrument, so an
operator sees only "Failed to interpret" with no indication of which file or why. If the
cause is module-scope DOM access, the bundler's GLOBALS proxies already produce a message
naming the offending file — that detail should reach the import log.
Cause B — check-then-act race drops instruments already present
Failed to import instrument from GAD_7: PrismaClientKnownRequestError:
Invalid `prisma.instrument.create()` invocation:
Unique constraint failed on the constraint: `_id_`
Also seen for OLDER_AMERICANS_RESOURCES_AND_SERVICES, TEN_ITEM_PERSONALITY_INVENTORY,
TMB_DIGIT_SYMBOL_MATCHING, TMB_FLANKER_ATTENTION, TMB_VERBAL_PA_STUDY — all names
provided by more than one repository.
create() guards with instrumentModel.exists({ id }) at instruments.service.ts:111,
then inserts at :137. Between those two points another import can insert the same id, so
the insert collides. The ConflictException handler in importInstrumentFromDir
(instrument-repos.service.ts:310-313) recovers the existing id from the message and still
associates the instrument with the repo — but a PrismaClientKnownRequestError is not a
ConflictException, so it escapes to the outer catch and the instrument is dropped from
that repo's instrumentIds entirely.
Note: the concurrency trigger is inferred from the log pattern (duplicate names across
repos) and has not been confirmed by reproduction.
There is also a genuinely silent path
importInstrumentFromDir returns null and importInstruments does
if (!result) continue; (:338) with no log above debug. One of those branches —
ConflictException where the id regex fails to match (:313) — would drop an instrument
with no trace at default log level.
Suggested fix
Treat a Prisma unique-constraint violation the same as ConflictException (recover the id,
count the instrument). Attach the underlying cause to the per-instrument failure log.
Replace the silent null/continue path with an explicit warning, or make it unreachable.
3. "View instruments" dialog hides all but the latest edition
Observed
A repo whose column reads 8 instruments opens a dialog listing 4.
Root cause
The column reads the stored count — row.instrumentIds.length
(apps/web/src/routes/_app/admin/instrument-repos/index.tsx:184) — while the dialog is
populated from useInstrumentInfoQuery() called with no arguments (:114) and filtered by
sourceRepo.id (:276).
With no arguments, allEditions defaults to false
(apps/api/src/instruments/instruments.service.ts:362), and that path keys results by
instrument name, keeping only the highest edition
(instruments.service.ts:427-432):
const currentEntry = results.get(info.internal.name);
if (!currentEntry || !('internal' in currentEntry) || info.internal.edition > currentEntry.internal.edition) {
results.set(info.internal.name, info);
}
So 8 instruments at two editions each collapse to 4 rows. Nothing is missing from the
database — the dialog simply isn't asking for it. This matches the observed data
(WHOQOL-BREF listed at edition 2, with its edition 1 absorbed).
Why it's wrong here
The dialog is titled "Instruments in <repo>" and exists to audit what a repository
contributed. Silently hiding earlier editions makes it disagree with the count shown one
click earlier, with no indication that filtering occurred.
Suggested fix
Pass { allEditions: true } at index.tsx:114. This affects only the dialog; the column
count comes from instrumentIds and is unchanged.
Related
PR #1482 fixes a separate defect that made all of the above harder to diagnose: repo import
failed entirely in bundled/production builds, and two layers of error handling discarded the
underlying cause. The improved error logging in that PR is a prerequisite for diagnosing
items 1 and 2 in the field.
Two notes on using this: the concurrency explanation in 2B is inferred from the log pattern, not reproduced, so I flagged it as such in the text rather than
asserting it. And item 1 contains a genuine design question (cross-repo series references) that probably needs a decision before anyone starts implementing
— it may deserve to be its own issue so that discussion doesn't get buried under the two smaller bugs.
What Did You Expect?
For the instrument repo sync to work properly
Operating System
ubuntu 24.04
Browser (if applicable)
No response
Steps to Reproduce
No response
Anything Else?
No response
What Happened?
Summary
These are three independent defects that happen to compound. Observed on a production
Docker deployment; the numbers are reproducible.
1. Series and file instruments are never imported
Observed
A repository containing
lib/series/*syncs successfully and reports no error, but noseries instrument ever appears. Nothing is logged, because nothing is attempted.
Root cause
discoverInstrumentDirs(apps/api/src/instrument-repos/instrument-repos.service.ts:174)iterates a hardcoded category list:
lib/series/andlib/file/are skipped before discovery, even though both are real kinds —$InstrumentKindis['FILE', 'FORM', 'INTERACTIVE', 'SERIES'](
packages/schemas/src/instrument/instrument.base.ts:31) and the built-in library has adirectory for each (
packages/instrument-library/src/{file,forms,interactive,series}/).This is currently documented as intended behaviour in
.agents/docs/architecture/instrument-pipeline.md:49-51:Why this is more than a one-line change
validateSeriesInstrument(apps/api/src/instruments/instruments.service.ts:691) resolvesevery series item to an existing instrument id and rejects the entire series if any is
missing (
Cannot find instrument '<name>' with edition '<edition>'). Consequences:reference. Appending
'series'to the category array happens to get this right, sinceformsandinteractiveare pushed first — but nothing enforces or documents that.only imports if B was synced first. There is no dependency resolution and no retry, so
the result depends on the order repos were added. Making this reliable needs a second
resolution pass after all repos are imported.
seriesGroupId, so a series wouldbe created with
seriesGroup: nulland access would flow through repo assignment likeother repo-sourced instruments. Consistent, but should be a deliberate choice.
Suggested fix
Add
'series'(and'file') to the scanned categories, make the ordering explicit ratherthan incidental, log a clear reason when a series is skipped for a missing item, and update
the two doc locations pinning this behaviour (
instrument-pipeline.md:47-51and:219).Decide explicitly whether cross-repo series references must work.
2. Import silently under-counts instruments
Observed
Two distinct causes were found in the logs.
Cause A — an instrument that fails to evaluate is reported only as a generic message
The instrument bundles, then fails when the API evaluates it
(
instruments.service.ts:93-99). The underlying reason is logged separately viathis.loggingService.error(result.error)and is not correlated with the instrument, so anoperator sees only "Failed to interpret" with no indication of which file or why. If the
cause is module-scope DOM access, the bundler's
GLOBALSproxies already produce a messagenaming the offending file — that detail should reach the import log.
Cause B — check-then-act race drops instruments already present
Also seen for
OLDER_AMERICANS_RESOURCES_AND_SERVICES,TEN_ITEM_PERSONALITY_INVENTORY,TMB_DIGIT_SYMBOL_MATCHING,TMB_FLANKER_ATTENTION,TMB_VERBAL_PA_STUDY— all namesprovided by more than one repository.
create()guards withinstrumentModel.exists({ id })atinstruments.service.ts:111,then inserts at
:137. Between those two points another import can insert the same id, sothe insert collides. The
ConflictExceptionhandler inimportInstrumentFromDir(
instrument-repos.service.ts:310-313) recovers the existing id from the message and stillassociates the instrument with the repo — but a
PrismaClientKnownRequestErroris not aConflictException, so it escapes to the outer catch and the instrument is dropped fromthat repo's
instrumentIdsentirely.Note: the concurrency trigger is inferred from the log pattern (duplicate names across
repos) and has not been confirmed by reproduction.
There is also a genuinely silent path
importInstrumentFromDirreturnsnullandimportInstrumentsdoesif (!result) continue;(:338) with no log abovedebug. One of those branches —ConflictExceptionwhere the id regex fails to match (:313) — would drop an instrumentwith no trace at default log level.
Suggested fix
Treat a Prisma unique-constraint violation the same as
ConflictException(recover the id,count the instrument). Attach the underlying cause to the per-instrument failure log.
Replace the silent
null/continuepath with an explicit warning, or make it unreachable.3. "View instruments" dialog hides all but the latest edition
Observed
A repo whose column reads 8 instruments opens a dialog listing 4.
Root cause
The column reads the stored count —
row.instrumentIds.length(
apps/web/src/routes/_app/admin/instrument-repos/index.tsx:184) — while the dialog ispopulated from
useInstrumentInfoQuery()called with no arguments (:114) and filtered bysourceRepo.id(:276).With no arguments,
allEditionsdefaults tofalse(
apps/api/src/instruments/instruments.service.ts:362), and that path keys results byinstrument name, keeping only the highest edition
(
instruments.service.ts:427-432):2. Import silently under-counts instruments
Observed
Two distinct causes were found in the logs.
Cause A — an instrument that fails to evaluate is reported only as a generic message
The instrument bundles, then fails when the API evaluates it
(
instruments.service.ts:93-99). The underlying reason is logged separately viathis.loggingService.error(result.error)and is not correlated with the instrument, so anoperator sees only "Failed to interpret" with no indication of which file or why. If the
cause is module-scope DOM access, the bundler's
GLOBALSproxies already produce a messagenaming the offending file — that detail should reach the import log.
Cause B — check-then-act race drops instruments already present
Also seen for
OLDER_AMERICANS_RESOURCES_AND_SERVICES,TEN_ITEM_PERSONALITY_INVENTORY,TMB_DIGIT_SYMBOL_MATCHING,TMB_FLANKER_ATTENTION,TMB_VERBAL_PA_STUDY— all namesprovided by more than one repository.
create()guards withinstrumentModel.exists({ id })atinstruments.service.ts:111,then inserts at
:137. Between those two points another import can insert the same id, sothe insert collides. The
ConflictExceptionhandler inimportInstrumentFromDir(
instrument-repos.service.ts:310-313) recovers the existing id from the message and stillassociates the instrument with the repo — but a
PrismaClientKnownRequestErroris not aConflictException, so it escapes to the outer catch and the instrument is dropped fromthat repo's
instrumentIdsentirely.Note: the concurrency trigger is inferred from the log pattern (duplicate names across
repos) and has not been confirmed by reproduction.
There is also a genuinely silent path
importInstrumentFromDirreturnsnullandimportInstrumentsdoesif (!result) continue;(:338) with no log abovedebug. One of those branches —ConflictExceptionwhere the id regex fails to match (:313) — would drop an instrumentwith no trace at default log level.
Suggested fix
Treat a Prisma unique-constraint violation the same as
ConflictException(recover the id,count the instrument). Attach the underlying cause to the per-instrument failure log.
Replace the silent
null/continuepath with an explicit warning, or make it unreachable.3. "View instruments" dialog hides all but the latest edition
Observed
A repo whose column reads 8 instruments opens a dialog listing 4.
Root cause
The column reads the stored count —
row.instrumentIds.length(
apps/web/src/routes/_app/admin/instrument-repos/index.tsx:184) — while the dialog ispopulated from
useInstrumentInfoQuery()called with no arguments (:114) and filtered bysourceRepo.id(:276).With no arguments,
allEditionsdefaults tofalse(
apps/api/src/instruments/instruments.service.ts:362), and that path keys results byinstrument name, keeping only the highest edition
(
instruments.service.ts:427-432):So 8 instruments at two editions each collapse to 4 rows. Nothing is missing from the
database — the dialog simply isn't asking for it. This matches the observed data
(WHOQOL-BREF listed at edition 2, with its edition 1 absorbed).
Why it's wrong here
The dialog is titled "Instruments in
<repo>" and exists to audit what a repositorycontributed. Silently hiding earlier editions makes it disagree with the count shown one
click earlier, with no indication that filtering occurred.
Suggested fix
Pass
{ allEditions: true }atindex.tsx:114. This affects only the dialog; the columncount comes from
instrumentIdsand is unchanged.Related
PR #1482 fixes a separate defect that made all of the above harder to diagnose: repo import
failed entirely in bundled/production builds, and two layers of error handling discarded the
underlying cause. The improved error logging in that PR is a prerequisite for diagnosing
items 1 and 2 in the field.
Two notes on using this: the concurrency explanation in 2B is inferred from the log pattern, not reproduced, so I flagged it as such in the text rather than
asserting it. And item 1 contains a genuine design question (cross-repo series references) that probably needs a decision before anyone starts implementing
— it may deserve to be its own issue so that discussion doesn't get buried under the two smaller bugs.
What Did You Expect?
For the instrument repo sync to work properly
Operating System
ubuntu 24.04
Browser (if applicable)
No response
Steps to Reproduce
No response
Anything Else?
No response