feat: Route to return product info as Knowledge Panels #267

pascalypperciel · 2025-03-26T06:39:58Z

What

Creating a GET route that allows users to fetch product info as Knowledge Panels. They can either get all the keys from the product or a selected one.
Models to mimic the Knowledge Panels structure.
Unit tests to test 4 different test cases for my route.
I'd like to note that every whitespace modification happened automatically because of the .editorconfig file.

Screenshot

With an input `k`

Without an input `k`, so all the keys in the product

Fixes bug(s)

N/A

Part of

Create a Knowledge Panel compliant route for the Folksonomy Engine list of properties for a given barcode #185

All the whitespaces modifications and similar things were automatic

pascalypperciel · 2025-03-26T16:25:53Z

Bonjour @stephanegigandet et @teolemon!

The pytest check failed. After some digging, it seems this could be due to a connection leak in the test code. I think that the database connections aren't being closed correctly:

Personally, I think that fixing it would step outside the scope of this issue. I think I should create a new issue to fix the problem (which I'd love to try and fix myself). Then, I'm not sure which way would be preferable here:

I can revert the unit test in this PR so the checks pass for now, and reintroduce it once the connection issue is resolved.
We leave this PR as-is for the moment, fix the test issue, and return to this PR afterward.

Let me know what works best. I’m open to whichever path you think is better, or something else entirely if there's a better solution obviously 😄!

pascalypperciel · 2025-04-01T21:45:14Z

Will reopen when #270 is merged.

alexgarel

It's the right direction @pascalypperciel but I don't think we want this rendering ! (it's far too verbose and not user oriented, there should be only one panel per property, and inside it, put a sensible html)

@teolemon might be able to provide what we would like to display.

I think there is a "table"

alexgarel · 2025-04-04T14:41:53Z

folksonomy/api.py

+    query = f"""
+        SELECT product, k, v, owner, version, editor, last_edit, comment
+        FROM folksonomy
+        WHERE product = %s
+    """
+    params = [product]
+    if k:
+        query+= " AND k = %s"
+        params.append(k)
+    query += " ORDER BY k;"
+
+    cur, timing = await db.db_exec(query, tuple(params))
+    rows = await cur.fetchall()
+
+    if not rows:
+        raise HTTPException(
+                status_code=404,
+                detail="Could not find product or key",
+            )


I would prefer that we reuse existing API functions here (or refactor them to use a common method).

teolemon · 2025-04-04T15:26:38Z

Here's what we currently have in the mobile app:

@alexgarel @pascalypperciel I'd very much like to be able to:

have the property name link to all instances of that property
Have a little (?) icon linking to the property documentation on the wiki next to the property name
have the property value link to all instances of that value

pascalypperciel · 2025-04-05T01:14:52Z

Thank you for all the feedback to both of you! I will be bringing changes soon. 😄

codecov-commenter · 2025-04-05T01:22:50Z

Codecov Report

Attention: Patch coverage is 92.63158% with 7 lines in your changes missing coverage. Please review.

Project coverage is 90.21%. Comparing base (b9cda65) to head (04932ef).
Report is 93 commits behind head on main.

Files with missing lines	Patch %	Lines
folksonomy/knowledge_panels.py	88.00%	6 Missing ⚠️
folksonomy/utils.py	95.45%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #267      +/-   ##
==========================================
- Coverage   95.06%   90.21%   -4.85%     
==========================================
  Files           5        6       +1     
  Lines         324      460     +136     
==========================================
+ Hits          308      415     +107     
- Misses         16       45      +29

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pascalypperciel · 2025-04-12T02:17:04Z

Hello again @alexgarel !

So I switch from using a text element to a table element because that's how I interpreted this comment:

I think there is a "table"

I added every requirement from teolemon, hopefully in a more user-friendly format.

Here is an example of a return in JSON:

And here's the same body but in HTML:

And for the following comment:

I would prefer that we reuse existing API functions here (or refactor them to use a common method).

I looked for existing helpers to reuse but couldn’t find one that fit. I considered creating a new one, but it ended up requiring changes across multiple routes to make it work cleanly. I held back on that refactor to avoid stepping beyond the scope of this issue or introducing potential merge conflicts. Happy to go through with it if you think I should, or revisit this in a follow-up PR!

alexgarel

@pascalypperciel first thank you so much for your work, and really sorry for the latency to review.

Overall it's good (although I'm not really convinced that we need a table, but it can be changed later).

I propose some (minor) changes.

alexgarel · 2025-04-23T09:12:09Z

tests/test_main.py

+        panels = data["knowledge_panels"]
+        assert "color" in panels
+        assert "size" in panels
+        assert "x-pg-timing" in response.headers


No need to assert that here !

alexgarel · 2025-04-23T09:14:29Z

folksonomy/api.py

+    panels_by_key = {}
+    for row in rows:
+
+        product_value, k, v, _, _, _, _, _ = row
+
+        if k not in panels_by_key:
+            panels_by_key[k] = {
+                "Barcode": product_value,
+                "Key": k,
+                "Value": v
+            }
+        else:
+            data = panels_by_key[k]
+            if not data.get("Barcode") and product_value:
+                data["Barcode"] = product_value
+            if not data.get("Value") and v:
+                data["Value"] = v
+
+    panels = {}
+    for k, data in panels_by_key.items():
+        rows_html = ""
+        for property, value in data.items():
+            if value is None:
+                continue
+
+            # Links for Key and Value property
+            if property == "Key":
+                value_cell = (
+                    f'<a href="world.openfoodfacts.org/property/{value}">{value}</a> '
+                    f'<a href="wiki.openfoodfacts.org/Folksonomy/Property/{value}">&#8505;</a>'
+                )
+            elif property == "Value":
+                value_cell = f'<a href="world.openfoodfacts.org/property/{data["Key"]}/value/{value}">{value}</a>'
+            else:
+                value_cell = value
+
+            row_html = f"<tr><td>{property}</td><td>{value_cell}</td></tr>"
+            rows_html += row_html
+
+
+
+        table_html = f"<table>{rows_html}</table>"
+
+        table_element = TableElement(
+            id=k,
+            title=f"Folksonomy Data for '{k}'",
+            rows=table_html,
+            columns=[
+                TableColumn(type="text", text="Property"),
+                TableColumn(type="text", text="Value")
+            ]
+        )
+
+        element = Element(
+            type="table",
+            table_element=table_element
+        )
+
+        panel = Panel(
+            title_element=TitleElement(
+                title=f"Folksonomy Data for '{k}'",
+                name=k
+            ),
+            elements=[element]
+        )
+        panels[k] = panel


I would like this to be in a separate file. To avoid having a long method. Maybe knowelge_panels.py

alexgarel · 2025-04-23T09:15:55Z

folksonomy/models.py

+class TitleElement(BaseModel):
+    name: str
+    title: str
+
+class TableColumn(BaseModel):
+    type: str
+    text: str
+
+class TableElement(BaseModel):
+    id: str
+    title: str
+    rows: str
+    columns: List[TableColumn]
+
+class Element(BaseModel):
+    type: str
+    table_element: TableElement
+
+class Panel(BaseModel):
+    title_element: TitleElement
+    elements: List[Element]
+
+class ProductKnowledgePanels(BaseModel):
+    knowledge_panels: Dict[str, Panel]


Can you add simple docstring to every object you introduce and eventually document fields ? (it will be in the generated documentation).
You might copy information from openfoodfacts-server OpenAPI documentation.

…ircular dependency tests passed with pytest command

Pytest now successfully complete again

pascalypperciel · 2025-05-01T02:44:53Z

Hello @alexgarel

Thank you for the kind words and for your guidance on this PR!

Apologies for the delay on my side as well! My busy winter semester has just wrapped up, so I finally have more time to contribute. 😄

Please let me know if I can bring any improvements!

alexgarel · 2025-07-07T12:46:08Z

Hi @pascalypperciel, really sorry that I missed this PR. You did a great job.
This branch will conflict with #295,
so maybe I will wait for it to fix/merge it.

pascalypperciel added 3 commits March 26, 2025 01:19

Creation of the route and knowledge panels compatible models

8ada108

All the whitespaces modifications and similar things were automatic

Allow the user to specify which key they want

2218adc

Added unit tests + improved 404 message

21472d1

pascalypperciel requested a review from a team as a code owner March 26, 2025 06:39

github-actions bot assigned pascalypperciel Mar 26, 2025

teolemon requested a review from stephanegigandet March 26, 2025 09:13

teolemon added the 📖 Knowledge Panels label Mar 26, 2025

teolemon added this to 🏷️ Folksonomy Engine Mar 27, 2025

github-project-automation bot moved this to Todo in 🏷️ Folksonomy Engine Mar 27, 2025

pascalypperciel mentioned this pull request Mar 31, 2025

Fix Pytest Check Error: "sorry, too many clients already" #269

Closed

pascalypperciel marked this pull request as draft April 1, 2025 21:44

alexgarel reviewed Apr 4, 2025

View reviewed changes

teolemon linked an issue Apr 4, 2025 that may be closed by this pull request

Create a Knowledge Panel compliant route for the Folksonomy Engine list of properties for a given barcode #185

Open

Merge remote-tracking branch 'upstream/main' into kp-barcode-route

a3b7d61

Re-introducing my unit-test

95e5a62

teolemon mentioned this pull request Apr 8, 2025

Create a new group of knowledge panels for power users (not sent by default by the API) openfoodfacts/openfoodfacts-server#6321

Open

18 tasks

pascalypperciel marked this pull request as ready for review April 9, 2025 15:57

Switching to TableElement, as suggested in PR review

8e4a31c

alexgarel reviewed Apr 23, 2025

View reviewed changes

pascalypperciel added 5 commits April 30, 2025 20:15

Removing unnecessary asserts in test

487d6d7

Moved long route to dedicated knowledge_panel file + utils to avoid c…

4f950e8

…ircular dependency tests passed with pytest command

docstrings + document fields for the models I introduced

ed366c7

Merge branch 'main' into kp-barcode-route

9e44e02

Fix some errors after merging main.

4fec3fa

Pytest now successfully complete again

Fixed linting errors

04932ef

github-actions bot added the 💥 Merge Conflicts label Jul 29, 2025

Uh oh!

feat: Route to return product info as Knowledge Panels #267

Are you sure you want to change the base?

feat: Route to return product info as Knowledge Panels #267

Uh oh!

Conversation

pascalypperciel commented Mar 26, 2025

What

Screenshot

With an input k

Without an input k, so all the keys in the product

Fixes bug(s)

Part of

Uh oh!

pascalypperciel commented Mar 26, 2025

Uh oh!

pascalypperciel commented Apr 1, 2025

Uh oh!

alexgarel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexgarel Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

teolemon commented Apr 4, 2025

Uh oh!

pascalypperciel commented Apr 5, 2025

Uh oh!

codecov-commenter commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pascalypperciel commented Apr 12, 2025

Uh oh!

alexgarel left a comment

Choose a reason for hiding this comment

Uh oh!

alexgarel Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

alexgarel Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

alexgarel Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

pascalypperciel commented May 1, 2025

Uh oh!

alexgarel commented Jul 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

With an input `k`

Without an input `k`, so all the keys in the product

alexgarel left a comment •

edited

Loading

codecov-commenter commented Apr 5, 2025 •

edited

Loading