feat: move import database metadata to metadata field

[sabrikaragonen]

Summary

• Moves operational metadata (row count, size, timestamps) from the asset description field to the structured metadata field in bruin import database
• On re-import, metadata is now always refreshed (previously it became stale since descriptions were never updated for existing assets)
• The description field now holds only the actual database table comment, and owner goes to the dedicated owner field

Test plan

• Unit tests pass (TestCreateAsset, TestBuildImportMetadata, TestBuildImportMetadataOptionalFields, TestReimportUpdatesMetadata)
• Full make test passes
• make format passes
• Manual: run bruin import database and verify YAML output has metadata: block instead of embedded description
• Manual: re-run import on same asset and verify metadata gets refreshed

🤖 Generated with Claude Code

[greptile-apps]

Prompt To Fix All With AI

This is a comment left during a code review.
Path: cmd/import.go
Line: 312-321

Comment:
**Stale import-generated metadata keys after re-import**

The merge loop only adds or overwrites keys that exist in `createdAsset.Metadata`; it never removes keys that were previously written by an import but are no longer returned by the DB. This directly contradicts the PR's stated goal: "On re-import, metadata is now always refreshed."

Concrete scenario:
1. First import: `RowCount` is available → `existingAsset.Metadata["row_count"] = "1 000 000"`.
2. Second import: DB no longer reports `RowCount` (stats dropped, permissions change, etc.) → `createdAsset.Metadata` has no `row_count` key.
3. After the merge loop the existing `row_count: "1 000 000"` stays untouched — permanently stale.

The same applies to `created_at`, `last_modified`, and `size`.

One way to fix this is to explicitly delete the known import-generated keys from `existingAsset.Metadata` before applying the merge, so absent metrics are cleared rather than left behind:

```go
// Always refresh metadata so it stays current on re-import.
// We merge key-by-key to preserve any user-added custom metadata keys.
if createdAsset.Metadata != nil {
    if existingAsset.Metadata == nil {
        existingAsset.Metadata = make(pipeline.EmptyStringMap)
    }
    // Clear import-generated keys first so absent metrics don't linger.
    for _, k := range []string{"extracted_at", "created_at", "last_modified", "row_count", "size"} {
        delete(existingAsset.Metadata, k)
    }
    for k, v := range createdAsset.Metadata {
        existingAsset.Metadata[k] = v
    }
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: cmd/import_test.go
Line: 341-352

Comment:
**Test duplicates production logic instead of exercising it**

`TestReimportUpdatesMetadata` manually copies the exact merge loop from `runImport()` rather than calling `runImport()` (or a helper extracted from it). This means if the merge logic in production changes, this test will continue to pass even though the behaviour is now wrong — it's testing a stale copy of the code, not the code itself.

Consider either:
- Extracting the merge logic into a package-level helper (e.g. `mergeAssetMetadata`) that both `runImport` and the test call, or
- Exercising the real `runImport()` path with a filesystem-backed test (even an in-memory `afero.MemMapFs`).

As written, the test provides false confidence that the re-import path works correctly.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: f262243}

[greptile-apps]

Stale import-generated metadata keys after re-import

The merge loop only adds or overwrites keys that exist in createdAsset.Metadata; it never removes keys that were previously written by an import but are no longer returned by the DB. This directly contradicts the PR's stated goal: "On re-import, metadata is now always refreshed."

Concrete scenario:

1. First import: RowCount is available → existingAsset.Metadata["row_count"] = "1 000 000".
2. Second import: DB no longer reports RowCount (stats dropped, permissions change, etc.) → createdAsset.Metadata has no row_count key.
3. After the merge loop the existing row_count: "1 000 000" stays untouched — permanently stale.

The same applies to created_at, last_modified, and size.

One way to fix this is to explicitly delete the known import-generated keys from existingAsset.Metadata before applying the merge, so absent metrics are cleared rather than left behind:

// Always refresh metadata so it stays current on re-import.
// We merge key-by-key to preserve any user-added custom metadata keys.
if createdAsset.Metadata != nil {
    if existingAsset.Metadata == nil {
        existingAsset.Metadata = make(pipeline.EmptyStringMap)
    }
    // Clear import-generated keys first so absent metrics don't linger.
    for _, k := range []string{"extracted_at", "created_at", "last_modified", "row_count", "size"} {
        delete(existingAsset.Metadata, k)
    }
    for k, v := range createdAsset.Metadata {
        existingAsset.Metadata[k] = v
    }
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: cmd/import.go
Line: 312-321

Comment:
**Stale import-generated metadata keys after re-import**

The merge loop only adds or overwrites keys that exist in `createdAsset.Metadata`; it never removes keys that were previously written by an import but are no longer returned by the DB. This directly contradicts the PR's stated goal: "On re-import, metadata is now always refreshed."

Concrete scenario:
1. First import: `RowCount` is available → `existingAsset.Metadata["row_count"] = "1 000 000"`.
2. Second import: DB no longer reports `RowCount` (stats dropped, permissions change, etc.) → `createdAsset.Metadata` has no `row_count` key.
3. After the merge loop the existing `row_count: "1 000 000"` stays untouched — permanently stale.

The same applies to `created_at`, `last_modified`, and `size`.

One way to fix this is to explicitly delete the known import-generated keys from `existingAsset.Metadata` before applying the merge, so absent metrics are cleared rather than left behind:

```go
// Always refresh metadata so it stays current on re-import.
// We merge key-by-key to preserve any user-added custom metadata keys.
if createdAsset.Metadata != nil {
    if existingAsset.Metadata == nil {
        existingAsset.Metadata = make(pipeline.EmptyStringMap)
    }
    // Clear import-generated keys first so absent metrics don't linger.
    for _, k := range []string{"extracted_at", "created_at", "last_modified", "row_count", "size"} {
        delete(existingAsset.Metadata, k)
    }
    for k, v := range createdAsset.Metadata {
        existingAsset.Metadata[k] = v
    }
}
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Test duplicates production logic instead of exercising it

TestReimportUpdatesMetadata manually copies the exact merge loop from runImport() rather than calling runImport() (or a helper extracted from it). This means if the merge logic in production changes, this test will continue to pass even though the behaviour is now wrong — it's testing a stale copy of the code, not the code itself.

Consider either:

• Extracting the merge logic into a package-level helper (e.g. mergeAssetMetadata) that both runImport and the test call, or
• Exercising the real runImport() path with a filesystem-backed test (even an in-memory afero.MemMapFs).

As written, the test provides false confidence that the re-import path works correctly.

Prompt To Fix With AI

This is a comment left during a code review.
Path: cmd/import_test.go
Line: 341-352

Comment:
**Test duplicates production logic instead of exercising it**

`TestReimportUpdatesMetadata` manually copies the exact merge loop from `runImport()` rather than calling `runImport()` (or a helper extracted from it). This means if the merge logic in production changes, this test will continue to pass even though the behaviour is now wrong — it's testing a stale copy of the code, not the code itself.

Consider either:
- Extracting the merge logic into a package-level helper (e.g. `mergeAssetMetadata`) that both `runImport` and the test call, or
- Exercising the real `runImport()` path with a filesystem-backed test (even an in-memory `afero.MemMapFs`).

As written, the test provides false confidence that the re-import path works correctly.

How can I resolve this? If you propose a fix, please make it concise.