Code Review

Overall: The motivation is solid and the approach is correct — jsonb_build_array with derived column aliases cleanly avoids both the TimeFusion compatibility issue and the NULL-byte/lone-surrogate hazard. The hasql composition is idiomatic. A few issues worth addressing before merge:

1. wrapForRowExtraction is exported but has zero callers — and duplicates logic in executeArbitraryQuery

wrapForRowExtraction :: Int -> Text -> Text in Pkg.Parser and the let aliases = … block inside executeArbitraryQuery implement identical logic:

-- Parser.hs
let aliases = T.intercalate "," [ "c" <> show i | i <- [1 .. n] ]

-- LogQueries.hs
let aliases = T.intercalate "," ["c" <> show i | i <- [1 .. colCount]]

The PR adds the helper for this purpose but then doesn't call it. The type mismatch (Text vs HI.Sql) is the blocker, but it's solvable. The recommended fix is to introduce a HI.Sql-level twin in LogQueries (or Pkg.DeriveUtils) and have executeArbitraryQuery delegate to it:

wrapForRowExtractionSql :: Int -> HI.Sql -> HI.Sql
wrapForRowExtractionSql n inner =
  let aliases = T.intercalate "," ["c" <> show i | i <- [1 .. n]]
  in rawSql ("SELECT jsonb_build_array(" <> aliases <> ") FROM (")
     <> inner
     <> rawSql (") sub(" <> aliases <> ")")

executeArbitraryQuery :: DB es => Int -> HI.Sql -> Eff es (V.Vector (V.Vector AE.Value))
executeArbitraryQuery colCount querySql =
  V.fromList . mapMaybe jsonArrayToVector <$>
    Hasql.interp (wrapForRowExtractionSql colCount querySql)

The Text-level wrapForRowExtraction in Parser.hs can then be expressed without repeating the alias formula (or defined as toText . wrapForRowExtractionSql n . rawSql if that's ergonomic).

2. YAGNI: don't export wrapForRowExtraction before it has a real caller

Exporting a function with no callers expands the public API surface and forces future renaming to be a breaking change. Remove it from the export list now; re-add it when the first real caller lands. The PR description says "exposed for future callers" but that's exactly the scenario to avoid.

3. The executeArbitraryQuery docstring is misleading

Caller passes the expected column count — see Pkg.Parser.wrapForRowExtraction.

The comment implies executeArbitraryQuery is implemented in terms of wrapForRowExtraction, but it isn't. Either wire the actual dependency or drop the cross-reference.

4. colCount = 0 edge case

Both callers derive colCount from a list length (length queryComponents.toColNames, length (colsNoAsClause cols)). If either ever returns [] (e.g. a malformed query, a SELECT * that wasn't expanded, a future caller mistake), the generated SQL becomes:

SELECT jsonb_build_array() FROM (...) sub()

sub() with zero column aliases is not valid standard SQL and may fail at parse time on TimeFusion or confuse PG. A cheap guard up-front would prevent a confusing runtime error:

executeArbitraryQuery 0 _ = pure V.empty  -- or throwError

Minor

• The wrapForRowExtraction docstring says "Replaces the legacy json_agg(json_each(row_to_json(sub.*)) WITH ORDINALITY)" — the actual legacy expression was json_agg(x.value ORDER BY x.ordinality) … FROM json_each(row_to_json(sub.*)) WITH ORDINALITY AS x. Small inaccuracy.
• executeSecuredQuery left on the legacy wrapper is fine per the PR description, but worth a TODO comment so it doesn't get overlooked.
• Test plan: only compile-check is ticked. Would be good to get the PG regression suite result before merge, even if the TimeFusion manual test depends on the companion PR.

🤖 Generated with Claude Code

Code Review

Overview

This PR replaces the json_each(row_to_json(*)) WITH ORDINALITY row-extraction wrapper with SELECT jsonb_build_array(c1,…,cN) FROM (inner) sub(c1,…,cN) to fix TimeFusion compatibility and PostgreSQL's JSON-parser hazard on NULL bytes. The approach is sound, but there is a critical count-mismatch bug and a few style issues.

🐛 Critical Bug — column alias count mismatch when hasCountOver = True

selectLogTable passes length queryComponents.toColNames as the column count. toColNames returns finalColumns, which is set to listToColNames nq.nqSelectCols — the displayed columns only.

But when hasCountOver = True (triggered by aggregations, service-entry-spans target, and the summarize path), the inner SQL appends count(*) OVER() as _total_count as an extra column. PostgreSQL then sees:

SELECT jsonb_build_array(c1,…,cN) FROM (
  SELECT col1,…,colN, count(*) OVER() as _total_count …
) sub(c1,…,cN)          -- N aliases vs N+1 output columns → ERROR

PostgreSQL will reject this with column alias list must be the same length as the number of output columns. The dropLast logic on line 342 relies on that extra column arriving at position N+1, so it must be included in the alias list.

Fix: pass length queryComponents.toColNames + (if queryComponents.hasCountOver then 1 else 0) — or cleaner, add an actualColCount :: Int field to QueryComponents that accounts for the count-over column from the start.

Style / Succinctness

List comprehension → point-free map (line 214):

-- current
let aliases = T.intercalate "," ["c" <> show i | i <- [1 .. colCount]]

-- more idiomatic
let aliases = T.intercalate "," $ map (("c" <>) . show) [1 .. colCount]

Overly long block comment (lines 199–210): half of it restates the PR description. The doc-comment only needs to say what the contract is (what the Int argument means and the <= 0 guard), not why the old wrapper was removed. That belongs in git history.

Missing from diff — PR description mismatch

The summary says:

New helper Pkg.Parser.wrapForRowExtraction :: Int -> Text -> Text exposed for future callers.

No such function appears in the diff. Either it was dropped before the PR was opened, or the description is stale. Worth clarifying so reviewers and future readers aren't chasing a phantom export.

Minor — colsNoAsClause called twice in fetchEventExamples

Lines 853 and 869 both call colsNoAsClause cols. The result is only used for processedCols (line 853) and the count (line 869). Binding it once is cleaner:

let rawCols     = colsNoAsClause cols
    processedCols = map … rawCols
    …
rows <- … $ executeArbitraryQuery (length rawCols) q

What's good

• The colCount <= 0 guard is correct and the comment explains why (zero-column sub() is invalid SQL).
• Using static integer aliases (c1, c2, …) instead of echoing user-supplied column names eliminates any injection surface in the wrapper itself.
• Restricting the migration to executeArbitraryQuery only (leaving executeSecuredQuery on the legacy wrapper with a TODO) is the right conservative call.

Found 1 test failure on Blacksmith runners:

Failure

Code Review

The PR makes sense and the motivation is solid — replacing the json_each/row_to_json/WITH ORDINALITY wrapper with jsonb_build_array is a clean fix for both the TimeFusion compatibility issue and the PG JSON parser hazard. A few suggestions below.

fromEnum over if … then 1 else 0

The hasCountOver adjustment can use the standard Bool Enum instance instead of the verbose conditional:

-- current
executeArbitraryQuery (length queryComponents.toColNames + if queryComponents.hasCountOver then 1 else 0) (rawSql q)

-- simpler
executeArbitraryQuery (length queryComponents.toColNames + fromEnum queryComponents.hasCountOver) (rawSql q)

fromEnum True == 1, fromEnum False == 0 — no need for the inline if.

wrapForRowExtraction is mentioned in the PR description but missing from the diff

New helper Pkg.Parser.wrapForRowExtraction :: Int -> Text -> Text exposed for future callers.

There's no change to Pkg.Parser.hs and the function doesn't appear in the repo. Either the PR description is inaccurate, or the extraction was intended and dropped. The aliases-building + SQL wrapping is currently duplicated between the rawSql calls inside executeArbitraryQuery. If callers are expected to call this from outside LogQueries, the helper should be added (and exported). If not, the PR description should be updated.

Alias list shape — consider V.mapMaybe to avoid the intermediate list

-- current
pure $ V.fromList $ mapMaybe jsonArrayToVector results

-- avoids rebuilding from list
pure $ V.mapMaybe jsonArrayToVector (V.fromList results)

Minor, but V.mapMaybe fuses the filter into the vector build. Only worth doing if results is large.

Column count mismatch fails at runtime, not compile time

The new Int parameter makes the caller responsible for keeping the column count in sync with the inner SELECT. A mismatch produces a runtime PostgreSQL error ("column alias list must be the same length…"). The hasCountOver adjustment in selectLogTable is already a hint that this is fragile. A brief note in the type signature or a newtype ColumnCount wrapper would make misuse more visible, though I recognise this may be over-engineering for one function.

TODO comment on executeSecuredQuery — track it

The inline TODO is good hygiene, but if there's no associated ticket the follow-up is easy to lose. Worth linking to one if it exists.

Test coverage

No tests cover:

• The colCount <= 0 guard (returns V.empty).
• The generated SQL shape (at least a unit test on the aliases construction would catch regressions if the wrapping logic changes).

These aren't blocking but would make the contract easier to verify.

Overall this is a well-motivated, focused change. The fromEnum simplification and clarifying the wrapForRowExtraction status are the two things worth addressing before merge.

Code Review

Overview

Clean, focused fix that replaces the json_each(row_to_json(sub.*)) WITH ORDINALITY wrapper with jsonb_build_array(c1,…,cN) to unblock log-explorer queries on the TimeFusion backend. Logic is correct; a few style/conciseness notes below.

Suggestions

1. Over-verbose Haddock block (executeArbitraryQuery)

The 8-line comment block spends most of its words describing the old implementation. Per the project's comment policy, only the non-obvious why should remain. Suggest trimming to:

-- Wraps the inner SELECT so each row arrives as a jsonb array of column values.
-- Uses jsonb_build_array(c1,…,cN) rather than json_each/row_to_json/WITH ORDINALITY
-- — the latter is unsupported by TimeFusion and trips PG's JSON parser on NULL bytes.

2. Overlong call-site comment in selectLogTable

The 3-line comment before the executeArbitraryQuery call could be a tighter single line:

-- +1 for count(*) OVER() appended to SELECT but absent from toColNames
$ executeArbitraryQuery (length queryComponents.toColNames + fromEnum queryComponents.hasCountOver) (rawSql q)

3. fromEnum hasCountOver — consider bool

fromEnum hasCountOver is valid Haskell but slightly opaque to readers unfamiliar with the Enum Bool instance. The Data.Bool.bool function (available via Relude) is more explicit:

$ executeArbitraryQuery (length queryComponents.toColNames + bool 0 1 queryComponents.hasCountOver) (rawSql q)

Minor style point; either is defensible.

4. Silent colCount <= 0 → V.empty could mask caller bugs

Dropping the query silently when colCount ≤ 0 is documented, but a miscounting caller (e.g. length [] = 0 on an empty column list) would get an empty result with no indication the query was never executed. Consider a logWarn / traceWhen call there, or at minimum an assertion in tests.

5. Intermediate list allocation in executeArbitraryQuery

pure $ V.mapMaybe jsonArrayToVector (V.fromList results)

results :: [AE.Value] is fully materialised before V.fromList converts it, then V.mapMaybe filters. If Hasql.interp can be made to decode directly into a Vector, the intermediate list allocation disappears. Not a blocker, but worth noting for a follow-up.

What's good

• Correctness: the alias list is computed once and reused in both the SELECT clause and the sub(...) alias list — no divergence risk.
• fetchEventExamples: binding rawCols = colsNoAsClause cols once and reusing it for both the column-count and the processedCols map is a clean improvement.
• Guard clause (colCount <= 0) prevents issuing invalid SQL — good defensive check.
• Overall diff is minimal and surgical (30 add / 13 del, single file).

Test coverage

The PR test plan notes manual verification against a dev TimeFusion cluster is still pending (blocked on companion TF PR deploy). The PG path should be covered by existing regression tests, but it would be worth adding an automated integration test that exercises executeArbitraryQuery with a known column count against both backends once the TF companion lands.

Contributing to Monoscope

Thank you for your interest in contributing! This guide covers development setup, running the project locally, testing, code style, and the PR workflow.

Development Setup

Prerequisites

• GHCup — Haskell toolchain manager: curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | sh
• Docker & Docker Compose — For database and services
• Node.js 22 — For frontend assets (web-components, Tailwind CSS)
• LLVM — Required for GHC compilation
• libpq — PostgreSQL development libraries

Initial Setup

# Clone the repository (fork first if contributing)
git clone https://github.com/monoscope-tech/monoscope.git
cd monoscope

# Install Haskell dependencies
cabal update

# Activate the pre-commit hook for auto-formatting
git config core.hooksPath .githooks

Running Locally with Docker Compose

The fastest way to get a running instance:

docker-compose up

Visit http://localhost:8080 — default credentials are admin / changeme.

Optional Profiles

Start additional services with Docker Compose profiles:

# pgAdmin for database management (http://localhost:5050)
docker-compose --profile dev up

# Ephemeral test database (port 5433)
docker-compose --profile test up

Environment Variables

Copy .env.example to .env and adjust as needed. The defaults are sufficient for local development.

Building from Source

# Build all targets
cabal build all -j

# Run the application
cabal run

# Or with live-reload using ghcid
make live-reload

Running Tests

Unit Tests (fast, no database required)

cabal test unit-tests --ghc-options="-O0" --test-show-details=direct

Or via Makefile:

make test-unit

Doctests

cabal test doctests --ghc-options="-O0" --test-show-details=direct

Or via Makefile:

make test-doctests

Integration Tests (requires external PostgreSQL)

USE_EXTERNAL_DB=true cabal test integration-tests --ghc-options="-O0" --test-show-details=direct

Or via Makefile:

make test-integration

All Tests

make test

Run Specific Tests

# Match by test name pattern
cabal test unit-tests --test-options='--match="/pattern/"'

# Run with live file-watching
make live-test-reload-unit

Code Style

Formatting

Fourmolu is used for Haskell code formatting. It is automatically run via a GitHub Actions workflow on every push — any formatting changes are auto-committed to your branch. You don't need to format manually before opening a PR.

To format locally:

make fmt

To format on commit (pre-commit hook is in .githooks/pre-commit):

# Already activated via git config core.hooksPath .githooks
# To bypass for a single commit:
SKIP_FOURMOLU=1 git commit ...

Linting

HLint checks for common Haskell code smells and style issues.

make lint

To auto-fix lint issues:

make fix-lint

Frontend

JavaScript/TypeScript uses Prettier (configured in .prettierrc). Run it via:

npx prettier --write .

Tailwind CSS is processed during the build — run it manually with:

make post-css

PR Workflow

Fork & Branch

1. Fork the repository on GitHub.

2. Add your fork as a remote:

   git remote add fork https://github.com/<your-username>/monoscope.git

3. Create a feature branch from master:

   git checkout master
   git pull upstream master
   git checkout -b feat/your-feature-name

Making Changes

1. Make your changes, writing tests as needed.

2. Ensure tests pass:

   cabal test unit-tests --ghc-options="-O0"
   cabal test doctests --ghc-options="-O0"

3. Run the linter:

   make lint

4. Commit your changes. The pre-commit hook will run fourmolu on staged .hs files. To bypass the hook for a specific commit (e.g., for documentation-only changes):

   SKIP_FOURMOLU=1 git commit -m "your message"

Opening a Pull Request

1. Push your branch to your fork:

   git push fork feat/your-feature-name

2. Open a PR against monoscope-tech/monoscope on GitHub.

3. Fill in the PR template with a clear description of the changes.

4. A maintainer will review and request changes if needed.

Auto-Formatting on Push

The fourmolu.yml GitHub Actions workflow runs on every push to the repository. It automatically formats any .hs files that don't match the fourmolu configuration and commits the changes back to your branch. This means:

• Don't worry about formatting — fourmolu will fix it for you after you push.
• Check the diff after pushing — the bot may have committed formatting fixes.
• Amend or rebase as needed if the bot's commit conflicts with your changes.

Project Structure

monoscope/
├── src/              # Haskell application source
├── static/           # Static assets, migrations, public files
├── web-components/   # TypeScript frontend components
├── cli/              # CLI tool source
├── proto/            # Protocol Buffer definitions
├── test/             # Test scripts and helpers
├── tests/            # Integration test suites
├── app/              # Application entry points
├── npm/              # Node.js helper scripts
├── config/           # Application configuration
├── docs/             # Documentation
└── cabal.project     # Cabal build configuration

Getting Help

• Discord — Join the community at https://discord.gg/BSFCaUHxt4
• GitHub Issues — For bugs and feature requests
• Documentation — https://docs.monoscope.tech

Code Review

Overview

Clean, well-motivated refactor. The switch from json_each(row_to_json(*)) WITH ORDINALITY to jsonb_build_array(c1,...,cN) is the right fix for TimeFusion compatibility and the NULL-byte / lone-surrogate JSON parser issue. Callers are updated correctly; the executeSecuredQuery carve-out with TODO is reasonable.

Issues / Suggestions

1. Unnecessary intermediate vector in executeArbitraryQuery

The new form allocates a full V.Vector AE.Value just to filter it with V.mapMaybe, and is inconsistent with executeSecuredQuery which still uses the list-first form:

-- new (worse — intermediate vector):
pure $ V.mapMaybe jsonArrayToVector (V.fromList results)

-- old pattern (still used in executeSecuredQuery — prefer this):
pure $ V.fromList $ mapMaybe jsonArrayToVector results

2. Alias construction — list comprehension that is just map

let aliases = T.intercalate "," ["c" <> show i | i <- [1 .. colCount]]

With OverloadedLists project-wide, list comprehensions that are just map are better written as map. Point-free:

let aliases = T.intercalate "," $ map (("c" <>) . show) [1 .. colCount]

3. Doc comment over-explains the guard

The sentence about colCount <= 0 duplicates the guard directly below it — drop it.

4. Fragile caller-side column-count arithmetic (maintenance risk)

executeArbitraryQuery (length queryComponents.toColNames + fromEnum queryComponents.hasCountOver) (rawSql q)

If queryASTToComponents ever adds another synthetic column, callers silently produce a runtime Postgres error rather than a compile-time failure. A helper on QueryComponents gives a single update point:

emittedColCount :: QueryComponents -> Int
emittedColCount qc = length qc.toColNames + fromEnum qc.hasCountOver

5. executeSecuredQuery TODO — trim to one line

-- TODO: migrate to jsonb_build_array once column count is derivable for arbitrary user/LLM SQL.

Positives

• rawCols binding in fetchEventExamples eliminates the duplicate colsNoAsClause cols call — good DRY cleanup.
• fromEnum hasCountOver is concise and idiomatic.
• Updated RawSessionRow comment accurately reflects the new state.
• No injection surface: aliases are built from [1..colCount] (integer range only).

Code Review

Overview

Clean, well-motivated fix. Replaces the json_each / row_to_json / WITH ORDINALITY wrapper with jsonb_build_array(c1,…,cN) to unblock TimeFusion compatibility and fix the NULL-byte/surrogate JSON parser issue. The PR description is thorough and honest about what's left (the executeSecuredQuery TODO).

Issues

Regression: intermediate vector allocation in executeArbitraryQuery

The PR changes:

-- before (list-based mapMaybe, then one V.fromList)
pure $ V.fromList $ mapMaybe jsonArrayToVector results

to:

-- after (full V.fromList first, then V.mapMaybe)
pure $ V.mapMaybe jsonArrayToVector (V.fromList results)

This allocates a full intermediate Vector AE.Value before filtering. The original order (filter the list first, then pack into a vector once) is cheaper. Meanwhile executeSecuredQuery keeps the original pattern — so this is now inconsistent as well as slightly worse. Suggest reverting to:

pure $ V.fromList $ mapMaybe jsonArrayToVector results

Suggestions

fromEnum hasCountOver is terse but obscure

-- current
length queryComponents.toColNames + fromEnum queryComponents.hasCountOver

fromEnum on Bool is idiomatic in Haskell but surprising for readers not expecting it here. The inline comment -- +1 for count(*) OVER() helps, but an explicit form would be self-documenting:

let extraCol = if queryComponents.hasCountOver then 1 else 0

Minor — keep it if the team is comfortable with fromEnum Bool.

SQL alias string is built and interpolated twice

aliases appears both in the SELECT jsonb_build_array(…) and in the sub(…) column-name list. That's structurally unavoidable, but it could be broken into two named bindings to make the intent clearer to the next reader:

let cols      = T.intercalate "," ["c" <> show i | i <- [1..colCount]]
    subAlias  = "sub(" <> cols <> ")"

Positive notes

• colCount <= 0 guard via a pattern guard clause is the right approach — avoids invalid SQL cleanly.
• Binding rawCols = colsNoAsClause cols once in fetchEventExamples and reusing it for both processedCols and the column count is a good DRY fix.
• Updated RawSessionRow comment accurately reflects the new semantics rather than leaving a stale workaround note.
• No SQL injection risk: aliases is derived entirely from an Int.
• show i correctly produces Text via Relude's show :: Show a => a -> Text — no T.pack noise needed.

Summary

One real issue (the V.fromList + V.mapMaybe ordering inversion creates an unnecessary intermediate vector and diverges from the pattern in executeSecuredQuery), one minor style note (fromEnum), otherwise a solid and well-scoped change.

Code Review

Overview

Replaces the json_each(row_to_json(*)) WITH ORDINALITY wrapper in executeArbitraryQuery with jsonb_build_array(c1,…,cN), fixing TimeFusion compatibility and NULL-byte/surrogate safety. The approach is sound and the callers are updated correctly.

Issues & Suggestions

1. fromEnum on Bool — prefer bool from Relude

-- current
length queryComponents.toColNames + fromEnum queryComponents.hasCountOver

-- cleaner — Relude exports bool :: b -> b -> Bool -> b
length queryComponents.toColNames + bool 0 1 queryComponents.hasCountOver

fromEnum on Bool is spec-guaranteed (False=0, True=1) but non-obvious to readers. Relude already exports bool, which is in use elsewhere in the project.

2. Fragile column-count invariant — worth a type-level assertion or closer coupling

The caller arithmetic length toColNames + fromEnum hasCountOver encodes an implicit invariant: the number of columns in the generated SELECT equals toColNames plus one iff hasCountOver. This silently breaks if the SQL in selectLogTable grows another column. A let colCount = length toColNames + bool 0 1 hasCountOver with a comment co-located with the SELECT construction would make the contract clearer, or better yet, have queryASTToComponents return the column count alongside the query text so callers don't have to recompute it.

3. aliases — list comprehension vs map is style-neutral, but spacing

-- current — fine
let aliases = T.intercalate "," ["c" <> show i | i <- [1 .. colCount]]

-- marginally shorter using point-free
let aliases = T.intercalate "," $ map (("c" <>) . show) [1 .. colCount]

Neither is wrong; just noting the alternative if point-free style is preferred elsewhere.

4. rawSql called with the same aliases text twice — consider a helper

rawSql ("SELECT jsonb_build_array(" <> aliases <> ") FROM (")
  <> querySql
  <> rawSql (") sub(" <> aliases <> ")")

Minor: the full SQL template is split across three fragments. A single-line helper or let binding for the wrapper would be easier to eyeball for SQL correctness:

let wrap inner = rawSql ("SELECT jsonb_build_array(" <> aliases <> ") FROM (") <> inner <> rawSql (") sub(" <> aliases <> ")")

5. Test coverage

No new tests are added. Would be useful to have:

• A unit test that executeArbitraryQuery 0 _ returns V.empty (the early-exit guard).
• A property/unit test that aliases for N columns generates the correct SQL fragment (guards against off-by-one if the range is ever changed).

The colCount arithmetic in the selectLogTable caller (length toColNames + fromEnum hasCountOver) is particularly susceptible to silent regression — a doctest or integration test that exercises the hasCountOver = True path would help.

6. executeSecuredQuery TODO — consider a GitHub issue

The inline TODO is fine for now, but since executeSecuredQuery is the path for user/LLM-submitted SQL, it would be worth opening a tracking issue so it doesn't get lost. The PR description already captures the constraint (column count not derivable for arbitrary SQL), so this is low-priority, just a tracking note.

Minor / Nits

• RawSessionRow doc comment update is accurate and an improvement — good cleanup.
• rawCols = colsNoAsClause cols in fetchEventExamples avoids the double call — good.
• No SQL injection risk: aliases is derived purely from Int via show, so rawSql usage is safe here.

Summary

The core change is correct and solves a real compatibility problem. Main asks: (1) switch fromEnum to bool 0 1, (2) add at least one test for the colCount = 0 guard and the column-count arithmetic, (3) consider returning the column count from queryASTToComponents to remove the fragile manual arithmetic in callers.

Superseded by #423 and its follow-ups on master (b68260d, 0078dc5, 89817bf, 4acc4e7): the jsonb_build_array wrap is now inlined in SQL codegen, and executeSecuredQuery is covered via jsonb_build_array(sub.*) (TF WildcardFnArgExpander / PG record overload from migration 0099), so the manual column-count threading here is no longer needed.