Implement custom reliability flags

[amyyeung17]

This PR introduces the new argument customValidations to createEvaluateValidity. customValidations allows apps to pass in new flags using existing or new conditions defined in the conditions array. Each customValidations item must contain the new flag name, conditions array, and logicalOperation to use with conditions.

• conditions callback function parameters: { responseTimes, responses, correct, completed, existingFlags }. responseTimes, responses, correct, and completed are existing values provided by the parent function baseEvaluateValidity (which is the function returned when calling createEvaluateValidity).
• Existing flags can be verified using the callback argument data.existingFlags
• Existing flags can be used to mark as reliability measure by itself even if it's used as a custom validation rule by passing the flag name to includedReliabilityFlags

Unrelated edit: Set fixed date for getAgeData test. The test was being flaky because the function relied on new Date().

[richford]

Thanks @amyyeung17. A few change requests below. Also, please run npm run format (i.e. prettier/eslint).

[richford]

jest.useFakeTimers() sits at the top level, so it activates at module load and applies to every test in the file until jest.useRealTimers() is called at the end of the getAgeData test body. This causes two issues: (1) unrelated tests registered earlier in the file run under fake timers unintentionally; (2) if that test throws before the restore line, fake timers leak into subsequent tests. Can you move setup/teardown into beforeAll/afterAll (or beforeEach/afterEach) scoped to that test/describe, restoring real timers in afterAll so a failure can't leak. The suite passes today, but it's a fragility trap.

[amyyeung17]

Hm, what are your thoughts about moving the data and fakeTimers into the test case? Or do we prefer defining a top-level variable so we can reuse it for other future tests?

[richford]

This is pre-existing but it looks like no "informational-only" flag is possible. The closing flags = flags.filter(x => includedReliabilityFlags.includes(x)) removes every flag not in includedReliabilityFlags from the returned output and from the reliability decision. So a custom flag you compute but don't include is invisible and you can't surface a flag purely for reporting without also making it count toward unreliability. This is pre-existing behavior but can you add a comment or something to the JsDoc documenting the known limitation. And maybe also a test for this.

[richford]

Unsupported logicalOperation values silently no-op here. Only 'and'/'or' are handled. Anything else ('only', a typo, 'xor') leaves conditionMet = false and the flag silently never fires, with no warning. Please add an else { throw } with console.warn.

[amyyeung17]

Gotcha. Now that you've brought up 'only' and 'xor', should we just implement them now?

[amyyeung17]

Implemented 'xor'. 'Only' can be handled by just passing a single condition.

[richford]

Also, a malformed rule throws and aborts the whole evaluation here. logicalOperation.toLowerCase() has no guard, so a rule missing logicalOperation (or with a non-string value) throws a TypeError, which propagates out of baseEvaluateValidity and takes the engagement-flag handling down with it. Real-world likelihood is low (rules are app-defined constants), but can you add a default value (logicalOperation = 'and') and also an explicit guard against non-string values?

[amyyeung17]

Added error handling during setup. I thought it might make sense to allow one-off custom rules that don't require logicalOperations. As a result, the validation only checks for valid logicalOperations when there are two or more conditions.

[richford]

existingFlags is a live reference shared across rules leading to order dependence. data is constructed once and existingFlags: flags aliases the same array that the forEach mutates later. So a later rule's condition can observe flags pushed by earlier custom rules, not just the default flags, which makes rule ordering important. Can you either snapshot it (existingFlags: [...flags]) so every rule sees the same baseline, or document that custom rules chain to each other. This also isn't exercised by tests (the multi-rule test only references base flags).

[amyyeung17]

Documented. I explored allowing customValidations rules to depend on one another, but the implementation became too unwieldy and introduced too many edge cases (e.g., circular dependencies).