Skip to content

Screener: fundamental fields silently null when filters arrive as serialized JSON #7

Description

@Kitkitkittt

Revised. The original body named the wrong fix: it proposed moving Fundamental Enrichment ahead of Advanced Filtering, which is necessary but not sufficient. Truncation to a Page happens in two places, both of which also precede enrichment, and the set of fundamental parameters is duplicated across two hand-maintained lists rather than one. The Problem Statement and most User Stories are unchanged. No work had begun against the earlier body.

Problem Statement

Users of the screener who select a Saved Screen that touches fundamental data get wrong results, with no error and no warning.

Two Saved Screens are affected today:

  • FCF Margin Expansion filters on FCF positive. It returns an empty table — zero rows, every time.
  • Low Debt Compounder displays a Moat column. That column renders blank for every row.

The api returns HTTP 200 with a well-formed payload in both cases, so nothing surfaces as a failure. A user cannot distinguish "no symbols match your criteria" from "the fundamental data was never loaded". The Saved Screen looks like it works and quietly gives a wrong answer, which is worse than an error — someone can act on an empty FCF screen believing no Vietnamese equities have positive free cash flow.

There is a second, quieter defect behind the first. Even once fundamental data loads, an Advanced Filter on a Fundamental Field screens only a Page, not the Universe. "Positive free cash flow" would mean "of the first hundred symbols in listing order, those with positive free cash flow" — a plausible-looking answer computed over six percent of the market.

Solution

Three properties, in the order they matter:

Fundamental Enrichment happens whenever a request needs it, regardless of how the request expresses that need. Today it happens only when fundamental criteria arrive as typed query parameters. Web never sends those — it sends one serialized blob — so for the actual frontend the enrichment never runs at all.

Fundamental Enrichment happens before the Candidate Set is cut to a Page, so that filters and sorts on Fundamental Fields see real values and screen the whole Universe. This matches how Discovery Fields already behave.

Every response states its Screen Scope, so that a Page-scoped answer is never mistaken for a full-Universe one.

From the user's perspective:

  • Selecting FCF Margin Expansion returns symbols with positive free cash flow, drawn from the whole Universe, with FCF positive and Margin of Safety populated.
  • Selecting Low Debt Compounder returns the same rows it always did, now with the Moat column filled in.
  • Screens touching no Fundamental Field stay exactly as fast as they are today. The request path that skips the Mongo read is preserved — that is the entire point of the change currently in the working tree.
  • When more symbols match than fit in one Page, the response says how many matched.
  • When fundamental data is unavailable, or when the screen could only examine a Page, the response says so.

User Stories

  1. As a screener user, I want the FCF Margin Expansion Saved Screen to return matching symbols, so that I can find companies with positive free cash flow.
  2. As a screener user, I want the Low Debt Compounder Saved Screen to show a populated Moat column, so that I can judge competitive durability at a glance.
  3. As a screener user, I want an Advanced Filter on a Fundamental Field to match rows, so that my screen is not silently empty.
  4. As a screener user, I want a Fundamental Field shown as a column to be populated even when I am not filtering on it, so that the column is not dead space.
  5. As a screener user, I want an empty result to mean "nothing matched", so that I can trust a zero-row table.
  6. As a screener user, I want a filter on a Fundamental Field to consider every symbol in the Universe, so that my screen is not silently restricted to an arbitrary hundred.
  7. As a screener user, I want to know how many symbols matched when more matched than were returned, so that I know a Page is not the whole answer.
  8. As a screener user, I want to know when a screen could only examine a Page, so that I do not read a partial answer as a complete one.
  9. As a screener user, I want to be told when fundamental data could not be loaded, so that I do not misread unavailability as absence of matches.
  10. As a screener user, I want screens with no Fundamental Field to stay fast, so that the common case is not taxed by data I did not ask for.
  11. As a screener user, I want Quick Filters and advanced filters to behave identically with respect to fundamentals, so that where I set a criterion does not change whether it works.
  12. As a screener user, I want a fundamental criterion combined with a price or volume criterion to apply both, so that compound screens are trustworthy.
  13. As a screener user, I want OR logic across a fundamental and a non-fundamental criterion to work, so that I can express either/or screens.
  14. As a screener user, I want nested filter groups mixing fundamental and non-fundamental fields to work at any depth, so that complex screens are not silently degraded.
  15. As a screener user, I want sorting by a Fundamental Field to order by real values, so that ranking by Margin of Safety is meaningful.
  16. As a screener user, I want a sort on a Fundamental Field to rank the whole Universe before the Page is cut, so that the top of my ranking is genuinely the top.
  17. As a screener user, I want fundamental values to be consistent whether my result came from the Screener Snapshot or a live fetch, so that repeated runs agree.
  18. As a screener user, I want a refreshed screen to re-apply fundamentals, so that a manual refresh does not blank out columns.
  19. As a screener user, I want fundamental behaviour to hold when the api falls back to a stale Screener Snapshot, so that degraded mode is still correct.
  20. As a screener user, I want a universe restriction such as VN30 combined with a fundamental filter to work, so that I can screen fundamentals within an index.
  21. As a screener user, I want an exchange or industry restriction combined with a fundamental filter to work, so that I can screen fundamentals within a sector.
  22. As an API consumer, I want the typed fundamental query parameters to keep working exactly as they do today, so that existing integrations do not break.
  23. As an API consumer, I want include_fundamental=true to force enrichment even with no fundamental filter, so that I can request the columns explicitly.
  24. As an API consumer, I want a Fundamental Field referenced only inside the serialized filter blob to trigger enrichment, so that I do not have to duplicate criteria as typed parameters.
  25. As an API consumer, I want response metadata to state Screen Scope, match count, and whether enrichment succeeded, so that I can handle degraded data programmatically.
  26. As an API consumer, I want a malformed filter blob to fail predictably rather than silently disabling enrichment, so that I can detect my own bugs.
  27. As a frontend developer, I want the screener client to keep sending one serialized blob, so that I do not have to maintain a parallel list of typed fundamental parameters.
  28. As a frontend developer, I want to add a Saved Screen using a Fundamental Field without touching backend code, so that Saved Screens stay a frontend concern.
  29. As a maintainer, I want the set of Fundamental Fields declared in exactly one place, so that adding a field cannot silently reintroduce this class of bug.
  30. As a maintainer, I want one request to perform at most one fundamental read, so that moving enrichment earlier does not double the cost.
  31. As a maintainer, I want a regression test that sends the real request shape web sends, so that this defect cannot reappear undetected.
  32. As a maintainer, I want a test proving the default path still performs no fundamental read, so that the performance win is protected.
  33. As a maintainer, I want the enrich-then-filter-then-limit ordering asserted by a test, so that a later refactor cannot quietly reverse it.
  34. As a maintainer, I want Fundamental Enrichment failure to leave rows intact rather than raise, so that a Mongo outage degrades the screener instead of breaking it.
  35. As a maintainer, I want a warning logged when a filter targets a Fundamental Field that is entirely empty, so that silent empty results become diagnosable.
  36. As an operator, I want to know at startup whether the fundamentals index exists, so that I find out before users report slow screens.
  37. As an on-call engineer, I want to distinguish "Mongo unavailable" from "no matches" in logs and metadata, so that I can triage an empty-screener report quickly.

Implementation Decisions

Baseline. The uncommitted working-tree change on fix/qa-verification-defects is the starting point, not something to revert. Its goal — keep the fundamental read off the default request path — is correct and must be preserved. This spec fixes how that decision is made, and where enrichment sits in the pipeline.

One declaration of what counts as fundamental. There are currently two hand-maintained lists that must agree and do not: the gate's tuple of five parameter names, and the advanced-filter predicate that decides whether the early-limit shortcut may fire. Neither mentions the serialized blob; the second mentions no fundamental parameter at all. Both are replaced by deriving the canonical field set from the existing document-to-attribute map, which already enumerates all seven Fundamental Fields. Adding a Fundamental Field to that map must be sufficient to make the whole path work.

One decision per request, computed once. Whether a request touches fundamentals is determined once, at the top of the handler, before any path branches. The result is threaded explicitly to the three places that need it: the early-limit shortcut, the enrichment step, and the response metadata. This is deliberately not a shared predicate function called three times — that would let the call sites drift in their arguments even while sharing logic, which is how the current pair diverged.

The decision reads the serialized blob. Detection walks the parsed filter tree — nested groups, arbitrary depth, both AND and OR — and opens if any condition names a Fundamental Field. Parsing reuses the existing filter parser rather than introducing a second JSON path. A blob that fails to parse is treated as naming no Fundamental Field, matching how the filter service already handles unparseable input; the existing parse-failure log line is the diagnostic.

Sort fields count. A sort on a Fundamental Field triggers enrichment on the same footing as a filter. Today such a sort is accepted, orders an entirely empty column, and returns arbitrary order — the same class of silent wrong answer as the filter defect, reached by a different door. It falls out of the single decision above at no extra cost.

Enrichment moves ahead of both truncations. This is the substantive ordering change, and the reason the earlier body was insufficient. The Candidate Set is cut to a Page in two places: the early-limit shortcut taken when no Advanced Filter is present, and the final slice after filtering and sorting. Fundamental Enrichment must precede both, on every row-emitting path. Moving it ahead of only the second leaves fundamental filters screening an arbitrary Page.

Merge and filter separate. Enrichment currently bundles two operations: joining Fundamental Snapshot values onto rows, and applying the typed fundamental filters. Only the join is position-sensitive. The join moves early; the typed filters stay where they are, so their semantics do not change. The helper that binds the two together is removed, because binding them is what forces the defect.

Exactly one fundamental read per request. With the join moved early, the late call site must be removed outright rather than made conditional or idempotent. An idempotency check on "has this row already been enriched" cannot work: a symbol legitimately absent from the fundamentals corpus is indistinguishable from a symbol not yet joined.

Four paths, two insertion points. Three of the four row-emitting paths — fresh Screener Snapshot, source-agnostic fallback, and stale fallback after a provider error — share one preparation helper. The live path is inline. So the change lands in two locations, not four. The two paths that emit no rows need nothing.

Screen Scope is reported, not enforced. On the live path the Universe is already truncated upstream at fetch time, so full-Universe screening is unreachable there regardless of this change. Rather than failing such requests, the response reports that only a Page was examined, along with the size of the Candidate Set actually screened. Refusing would turn a cold Screener Snapshot into a screener outage, and a cold Screener Snapshot is routine before the morning sync.

Match count is reported; pagination is not built. The response states how many rows matched before the Page was cut. Returning 100 of 400 matches while implying 100 is the answer repeats the sin this ticket exists to fix. Real pagination — offset or cursor — is a separate ticket with its own consistency questions, since the matched set is recomputed per request.

The fundamentals index is asserted, not created. Enrichment now aggregates over roughly 1,600 Fundamental Snapshots rather than 100. That aggregation depends on the (symbol, snapshotDate) index, which today is created only by the manual backfill script. Startup checks for it and logs loudly if absent, and the health surface reports it. Startup does not fail — a missing index makes screens slow, not wrong — and startup does not create it, because the backfill script owns that collection.

Empty columns are diagnosed. The filter service passes a condition through unchanged when its column is absent, but all seven Fundamental Fields are always declared, so the column always exists and is merely empty when enrichment is skipped. That is precisely why the failure is silent: the escape hatch never fires, and an equality comparison against an entirely empty column yields nothing. A warning is added when a condition targets a column that exists but is wholly empty. Behaviour is unchanged; this is a diagnostic only.

Frontend contract is unchanged. Web keeps serializing Quick Filters and advanced conditions into one blob. No typed fundamental parameters are added to the client, and no Saved Screen definitions change. The fix is entirely backend. The alternative — teaching web to hoist fundamental conditions into typed parameters — would put the canonical field list on both sides of the wire and reintroduce exactly the duplication that caused this bug.

API compatibility. Typed fundamental parameters, include_fundamental, and all existing response fields keep their current behaviour. The change is additive: requests that work today continue to work identically.

Testing Decisions

What makes a good test here. Tests assert what a caller can observe from an HTTP response — status, row membership, field values, metadata. Not which internal helper ran. The one deliberate exception is the performance guarantee: "the default path performs no fundamental read" is observable only by asserting the read is not invoked, and the existing suite already encodes it that way. That exception stays narrow and is not extended.

Seam. The existing HTTP seam in the fundamental screener test module: an async client issuing real GET /api/v1/screener/ requests, with the vnstock fetcher and the Mongo read monkeypatched. This is the highest seam available and already exercises the gate. No new seam is introduced — every new case is a new test at this same seam, which keeps the whole feature covered from one place. Note that the monkeypatch target moves when the join relocates; the seam itself does not.

Prior art. The two tests added alongside the working-tree change are the direct template: one asserting the default request performs no fundamental read, one parameterized over typed triggers asserting the join runs exactly once and the value reaches the response. New cases follow their structure, naming, and monkeypatch style. The neighbouring enrichment test module is the reference for row fixtures.

Cases to add, all at that seam:

  • A request whose only fundamental reference is inside the serialized blob triggers enrichment and returns matching rows — the exact shape web sends for FCF Margin Expansion. This is the headline regression test.
  • The same, with the fundamental condition nested inside a sub-group, under both AND and OR logic.
  • A blob mixing a fundamental and a non-fundamental condition applies both.
  • A blob naming no Fundamental Field still performs no fundamental read — the performance guarantee, restated against the new decision point.
  • A malformed blob does not trigger enrichment and does not error the request.
  • Ordering against the Page cut: with a Candidate Set larger than the Page and matching symbols positioned beyond the first Page, a fundamental filter returns those symbols. This fails if enrichment runs after either truncation, and is the test that pins the central decision.
  • The early-limit shortcut does not fire when the request names a Fundamental Field.
  • A sort on a Fundamental Field triggers enrichment and orders by real values.
  • Exactly one fundamental read occurs per request.
  • Screener Snapshot and stale-fallback paths enrich under the same conditions as the live path.
  • Enrichment failure leaves rows intact, returns 200, and marks the degraded state in metadata.
  • Metadata reports Screen Scope, Candidate Set size, and match count correctly for a full-Universe screen and for a Page-scoped one.

Unit-level, below the HTTP seam, only for the filter-tree walk: nesting depth, AND/OR, unknown fields, empty and malformed input. A pure function over parsed input, cheaper to test directly than to drive through HTTP on every branch. Additive to the HTTP cases, not a substitute.

Commands. Backend suite python -m pytest apps/api/tests/test_api/test_screener_fundamental.py -v; lint python -m ruff check apps/api; full gate pnpm run ci:gate.

Out of Scope

  • Any change to web. No Saved Screen definitions, no client parameter types, no widget code.
  • Pagination. Match count is reported; offset and cursor are a separate ticket.
  • The live path's inability to screen the full Universe. Reported via Screen Scope, not fixed — fixing it means changing how the provider bounds its fetch.
  • Issue Screener live path can shrink the shared daily snapshot to one request's limit #8, the live path overwriting the shared Screener Snapshot. Same file, same pipeline, unrelated mechanism, and it turns on an ownership question that is not settled.
  • Backfilling or repairing the fundamentals collection. This spec assumes the data is correct where present.
  • Creating the fundamentals index. Asserted and reported only.
  • Persisting Fundamental Fields onto the Screener Snapshot. Deliberately rejected for now — see ADR-0002.
  • Appwrite writes, which stay frozen.
  • Broadening the filter service's operator set or changing its empty-value semantics. Only a diagnostic warning is added.
  • The discovery filter path, which has the correct ordering already and is the model being copied.
  • Surfacing the new metadata in the UI. The api will report it; consuming it is follow-up work.
  • Committing or pushing the working-tree change. The author owns that branch.

Further Notes

Recorded decisions. Two ADRs were written while specifying this, both in vnibb/apps/api/docs/adr/: ADR-0001 on why enrichment precedes truncation, and ADR-0002 on why Fundamental Fields are joined per request rather than stored on the Screener Snapshot. ADR-0002 matters here because storing them would dissolve this entire problem — no detection, no ordering constraint, Fundamental Fields become ordinary columns. It is rejected only because the two sources refresh on independent cadences with independent as-of dates. Revisit if the fundamental rebuild becomes scheduled.

Provenance. Found while validating an explainer for the uncommitted change, then confirmed by static reading. Four observations close off the benign interpretation:

  1. The gate opens only on typed parameters.
  2. Web sends fundamental criteria exclusively inside the serialized blob and declares no typed fundamental parameters.
  3. All seven Fundamental Fields are always declared on the row model, so the column always exists — the filter service's "column absent, pass through" escape hatch never fires.
  4. Both truncations, and the advanced filters, run before enrichment.

Point 3 makes this severe rather than cosmetic. An equality filter against an entirely empty column returns nothing, so FCF Margin Expansion yields zero rows rather than rows with a blank column.

Why the tests missed it. Both new tests drive typed query parameters. Neither sends a serialized blob. The request shape web actually produces was untested, so the suite passes while the feature is broken. The headline regression test closes exactly that gap.

Pre-existing versus introduced. The ordering problem predates the working-tree change — enrichment has always sat at the end of the pipeline. Before the change the join was unconditional, so Fundamental Fields were populated for display even though a filter on them could never match. The change turned a latent filtering bug into a visible one and additionally emptied the columns. Both halves need fixing for the affected Saved Screens to work, which is why they are scoped together.

Fastest reproduction, against a running api:

GET /api/v1/screener/?limit=5&use_cache=false&filters={"logic":"AND","conditions":[{"field":"fcf_positive","operator":"eq","value":true,"enabled":true,"id":"x"}]}

Expected today: 200 with zero rows. Expected after the fix: symbols with positive free cash flow, FCF positive populated, and metadata stating Screen Scope and match count.

Verification note. Everything here comes from static reading of the working tree at HEAD 143cbda on fix/qa-verification-defects. No tests were executed and no services were started, so the reproduction above is a prediction from source, not an observed run. It should be the implementer's first action.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingready-for-agentFully specified, ready for an AFK agent

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions