Conversation
This patch fixes 5 ESLint warnings.
The max-lines disable in actionStatistics.ts no longer suppresses any
warning and only adds noise.
In apiKey.ts, "||" is replaced with "??" for description and lastUsedAt
fallbacks. "||" coerces any falsy value (including empty string: "") to
the default, whereas "??" only fires on null/undefined - which may be
the actual intent here. An empty-string description would be silently
discarded with "||".
In integrationRegistry/index.ts and RetryFailedNcmecDecisionsJob.ts,
null-guarded assignment blocks (if (x == null) { x = ... }) are
collapsed to "??=" assignments, which is the idiomatic shorthand for
exactly that pattern and makes the intent immediately readable.
…ationResolvers The Query and Mutation objects in apiKey.ts are typed as any, which removes all type-checking on resolver arguments and the context object. The codegen pipeline already produces GQLQueryResolvers and GQLMutationResolvers with the correct signatures. Using those types means wrong argument names, missing fields, or context misuse will be caught at compile time rather than at runtime. Error names must match a GraphQL union member because gqlErrorResult uses them as type name. 'InternalServerError' is a valid CoopErrorName but not a member of the RotateApiKeyResponse or RotateWebhookSigningKeyResponse unions, so Apollo can't resolve the type.
…ute any route.get returns Route<never, ...> which is invariant with Route<any, any> in ControllerRouteList, so individual routes needed as Route<any, ...> casts. Move the cast to the routes array level using as ControllerRouteList, which is already covered by the centralized eslint-disable on the type definition.
The SAML strategy callbacks passed Sequelize model instances to
done(), which expects "Record<string, unknown>". The original code used
"as any" to fix the mismatch; simply removing those casts broke the
build because Sequelize models carry no string index signature.
Use "user.toJSON()" instead - it returns a plain object that
genuinely satisfies "Record<string, unknown>" at runtime.
For serializeUser, augment "Express.User" in decs.d.ts with the
single field it actually accesses ("id: string").
PostgresAnalyticsAdapter and ClickhouseKyselyAdapter declare their Kysely fields as Kysely<any>, which disables query-builder type- checking for every table access and column reference in those adapters. PostgresAnalyticsAdapter is parameterised with Kysely<AnalyticsSchema> and ClickhouseKyselyAdapter with Kysely<Record<string, unknown>>, so Kysely can enforce table and column existence at compile time rather than at runtime. Stub method signatures using ...args: any[] are replaced with unknown[] so that the interface contract is actually enforced on the concrete classes.
…icsSchema
RuleExecutionLogger, ReportingRuleExecutionLogger, RoutingRuleExecutionLogger,
and ContentApiLogger cast table-name strings and row arrays to any when calling
bulkWrite because those table names were not yet keys of AnalyticsSchema. The
casts silenced the errors but also prevented the compiler from catching a
misspelled or removed table name.
Adding the missing tables (REPORTING_SERVICE.REPORTING_RULE_EXECUTIONS,
MANUAL_REVIEW_TOOL.ROUTING_RULE_EXECUTIONS) to AnalyticsSchema and widening
CONTENT_API_REQUESTS to {[key:string]:unknown} makes the constraint real and
allows all call-site casts to be deleted or narrowed to their correct types.
PostgresAnalyticsAdapter.flushTable used a cast that typed rows as a union
of all schema table types. Kysely requires an intersection of required fields
across those types, which the open-ended {[key:string]:unknown} entries
(introduced in this commit) cannot satisfy. Instead, bulkWrite now stores a
pre-built, fully-typed insert closure alongside each row buffer while it
still knows the concrete table type (flushTable just calls that closure).
@opentelemetry/semantic-conventions deprecated the SEMATTRS_EXCEPTION_* identifiers in favor of ATTR_EXCEPTION_*. The two names refer to the same attribute keys, so this is a pure rename. Clears three @typescript-eslint/no-deprecated warnings.
Small, mechanical cleanups flagged by @typescript-eslint/no-unnecessary-condition and @typescript-eslint/no-unnecessary-type-assertion. Each change either removes a guard the type system has already proven redundant, or removes an assertion that doesn't change the inferred type. * routes/integration_logos/serveIntegrationLogo*.ts: typed the express sendFile callback as "(err?: Error)" and simplified "err != null" to a truthiness check. Matches express's actual contract. * routes/reporting/submitReport.ts: dropped an always-truthy "hashes &&" guard. * rule_engine/RuleEngine.ts: dropped "?? []" on a non-nullable return. * utils/sql.ts: removed two "SelectQueryBuilder<...>" casts the inference now handles cleanly.
up a FileAnnotations object one case at a time. That tripped the complexity rule (max 20) and made the mapping itself hard to audit. Replaced with a single "Record<NCMECFileAnnotationType, keyof FileAnnotations>" lookup table plus a small for-loop. The table is the single source of truth for the NCMEC-enum -> field-name correspondence, and TypeScript's exhaustiveness checking on the Record key forces every enum variant to be mapped.
Each case below previously used "any" (or "as any") to paper over a typing problem. Where a cast is still needed, it goes through the narrowest meaningful type. - condition_evaluator/conditionSet.ts: typed the working array as "Array<ReadonlyDeep<LeafConditionWithResult | ConditionSetWithResult>>" and replaced two "as any" casts with a single documented narrowing cast at the return site. The public type is a discriminated union over arrays-of-leaves vs arrays-of-sets, which can't be pushed into during the loop; runtime contents stay uniform with the input conditions. - condition_evaluator/leafCondition.ts: typed the signal-result generic as "SignalResult<SignalOutputType>", removing both an "as any" and an unnecessary "as SignalOutputType" assertion at the call time. - graphql/datasources/UserApi.ts: typed the login/signUp/logout passport glue using the generated "GQLMutation-Args" types and a small structural context type (instead of "any"). - services/apiKeyService/apiKeyService.ts: derived "ApiKeyRow" from "Selectable<ApiKeyServicePg['public.api_keys']>" (Kysely) and used it as the param type of "mapDbRecordToApiKeyRecord". - services/manualReviewToolService/modules/CommentOperations.ts: replaced "as any" / "as any[]" with "as JobId" / "as JobId[]" using the existing branded type. - services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.ts: typed warehouse rows as "Record<string, unknown>" with explicit field-level casts, and the binding accumulator as "string[]". - services/ruleAnomalyDetectionService/detectRulePassRateAnomaliesJob.ts: preserved the runtime guard against missing org rows by indexing through "Partial<typeof orgsForChangedRules>". keyBy's "Record<string, T>" return type lies about lookups always succeeding. - bin/run-worker-or-job.ts: replaced "(container as any)[name]" with a typed "container[name as keyof Dependencies] as WorkerOrJob" lookup.
Three locations have "any" casts that can't be replaced today without upstream typing work that's already tracked by TODOs. - graphql/modules/insights.ts: resolver maps five times "getRulePassingContentSamples" results onto types that require "tags" and "policies" fields the datasource doesn't yet populate. The fix belongs in the datasource's typing, not in each resolver. Kept the localized "as any" cast and the existing TODO comments - graphql/resolvers.ts: same shape of pre-existing TODO on "getAllRuleInsights" return type - services/analyticsQueries/ItemHistoryQueries.ts: the data warehouse schema isn't modeled in TS, so the Kysely instance is intentionally "Kysely<any>" (so the string-based column selections pass type-check).
Tests that mock private methods or attach properties to typed services
need escape hatches that production code shouldn't. This patch replaces
"any" with real types where possible.
- condition_evaluator/conditionSet.test.ts: wrap RuleEvaluationContext /
SafeTracer mock stubs with "eslint-disable" / "eslint-enable" and a
comment.
- services/derivedFieldsService/helpers.test.ts: the test intentionally
passes invalid inputs to exercise error paths. Wrapped that block in a
paired "eslint-disable" / "eslint-enable" for "no-explicit-any" with a
comment.
- services/manualReviewToolService/modules/CommentOperations.test.ts:
replaced "as any" on job ids with "as JobId"; switched the
"(commentOps as any).getRelatedJobIds(...)" private-method probe to
bracket access ("commentOps['getRelatedJobIds'](...)"). Same access
pattern at runtime without a need for casting.
- services/orgAwareSignalExecutionService/signalExecutionService.test.ts:
extended two existing narrow "eslint-disable" directives to also cover
"no-explicit-any" for jest mock assignments. Added a paired
"eslint-disable" / "eslint-enable" block where the original
next-line directive couldn't reach the multi-line cast.
- services/ruleAnomalyDetectionService/getRuleAnomalyDetectionStatistics.test.ts:
typed the mock query's "tracer" parameter as "unknown". Kept the
unavoidable "jest.fn() as any" for the typed mock fixture and "{} as any"
for the unused tracer arg, both with narrow per-line disables that name
the rule and the reason.
- test/extendExpect.ts: replaced "(toMatchSnapshot as any).call" with
"toMatchSnapshot.call(this as SnapshotContext, ...)" using
jest-snapshot's exported "Context" type. Added an explicit return type
to satisfy "promise-function-async" (the inferred sync|async union was
tripping the rule).
Three switches dispatch on a discriminated union that today has only one variant: AggregationsService.evaluateAggregation, serializeAggregation, and the derivedFieldsService recipe-operation reducer. With one variant each case label is trivially equal to the discriminant, which trips @typescript-eslint/no-unnecessary-condition. The switches stay because they're load-bearing: when a new variant is added to the union, the existing case stops being exhaustive and the "default: assertUnreachable(...)" branch surfaces the gap at compile time.
pawiecz
force-pushed
the
fix-eslint-server
branch
from
6104272 to
7c836be
Compare
pawiecz
force-pushed
the
fix-eslint-server
branch
from
73aa227 to
6f6295d
Compare
The two SAML strategy callbacks still called `user.toJSON()` left over from the Sequelize User instance. `kyselyUserFindByEmail` now returns a plain `GraphQLUserParent` object.
The "fresh deploy" guard checked "lastTimestamp.last_insert == null",
which triggered @typescript-eslint/no-unnecessary-condition: the Kysely
schema types "last_insert" as a non-null "Date", matching the SQL
column ("NOT NULL DEFAULT '-infinity'").
This patch replaces "== null" with
"Number.isFinite(last_insert.valueOf())". This removes the dead branch
(and the lint warning) and makes the fresh-deploy path actually fall
back to "new Date(0)" as intended.
pawiecz
requested review from
cassidyjames,
dom-notion,
juanmrad,
julietshen and
vinaysrao1
as code owners
juanmrad
requested changes
May 1, 2026
Comment on lines
92
to
106
| CONTENT_API_REQUESTS: { | ||
| ds: string; | ||
| ts: number; | ||
| org_id: string; | ||
| item_id: string; | ||
| item_type_id: string; | ||
| item_submission_id?: string; | ||
| item_creator_id?: string; | ||
| endpoint: string; | ||
| method: string; | ||
| correlation_id: string; | ||
| duration_ms: number; | ||
| failed: boolean; | ||
| error_message?: string; | ||
| [key: string]: unknown; // Logged by ContentApiLogger | ||
| }; |
Member
There was a problem hiding this comment.
why are we removing the shape? We are losing type controls checks on compile time.
Comment on lines
4
to
+5
| import getBottle from '../iocContainer/index.js'; | ||
| import { type Dependencies } from '../iocContainer/index.js'; |
Member
There was a problem hiding this comment.
Suggested change
| import getBottle from '../iocContainer/index.js'; | |
| import { type Dependencies } from '../iocContainer/index.js'; | |
| import getBottle, { type Dependencies } from '../iocContainer/index.js'; |
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
2 participants
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.
Context & Requests for Reviewers
This PR consists of several separate commits so that they would be easier to read by Reviewers. These changes were submitted in a single request to keep related patches together instead of making noise in the PR queue.
Fixes: #198
Tests
Running
npm run lintinserverdirectory should produce no warnings (with an exception for the ones introduced in #349 which was developed in parallel to this patch set).