autogenerate tasks from existing catalogs

.agents/skills/code-review/SKILL.md

@@ -0,0 +1,101 @@
+---
+name: code-review
+description: Only use this skill when explicitly asked to conduct a code review for a piece of code or a pull request.
+---
+
+Below is a set of different specific code review rules that you should check when asked for a review. Only check these rules and nothing else. Open-ended code reviews are not covered by this skill.
+
+When conducting a code review, read the diff and check it against each rule below. Read relevant pieces of code if needed for understanding the context around the diff. Each rule consists of the slug of the rule and textual description of what specifically should be checked by the rule.
+
+Your response should be a structured response in JSON form that you write in `.vscode/code-review.json` file in this repository.
+
+JSON should have strictly the following form:
+
+```json
+[
+    {
+        "name": "typos",
+        "file": "path/from/the/root/of/the/repo/file.py",
+        "line_from": 10,
+        "line_to": 20,
+        "description": "Word 'asembly' is written with a typo, you likely meant 'assembly'"
+    },
+    {
+        "name": "incorrect-comments",
+        "file": "path/from/the/root/of/the/repo/another.py",
+        "line_from": 15,
+        "line_to": 21,
+        "description": "The comment says that this algorithm works in O(N) but most common path according to numpy documentation is O(N^2)"
+    }
+]
+```
+
+You MUST adhere to this form because it will be later used by automation to create a user-friendly UI.
+
+If the rule asks for a citation, put it in the `description` field.
+
+Below are rules you should check when reviewing the code.
+
+### duplicate-code
+
+The code written is an obvious copy-pasta of the other part of the code in the diff or in other parts of the codebase. This rule only includes code that is an actual copy and paste of another part of the codebase or is very trivially extractable into functions.
+
+### incorrect-comments
+
+The comment used for a function, variable, or expression contradicts the content of the token it describes. This also includes docstrings or any other kind of documentation directly inside the code. Token names are not included in this rule.
+
+### misleading-name
+
+The name of a function, method, variable, parameter, or class contradicts what it actually does. Examples of contradictions: a get_* or fetch_* function that mutates state, an is_*/has_* name that does not return a boolean, a singular name bound to a collection, a boolean flag whose name implies the opposite polarity of the behavior it controls, or a verb that names a different operation than the one performed. Only flag when the mismatch is visible in the body of the token in the diff or in the code the diff calls. Do not flag names that are merely vague, short, or abbreviated.
+
+### typos
+
+A comment, token, or any other part of the code contains a typo.
+
+### obvious-comment
+
+A new or changed comment only restates what the next line or block already makes obvious, and adds no non-obvious rationale, constraint, or decision. Do not flag comments that explain why, trade-offs, invariants, or external constraints.
+
+### stale-reference
+
+The diff renames, moves, or deletes a symbol, module, file, CLI flag, config key, environment variable, or API field, but references to the old name survive elsewhere in the repository, in places the type checker does not see: comments, docstrings, error and log message strings, README or docs pages, YAML or TOML config, makefile targets, or test names. Only flag when you have located the surviving old reference and can cite its file and line.
+
+### caller-impact
+
+The diff changes the observable contract of a function used outside the diff, without updating those callers. Contract changes that count: the meaning or unit of a return value, None versus empty collection, ordering guarantees, which exception type is raised, whether an input argument is mutated, or the value of a default parameter. Only flag when at least one caller outside the diff is identified by file and line and would behave differently under the new contract. Do not flag changes the type checker would already reject.
+
+### unit-mismatch
+
+A quantity is produced in one unit, scale, or coordinate convention and consumed as if it were in another: degrees versus radians, seconds versus milliseconds, bytes versus kilobytes, zero-based versus one-based indices, or a value converted to a target unit in one branch but not another. Only flag when the source unit is determinable from the code, a column definition, a constant name, or a unit-bearing type in the diff or its immediate context.
+
+### swallowed-error
+
+An except clause catches an exception and then neither re-raises it, nor logs it, nor forwards it to the caller or user through the project's reporting mechanism, so the failure becomes invisible. Only flag when the handler body discards the exception entirely (pass, bare return, or assigning a fallback with no record of the failure). Do not flag handlers at documented process boundaries that log or report the exception, and do not flag handlers whose enclosing function's contract is explicitly to tolerate the failure.
+
+### inconsistent-duplicate
+
+The diff modifies one instance of code that is duplicated elsewhere in the repository while leaving the other instances unchanged, so the copies now disagree. Only flag when you have located the near-identical counterpart and can cite its file and line, and when the divergence affects behavior rather than only formatting.
+
+### generic-exception
+
+The code raises a built-in exception such as Exception, RuntimeError, or ValueError in a situation for which the project already defines a more specific exception class, or raises an exception belonging to a different layer than the one it is raised from. Only flag when the more appropriate exception class exists in the repository and is used for comparable situations elsewhere, cited by file and line.
+
+### layer-violation
+
+The diff introduces an import that inverts the dependency direction already established between two packages: module A imports package B, while B already imports A's package. Only flag when you can cite an existing import establishing the opposite direction. Also flag imports of code the project treats as generated or vendored being edited or wrapped in a way that a regeneration step would overwrite.
+
+### test-name-mismatch
+
+The name of a test describes a behavior, input, or outcome different from what the test body actually sets up and asserts. Only flag when the discrepancy is in the substance of the test, not in wording or abbreviation.
+
+### misleading-user-text
+
+Text that reaches an end user contradicts the code that produces or consumes it: a form field title or description, CLI help text, a task description, or an error message.
+
+### unmanaged-resource
+
+A resource that requires deterministic release — database connection or cursor, transaction, file, thread, executor, temporary directory, network client — is acquired in the diff without a with block or an equivalent guaranteed-cleanup path, so it leaks when an exception is raised. Only flag when the project provides a context manager or cleanup helper for that resource, cited by file and line, or when the same resource is acquired with a with block elsewhere in the codebase.
+
+### call-in-loop
+
+A database query, HTTP request, or other heavy I/O call is issued once per item inside a loop where the project already provides a batched or bulk equivalent. Only flag when the batched helper exists, cited by file and line, and the loop bound is data-dependent rather than a small fixed number.

makefile

@@ -118,6 +118,7 @@ frontend:
 
 dev:
 	@set -e; \
+	rm -rf uploader/static; \
 	trap 'kill $$backend_pid $$frontend_pid 2>/dev/null || true; wait $$backend_pid $$frontend_pid 2>/dev/null || true' INT TERM EXIT; \
 	$(MAKE) --no-print-directory serve & \
 	backend_pid=$$!; \

tests/test_designation_upload.py

@@ -18,7 +18,7 @@ def _mock_client() -> Mock:
 
 
 @patch("uploader.app.structured.designations.upload.rawdata_batches")
-@patch("uploader.app.structured.designations.upload._fetch_column_units")
+@patch("uploader.app.structured.designations.upload.fetch_column_units")
 def test_simple_column_expression(
     mock_fetch_column_units: Mock,
     mock_rawdata_batches: Mock,
@@ -43,7 +43,7 @@ def test_simple_column_expression(
 
 
 @patch("uploader.app.structured.designations.upload.rawdata_batches")
-@patch("uploader.app.structured.designations.upload._fetch_column_units")
+@patch("uploader.app.structured.designations.upload.fetch_column_units")
 def test_composed_string_expression(
     mock_fetch_column_units: Mock,
     mock_rawdata_batches: Mock,
@@ -65,7 +65,7 @@ def test_composed_string_expression(
     assert total == 1
 
 
-@patch("uploader.app.structured.designations.upload._fetch_column_units")
+@patch("uploader.app.structured.designations.upload.fetch_column_units")
 def test_missing_referenced_columns(mock_fetch_column_units: Mock) -> None:
     mock_fetch_column_units.return_value = ({"other"}, {})
 
@@ -81,7 +81,7 @@ def test_missing_referenced_columns(mock_fetch_column_units: Mock) -> None:
 
 
 @patch("uploader.app.structured.designations.upload.rawdata_batches")
-@patch("uploader.app.structured.designations.upload._fetch_column_units")
+@patch("uploader.app.structured.designations.upload.fetch_column_units")
 def test_null_referenced_values_counted_as_unmatched(
     mock_fetch_column_units: Mock,
     mock_rawdata_batches: Mock,
@@ -104,7 +104,7 @@ def test_null_referenced_values_counted_as_unmatched(
 
 
 @patch("uploader.app.structured.designations.upload.rawdata_batches")
-@patch("uploader.app.structured.designations.upload._fetch_column_units")
+@patch("uploader.app.structured.designations.upload.fetch_column_units")
 def test_expression_evaluation_error_becomes_runtime_error(
     mock_fetch_column_units: Mock,
     mock_rawdata_batches: Mock,
@@ -126,7 +126,7 @@ def test_expression_evaluation_error_becomes_runtime_error(
 
 
 @patch("uploader.app.structured.designations.upload.rawdata_batches")
-@patch("uploader.app.structured.designations.upload._fetch_column_units")
+@patch("uploader.app.structured.designations.upload.fetch_column_units")
 def test_output_file_writes_id_designation_pairs(
     mock_fetch_column_units: Mock,
     mock_rawdata_batches: Mock,

tests/test_geometry_upload.py

@@ -30,7 +30,7 @@ def _base_expressions() -> dict[str, str]:
 
 
 @patch("uploader.app.structured.geometry.upload.rawdata_batches")
-@patch("uploader.app.structured.geometry.upload._fetch_column_units")
+@patch("uploader.app.structured.geometry.upload.fetch_column_units")
 def test_isophote_unit_conversion_error_includes_field_details(
     mock_fetch_column_units: Mock,
     mock_rawdata_batches: Mock,
@@ -79,7 +79,7 @@ def test_isophote_unit_conversion_error_includes_field_details(
 
 
 @patch("uploader.app.structured.geometry.upload.rawdata_batches")
-@patch("uploader.app.structured.geometry.upload._fetch_column_units")
+@patch("uploader.app.structured.geometry.upload.fetch_column_units")
 def test_constant_isophote_unit_error_omits_empty_columns(
     mock_fetch_column_units: Mock,
     mock_rawdata_batches: Mock,

tests/test_structured_catalog.py

@@ -0,0 +1,188 @@
+from unittest.mock import Mock, patch
+
+import pytest
+
+from uploader.app.lib import expression, formula
+from uploader.app.structured.generic.upload import is_numeric_datatype, upload_catalog_columns
+from uploader.clients.gen.client import adminapi
+from uploader.clients.gen.client.adminapi.models.catalog_field import CatalogField
+from uploader.clients.gen.client.adminapi.models.catalog_schema import CatalogSchema
+from uploader.clients.gen.client.adminapi.models.datatype_enum import DatatypeEnum
+from uploader.forms.structured_catalog import (
+    _task_description,
+    build_catalog_form,
+    register_structured_catalog_tasks,
+)
+from uploader.tasks import TASKS
+
+
+def _mock_storage(total: int = 1) -> Mock:
+    storage = Mock()
+    storage.query.return_value = [{"cnt": total}]
+    return storage
+
+
+def _mock_client() -> Mock:
+    return Mock(spec=adminapi.AuthenticatedClient)
+
+
+def _sample_schema() -> CatalogSchema:
+    return CatalogSchema(
+        catalog="demo",
+        title="Demo catalog",
+        description="Demo structured catalog.",
+        fields=[
+            CatalogField(name="label", data_type=DatatypeEnum.STRING, description="Object label"),
+            CatalogField(name="mag", data_type=DatatypeEnum.FLOAT, unit="mag", description="Magnitude"),
+            CatalogField(name="n", data_type=DatatypeEnum.INTEGER, required=False),
+        ],
+    )
+
+
+def test_is_numeric_datatype() -> None:
+    assert is_numeric_datatype(DatatypeEnum.FLOAT)
+    assert is_numeric_datatype(DatatypeEnum.INTEGER)
+    assert not is_numeric_datatype(DatatypeEnum.STRING)
+    assert not is_numeric_datatype(DatatypeEnum.TIMESTAMP_WITHOUT_TIME_ZONE)
+
+
+def test_build_catalog_form_numeric_fields_are_expressions() -> None:
+    form_model = build_catalog_form(_sample_schema())
+    fields = form_model.model_fields
+
+    assert fields["label"].title == "label column"
+    assert fields["label"].description == "Object label"
+
+    assert fields["mag"].title == "mag"
+    assert fields["mag"].description == "Expression. Magnitude"
+
+    assert fields["n"].title == "n"
+    assert fields["n"].description == "Expression. Value for n."
+    assert fields["n"].is_required() is False
+
+
+def test_task_description_matches_upload_expression_engine() -> None:
+    description = _task_description(_sample_schema())
+    assert "Demo structured catalog." in description
+    assert expression.expression_syntax_help() in description
+    assert formula.expression_syntax_help() not in description
+    assert "str(x)" not in description
+    assert "String literals and + concatenation are supported." not in description
+    assert "Modulo divisors must carry units" not in description
+
+
+@patch("uploader.forms.structured_catalog.log.logger.warning")
+@patch("uploader.forms.structured_catalog.fetch_catalogs", side_effect=RuntimeError("api down"))
+def test_register_structured_catalog_tasks_logs_fetch_failure(
+    _mock_fetch_catalogs: Mock,
+    mock_warning: Mock,
+) -> None:
+    before = set(TASKS)
+    register_structured_catalog_tasks()
+    assert set(TASKS) == before
+    mock_warning.assert_called_once()
+    assert mock_warning.call_args.args[0] == "failed to fetch catalogs for structured catalog tasks"
+    assert mock_warning.call_args.kwargs["error"] == "api down"
+
+
+@patch("uploader.app.structured.generic.upload.handle_call")
+@patch("uploader.app.structured.generic.upload.save_structured_data.sync_detailed")
+@patch("uploader.app.structured.generic.upload.rawdata_batches")
+@patch("uploader.app.structured.generic.upload.fetch_column_units")
+def test_upload_evaluates_numeric_expressions(
+    mock_fetch_column_units: Mock,
+    mock_rawdata_batches: Mock,
+    mock_sync_detailed: Mock,
+    mock_handle_call: Mock,
+) -> None:
+    mock_fetch_column_units.return_value = (
+        {"name", "e_ra", "count"},
+        {"name": "", "e_ra": "arcsec", "count": ""},
+    )
+    mock_rawdata_batches.return_value = iter(
+        [
+            [
+                {
+                    "hyperleda_internal_id": "000079ce-5ffd-82c6-3f75-3a083f0fde80",
+                    "name": "NGC 1",
+                    "e_ra": 0.5,
+                    "count": 3,
+                },
+            ],
+        ],
+    )
+
+    total = upload_catalog_columns(
+        _mock_storage(),
+        "test_table",
+        "demo",
+        column_map={"label": "name"},
+        expressions={
+            "mag": 'col("e_ra")',
+            "n": 'col("count") * 2',
+        },
+        field_types={
+            "label": DatatypeEnum.STRING,
+            "mag": DatatypeEnum.FLOAT,
+            "n": DatatypeEnum.INTEGER,
+        },
+        field_units={"mag": "arcsec"},
+        columns=["label", "mag", "n"],
+        batch_size=100,
+        client=_mock_client(),
+        write=True,
+        report_func=lambda _: None,
+    )
+
+    assert total == 1
+    mock_rawdata_batches.assert_called_once()
+    fetch_columns = mock_rawdata_batches.call_args.args[2]
+    assert set(fetch_columns) == {"name", "e_ra", "count"}
+    body = mock_sync_detailed.call_args.kwargs["body"]
+    assert body.catalog == "demo"
+    assert body.columns == ["label", "mag", "n"]
+    assert body.ids == ["000079ce-5ffd-82c6-3f75-3a083f0fde80"]
+    assert body.data == [["NGC 1", 0.5, 6]]
+
+
+@patch("uploader.app.structured.generic.upload.rawdata_batches")
+@patch("uploader.app.structured.generic.upload.fetch_column_units")
+def test_upload_unit_conversion_error_includes_field_details(
+    mock_fetch_column_units: Mock,
+    mock_rawdata_batches: Mock,
+) -> None:
+    mock_fetch_column_units.return_value = (
+        {"bt"},
+        {"bt": ""},
+    )
+    mock_rawdata_batches.return_value = iter(
+        [
+            [
+                {
+                    "hyperleda_internal_id": "000079ce-5ffd-82c6-3f75-3a083f0fde80",
+                    "bt": 12.5,
+                },
+            ],
+        ],
+    )
+
+    with pytest.raises(RuntimeError, match="failed to evaluate expressions for row") as exc_info:
+        upload_catalog_columns(
+            _mock_storage(),
+            "test_table",
+            "demo",
+            column_map={},
+            expressions={"mag": 'col("bt")'},
+            field_types={"mag": DatatypeEnum.FLOAT},
+            field_units={"mag": "arcsec"},
+            columns=["mag"],
+            batch_size=100,
+            client=_mock_client(),
+            write=False,
+            report_func=lambda _: None,
+        )
+
+    message = str(exc_info.value)
+    assert "mag" in message
+    assert 'col("bt")' in message
+    assert "arcsec" in message

uploader/app/catalogs.py

@@ -0,0 +1,11 @@
+from uploader.app.endpoints import env_map
+from uploader.app.upload import handle_call
+from uploader.clients.gen.client import adminapi
+from uploader.clients.gen.client.adminapi.api.default import get_catalogs
+from uploader.clients.gen.client.adminapi.models.catalog_schema import CatalogSchema
+
+
+def fetch_catalogs(*, base_url: str | None = None) -> list[CatalogSchema]:
+    client = adminapi.Client(base_url=base_url or env_map["prod"])
+    response = handle_call(get_catalogs.sync_detailed(client=client))
+    return list(response.data.catalogs)